Auto merge of rust-lang#139269 - matthiaskrgr:rollup-pk78gig, r=matthiaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#138992 (literal pattern lowering: use the pattern's type instead of the literal's in `const_to_pat`)
 - rust-lang#139211 (interpret: add a version of run_for_validation for &self)
 - rust-lang#139235 (`AstValidator` tweaks)
 - rust-lang#139237 (Add a dep kind for use of the anon node with zero dependencies)
 - rust-lang#139260 (Add dianqk to codegen reviewers)
 - rust-lang#139264 (Fix two incorrect turbofish suggestions)

r? `@ghost`
`@rustbot` modify labels: rollup

Author: bors

compiler/rustc_ast_passes/src/ast_validation.rs

@@ -8,8 +8,9 @@
 
 use std::assert_matches::assert_matches;
 use std::borrow::{Borrow, Cow};
+use std::cell::Cell;
 use std::collections::VecDeque;
-use std::{fmt, mem, ptr};
+use std::{fmt, ptr};
 
 use rustc_abi::{Align, HasDataLayout, Size};
 use rustc_ast::Mutability;
@@ -131,7 +132,7 @@ pub struct Memory<'tcx, M: Machine<'tcx>> {
     /// This stores whether we are currently doing reads purely for the purpose of validation.
     /// Those reads do not trigger the machine's hooks for memory reads.
     /// Needless to say, this must only be set with great care!
-    validation_in_progress: bool,
+    validation_in_progress: Cell<bool>,
 }
 
 /// A reference to some allocation that was already bounds-checked for the given region
@@ -158,7 +159,7 @@ impl<'tcx, M: Machine<'tcx>> Memory<'tcx, M> {
             alloc_map: M::MemoryMap::default(),
             extra_fn_ptr_map: FxIndexMap::default(),
             dead_alloc_map: FxIndexMap::default(),
-            validation_in_progress: false,
+            validation_in_progress: Cell::new(false),
         }
     }
 
@@ -715,15 +716,15 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
         // We want to call the hook on *all* accesses that involve an AllocId, including zero-sized
         // accesses. That means we cannot rely on the closure above or the `Some` branch below. We
         // do this after `check_and_deref_ptr` to ensure some basic sanity has already been checked.
-        if !self.memory.validation_in_progress {
+        if !self.memory.validation_in_progress.get() {
             if let Ok((alloc_id, ..)) = self.ptr_try_get_alloc_id(ptr, size_i64) {
                 M::before_alloc_read(self, alloc_id)?;
             }
         }
 
         if let Some((alloc_id, offset, prov, alloc)) = ptr_and_alloc {
             let range = alloc_range(offset, size);
-            if !self.memory.validation_in_progress {
+            if !self.memory.validation_in_progress.get() {
                 M::before_memory_read(
                     self.tcx,
                     &self.machine,
@@ -801,7 +802,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
     ) -> InterpResult<'tcx, Option<AllocRefMut<'a, 'tcx, M::Provenance, M::AllocExtra, M::Bytes>>>
     {
         let tcx = self.tcx;
-        let validation_in_progress = self.memory.validation_in_progress;
+        let validation_in_progress = self.memory.validation_in_progress.get();
 
         let size_i64 = i64::try_from(size.bytes()).unwrap(); // it would be an error to even ask for more than isize::MAX bytes
         let ptr_and_alloc = Self::check_and_deref_ptr(
@@ -1087,23 +1088,43 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
     ///
     /// We do this so Miri's allocation access tracking does not show the validation
     /// reads as spurious accesses.
-    pub fn run_for_validation<R>(&mut self, f: impl FnOnce(&mut Self) -> R) -> R {
+    pub fn run_for_validation_mut<R>(&mut self, f: impl FnOnce(&mut Self) -> R) -> R {
         // This deliberately uses `==` on `bool` to follow the pattern
         // `assert!(val.replace(new) == old)`.
         assert!(
-            mem::replace(&mut self.memory.validation_in_progress, true) == false,
+            self.memory.validation_in_progress.replace(true) == false,
             "`validation_in_progress` was already set"
         );
         let res = f(self);
         assert!(
-            mem::replace(&mut self.memory.validation_in_progress, false) == true,
+            self.memory.validation_in_progress.replace(false) == true,
+            "`validation_in_progress` was unset by someone else"
+        );
+        res
+    }
+
+    /// Runs the closure in "validation" mode, which means the machine's memory read hooks will be
+    /// suppressed. Needless to say, this must only be set with great care! Cannot be nested.
+    ///
+    /// We do this so Miri's allocation access tracking does not show the validation
+    /// reads as spurious accesses.
+    pub fn run_for_validation_ref<R>(&self, f: impl FnOnce(&Self) -> R) -> R {
+        // This deliberately uses `==` on `bool` to follow the pattern
+        // `assert!(val.replace(new) == old)`.
+        assert!(
+            self.memory.validation_in_progress.replace(true) == false,
+            "`validation_in_progress` was already set"
+        );
+        let res = f(self);
+        assert!(
+            self.memory.validation_in_progress.replace(false) == true,
             "`validation_in_progress` was unset by someone else"
         );
         res
     }
 
     pub(super) fn validation_in_progress(&self) -> bool {
-        self.memory.validation_in_progress
+        self.memory.validation_in_progress.get()
     }
 }
 
@@ -1375,7 +1396,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
         };
         let src_alloc = self.get_alloc_raw(src_alloc_id)?;
         let src_range = alloc_range(src_offset, size);
-        assert!(!self.memory.validation_in_progress, "we can't be copying during validation");
+        assert!(!self.memory.validation_in_progress.get(), "we can't be copying during validation");
         // For the overlapping case, it is crucial that we trigger the read hook
         // before the write hook -- the aliasing model cares about the order.
         M::before_memory_read(

compiler/rustc_const_eval/src/interpret/memory.rs

@@ -1322,7 +1322,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
         trace!("validate_operand_internal: {:?}, {:?}", *val, val.layout.ty);
 
         // Run the visitor.
-        self.run_for_validation(|ecx| {
+        self.run_for_validation_mut(|ecx| {
             let reset_padding = reset_provenance_and_padding && {
                 // Check if `val` is actually stored in memory. If not, padding is not even
                 // represented and we need not reset it.

compiler/rustc_const_eval/src/interpret/validity.rs

@@ -632,10 +632,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             {
                 let tcx = self.tcx;
                 trace!(?lt.hir_id.local_id, "polymorphic byte string lit");
-                self.typeck_results
-                    .borrow_mut()
-                    .treat_byte_string_as_slice
-                    .insert(lt.hir_id.local_id);
                 pat_ty =
                     Ty::new_imm_ref(tcx, tcx.lifetimes.re_static, Ty::new_slice(tcx, tcx.types.u8));
             }

compiler/rustc_hir_typeck/src/pat.rs

@@ -81,9 +81,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         debug!("used_trait_imports({:?}) = {:?}", item_def_id, used_trait_imports);
         wbcx.typeck_results.used_trait_imports = used_trait_imports;
 
-        wbcx.typeck_results.treat_byte_string_as_slice =
-            mem::take(&mut self.typeck_results.borrow_mut().treat_byte_string_as_slice);
-
         debug!("writeback: typeck results for {:?} are {:#?}", item_def_id, wbcx.typeck_results);
 
         self.tcx.arena.alloc(wbcx.typeck_results)

compiler/rustc_hir_typeck/src/writeback.rs

@@ -89,6 +89,7 @@ rustc_query_append!(define_dep_nodes![
     /// We use this to create a forever-red node.
     [] fn Red() -> (),
     [] fn SideEffect() -> (),
+    [] fn AnonZeroDeps() -> (),
     [] fn TraitSelect() -> (),
     [] fn CompileCodegenUnit() -> (),
     [] fn CompileMonoItem() -> (),

compiler/rustc_middle/src/dep_graph/dep_node.rs

@@ -53,6 +53,7 @@ impl Deps for DepsType {
     const DEP_KIND_NULL: DepKind = dep_kinds::Null;
     const DEP_KIND_RED: DepKind = dep_kinds::Red;
     const DEP_KIND_SIDE_EFFECT: DepKind = dep_kinds::SideEffect;
+    const DEP_KIND_ANON_ZERO_DEPS: DepKind = dep_kinds::AnonZeroDeps;
     const DEP_KIND_MAX: u16 = dep_node::DEP_KIND_VARIANTS - 1;
 }
 

compiler/rustc_middle/src/dep_graph/mod.rs

@@ -197,12 +197,6 @@ pub struct TypeckResults<'tcx> {
     /// formatting modified file tests/ui/coroutine/retain-resume-ref.rs
     pub coroutine_stalled_predicates: FxIndexSet<(ty::Predicate<'tcx>, ObligationCause<'tcx>)>,
 
-    /// We sometimes treat byte string literals (which are of type `&[u8; N]`)
-    /// as `&[u8]`, depending on the pattern in which they are used.
-    /// This hashset records all instances where we behave
-    /// like this to allow `const_to_pat` to reliably handle this situation.
-    pub treat_byte_string_as_slice: ItemLocalSet,
-
     /// Contains the data for evaluating the effect of feature `capture_disjoint_fields`
     /// on closure size.
     pub closure_size_eval: LocalDefIdMap<ClosureSizeProfileData<'tcx>>,
@@ -237,7 +231,6 @@ impl<'tcx> TypeckResults<'tcx> {
             closure_fake_reads: Default::default(),
             rvalue_scopes: Default::default(),
             coroutine_stalled_predicates: Default::default(),
-            treat_byte_string_as_slice: Default::default(),
             closure_size_eval: Default::default(),
             offset_of_data: Default::default(),
         }

compiler/rustc_middle/src/ty/typeck_results.rs

@@ -58,25 +58,13 @@ struct ConstToPat<'tcx> {
     span: Span,
     id: hir::HirId,
 
-    treat_byte_string_as_slice: bool,
-
     c: ty::Const<'tcx>,
 }
 
 impl<'tcx> ConstToPat<'tcx> {
     fn new(pat_ctxt: &PatCtxt<'_, 'tcx>, id: hir::HirId, span: Span, c: ty::Const<'tcx>) -> Self {
         trace!(?pat_ctxt.typeck_results.hir_owner);
-        ConstToPat {
-            tcx: pat_ctxt.tcx,
-            typing_env: pat_ctxt.typing_env,
-            span,
-            id,
-            treat_byte_string_as_slice: pat_ctxt
-                .typeck_results
-                .treat_byte_string_as_slice
-                .contains(&id.local_id),
-            c,
-        }
+        ConstToPat { tcx: pat_ctxt.tcx, typing_env: pat_ctxt.typing_env, span, id, c }
     }
 
     fn type_marked_structural(&self, ty: Ty<'tcx>) -> bool {
@@ -108,8 +96,6 @@ impl<'tcx> ConstToPat<'tcx> {
         uv: ty::UnevaluatedConst<'tcx>,
         ty: Ty<'tcx>,
     ) -> Box<Pat<'tcx>> {
-        trace!(self.treat_byte_string_as_slice);
-
         // It's not *technically* correct to be revealing opaque types here as borrowcheck has
         // not run yet. However, CTFE itself uses `TypingMode::PostAnalysis` unconditionally even
         // during typeck and not doing so has a lot of (undesirable) fallout (#101478, #119821).
@@ -307,21 +293,8 @@ impl<'tcx> ConstToPat<'tcx> {
                             ty,
                         );
                     } else {
-                        // `b"foo"` produces a `&[u8; 3]`, but you can't use constants of array type when
-                        // matching against references, you can only use byte string literals.
-                        // The typechecker has a special case for byte string literals, by treating them
-                        // as slices. This means we turn `&[T; N]` constants into slice patterns, which
-                        // has no negative effects on pattern matching, even if we're actually matching on
-                        // arrays.
-                        let pointee_ty = match *pointee_ty.kind() {
-                            ty::Array(elem_ty, _) if self.treat_byte_string_as_slice => {
-                                Ty::new_slice(tcx, elem_ty)
-                            }
-                            _ => *pointee_ty,
-                        };
                         // References have the same valtree representation as their pointee.
-                        let subpattern = self.valtree_to_pat(cv, pointee_ty);
-                        PatKind::Deref { subpattern }
+                        PatKind::Deref { subpattern: self.valtree_to_pat(cv, *pointee_ty) }
                     }
                 }
             },

compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs

@@ -11,7 +11,7 @@ use rustc_abi::{FieldIdx, Integer};
 use rustc_errors::codes::*;
 use rustc_hir::def::{CtorOf, DefKind, Res};
 use rustc_hir::pat_util::EnumerateAndAdjustIterator;
-use rustc_hir::{self as hir, RangeEnd};
+use rustc_hir::{self as hir, LangItem, RangeEnd};
 use rustc_index::Idx;
 use rustc_middle::mir::interpret::LitToConstInput;
 use rustc_middle::thir::{
@@ -130,7 +130,7 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
 
         // Lower the endpoint into a temporary `PatKind` that will then be
         // deconstructed to obtain the constant value and other data.
-        let mut kind: PatKind<'tcx> = self.lower_pat_expr(expr);
+        let mut kind: PatKind<'tcx> = self.lower_pat_expr(expr, None);
 
         // Unpeel any ascription or inline-const wrapper nodes.
         loop {
@@ -294,7 +294,7 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
 
             hir::PatKind::Never => PatKind::Never,
 
-            hir::PatKind::Expr(value) => self.lower_pat_expr(value),
+            hir::PatKind::Expr(value) => self.lower_pat_expr(value, Some(ty)),
 
             hir::PatKind::Range(ref lo_expr, ref hi_expr, end) => {
                 let (lo_expr, hi_expr) = (lo_expr.as_deref(), hi_expr.as_deref());
@@ -630,7 +630,11 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
     /// - Paths (e.g. `FOO`, `foo::BAR`, `Option::None`)
     /// - Inline const blocks (e.g. `const { 1 + 1 }`)
     /// - Literals, possibly negated (e.g. `-128u8`, `"hello"`)
-    fn lower_pat_expr(&mut self, expr: &'tcx hir::PatExpr<'tcx>) -> PatKind<'tcx> {
+    fn lower_pat_expr(
+        &mut self,
+        expr: &'tcx hir::PatExpr<'tcx>,
+        pat_ty: Option<Ty<'tcx>>,
+    ) -> PatKind<'tcx> {
         let (lit, neg) = match &expr.kind {
             hir::PatExprKind::Path(qpath) => {
                 return self.lower_path(qpath, expr.hir_id, expr.span).kind;
@@ -641,7 +645,31 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
             hir::PatExprKind::Lit { lit, negated } => (lit, *negated),
         };
 
-        let ct_ty = self.typeck_results.node_type(expr.hir_id);
+        // We handle byte string literal patterns by using the pattern's type instead of the
+        // literal's type in `const_to_pat`: if the literal `b"..."` matches on a slice reference,
+        // the pattern's type will be `&[u8]` whereas the literal's type is `&[u8; 3]`; using the
+        // pattern's type means we'll properly translate it to a slice reference pattern. This works
+        // because slices and arrays have the same valtree representation.
+        // HACK: As an exception, use the literal's type if `pat_ty` is `String`; this can happen if
+        // `string_deref_patterns` is enabled. There's a special case for that when lowering to MIR.
+        // FIXME(deref_patterns): This hack won't be necessary once `string_deref_patterns` is
+        // superseded by a more general implementation of deref patterns.
+        let ct_ty = match pat_ty {
+            Some(pat_ty)
+                if let ty::Adt(def, _) = *pat_ty.kind()
+                    && self.tcx.is_lang_item(def.did(), LangItem::String) =>
+            {
+                if !self.tcx.features().string_deref_patterns() {
+                    span_bug!(
+                        expr.span,
+                        "matching on `String` went through without enabling string_deref_patterns"
+                    );
+                }
+                self.typeck_results.node_type(expr.hir_id)
+            }
+            Some(pat_ty) => pat_ty,
+            None => self.typeck_results.node_type(expr.hir_id),
+        };
         let lit_input = LitToConstInput { lit: &lit.node, ty: ct_ty, neg };
         let constant = self.tcx.at(expr.span).lit_to_const(lit_input);
         self.const_to_pat(constant, ct_ty, expr.hir_id, lit.span).kind