Auto merge of #164224 - JonathanBrouwer:rollup-HbN5FJA, r=JonathanBrouwer

Rollup of 4 perf-sensitive pull requests

Successful merges:

 - #163425 (Treat union fields as aliasing each other in value_analysis)
 - #164074 (Avoid cloning impl trait AST nodes in late resolution)
 - #164089 (Implement `clone_from` for `BorrowckDomain`)
 - #164150 (Reserve more capacity for Symbol interner)

Author: bors

compiler/rustc_borrowck/src/dataflow.rs

@@ -103,13 +103,29 @@ impl JoinSemiLattice for BorrowckDomain {
 }
 
 /// The transient state of the dataflow analyses used by the borrow checker.
-#[derive(Clone, Debug, PartialEq, Eq)]
+#[derive(Debug, PartialEq, Eq)]
 pub(crate) struct BorrowckDomain {
     pub(crate) borrows: BorrowsDomain,
     pub(crate) uninits: MaybeUninitializedPlacesDomain,
     pub(crate) ever_inits: EverInitializedPlacesDomain,
 }
 
+impl Clone for BorrowckDomain {
+    fn clone(&self) -> Self {
+        Self {
+            borrows: self.borrows.clone(),
+            uninits: self.uninits.clone(),
+            ever_inits: self.ever_inits.clone(),
+        }
+    }
+
+    fn clone_from(&mut self, source: &Self) {
+        self.borrows.clone_from(&source.borrows);
+        self.uninits.clone_from(&source.uninits);
+        self.ever_inits.clone_from(&source.ever_inits);
+    }
+}
+
 rustc_index::newtype_index! {
     #[orderable]
     #[debug_format = "bw{}"]

compiler/rustc_mir_dataflow/src/value_analysis.rs

@@ -786,6 +786,12 @@ impl<'tcx> Map<'tcx> {
         };
         let elems = place.projection.iter().map(|&elem| elem.try_into()).chain(tail_elem.map(Ok));
         for elem in elems {
+            // All fields of a union alias each other.
+            if self.places[index].ty.is_union() {
+                self.for_each_value_inside(index, f);
+                return;
+            }
+
             // A field aliases the parent place.
             if let Some(vi) = self.places[index].value_index {
                 f(vi);

compiler/rustc_resolve/src/late.rs

@@ -824,7 +824,7 @@ pub(crate) struct DiagMetadata<'ast> {
     current_impl_item: Option<&'ast AssocItem>,
 
     /// When processing impl trait
-    currently_processing_impl_trait: Option<(TraitRef, Ty)>,
+    currently_processing_impl_trait: Option<(&'ast TraitRef, &'ast Ty)>,
 
     /// Accumulate the errors due to missed lifetime elision,
     /// and report them all at once for each function.
@@ -857,7 +857,7 @@ struct LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
     lifetime_elision_candidates: Option<Vec<(LifetimeRes, LifetimeElisionCandidate)>>,
 
     /// The trait that the current context can refer to.
-    current_trait_ref: Option<(Module<'ra>, TraitRef)>,
+    current_trait_ref: Option<(Module<'ra>, &'ast TraitRef)>,
 
     /// Fields used to add information to diagnostic errors.
     diag_metadata: Box<DiagMetadata<'ast>>,
@@ -2004,8 +2004,8 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
                         // are we trying to use an anonymous lifetime
                         // on a non GAT associated trait type?
                         if !self.in_func_body
-                            && let Some((module, _)) = &self.current_trait_ref
-                            && let Some(ty) = &self.diag_metadata.current_self_type
+                            && let Some((module, _)) = self.current_trait_ref
+                            && let Some(ty) = self.diag_metadata.current_self_type
                             && Some(true) == self.diag_metadata.in_non_gat_assoc_type
                             && let crate::ModuleKind::Def(DefKind::Trait, trait_id, _, _) =
                                 module.kind
@@ -3509,16 +3509,15 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
     /// This is called to resolve a trait reference from an `impl` (i.e., `impl Trait for Foo`).
     fn with_optional_trait_ref<T>(
         &mut self,
-        opt_trait_ref: Option<&TraitRef>,
+        opt_trait_ref: Option<&'ast TraitRef>,
         self_type: &'ast Ty,
         f: impl FnOnce(&mut Self, Option<DefId>) -> T,
     ) -> T {
         let mut new_val = None;
         let mut new_id = None;
         if let Some(trait_ref) = opt_trait_ref {
             let path: Vec<_> = Segment::from_path(&trait_ref.path);
-            self.diag_metadata.currently_processing_impl_trait =
-                Some((trait_ref.clone(), self_type.clone()));
+            self.diag_metadata.currently_processing_impl_trait = Some((trait_ref, self_type));
             let res = self.smart_resolve_path_fragment(
                 &None,
                 &path,
@@ -3530,7 +3529,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
             self.diag_metadata.currently_processing_impl_trait = None;
             if let Some(def_id) = res.expect_full_res().opt_def_id() {
                 new_id = Some(def_id);
-                new_val = Some((self.r.expect_module(def_id), trait_ref.clone()));
+                new_val = Some((self.r.expect_module(def_id), trait_ref));
             }
         }
         let original_trait_ref = replace(&mut self.current_trait_ref, new_val);
@@ -3917,7 +3916,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
         let Some(decl) = decl else {
             // We could not find the method: report an error.
             let candidate = self.find_similarly_named_assoc_item(reported_ident.name, kind);
-            let path = &self.current_trait_ref.as_ref().unwrap().1.path;
+            let path = &self.current_trait_ref.unwrap().1.path;
             let path_names = path_names_to_string(path);
             self.report_error(span, err(reported_ident, path_names, candidate));
             feed_visibility(self, module.def_id());
@@ -3931,7 +3930,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
         match seen_trait_items.entry(id_in_trait) {
             Entry::Occupied(entry) => {
                 let trait_span = decl.parent_module.unwrap().span.shrink_to_lo();
-                let impl_span = self.current_trait_ref.as_ref().unwrap().1.path.span;
+                let impl_span = self.current_trait_ref.unwrap().1.path.span;
                 self.report_error(
                     span,
                     ResolutionError::TraitImplDuplicate {
@@ -3961,7 +3960,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
         }
 
         // The method kind does not correspond to what appeared in the trait, report.
-        let path = &self.current_trait_ref.as_ref().unwrap().1.path;
+        let path = &self.current_trait_ref.unwrap().1.path;
         let (code, kind) = match kind {
             AssocItemKind::Const(..) => (E0323, "const"),
             AssocItemKind::Fn(..) => (E0324, "method"),
@@ -3973,7 +3972,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
         };
         let trait_path = path_names_to_string(path);
         let trait_span = decl.parent_module.unwrap().span.shrink_to_lo();
-        let impl_span = self.current_trait_ref.as_ref().unwrap().1.path.span;
+        let impl_span = self.current_trait_ref.unwrap().1.path.span;
         self.report_error(
             span,
             ResolutionError::TraitImplMismatch {
@@ -5542,7 +5541,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
 
     fn record_traits_in_scope(&mut self, node_id: NodeId, ident: Ident) {
         let traits = self.r.traits_in_scope(
-            self.current_trait_ref.as_ref().map(|(module, _)| *module),
+            self.current_trait_ref.map(|(module, _)| module),
             &self.parent_scope,
             ident.span,
             Some((ident.name, ValueNS)),

compiler/rustc_resolve/src/late/diagnostics.rs

@@ -1884,8 +1884,7 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
         res: Option<Res>,
         span: Span,
     ) {
-        if let Some((trait_ref, self_ty)) =
-            self.diag_metadata.currently_processing_impl_trait.clone()
+        if let Some((trait_ref, self_ty)) = self.diag_metadata.currently_processing_impl_trait
             && let TyKind::Path(_, self_ty_path) = &self_ty.kind
             && let PathResult::Module(ModuleOrUniformRoot::Module(module)) =
                 self.resolve_path(&Segment::from_path(self_ty_path), Some(TypeNS), None, source)
@@ -2921,15 +2920,15 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
         ident: Symbol,
         kind: &AssocItemKind,
     ) -> Option<Symbol> {
-        let (module, _) = self.current_trait_ref.as_ref()?;
+        let (module, _) = self.current_trait_ref?;
         if ident == kw::Underscore {
             // We do nothing for `_`.
             return None;
         }
 
         let targets = self
             .r
-            .resolutions(*module)
+            .resolutions(module)
             .iter()
             .filter_map(|(key, res)| {
                 res.borrow(self.r).best_decl().map(|binding| (key, binding.res()))

compiler/rustc_span/src/symbol.rs

@@ -2896,7 +2896,15 @@ impl Interner {
     // `ByteSymbol`.
     fn prefill(init: &[&'static str], extra: &[&'static str]) -> Self {
         let values = init.iter().copied().chain(extra.iter().copied()).map(|str| str.as_bytes());
-        let (size_hint, _) = values.size_hint();
+        // At the time of writing, `helloworld` Check benchmark interns ~1k additional symbols.
+        // Some of the noisy ones (`include-blob`, `tt-muncher`, `externs`, `large-workspace`, etc.)
+        // are between 1k and 2k. Lowest number among non-artificial benchmarks is ~5k for `ucd`.
+        // This was chosen to make small, artificial benchmarks more stable, and to
+        // reduce resizing for normal code without regresing memory use for smaller crates.
+        // FIXME(panstromek): we could do better if we take into account whether this is
+        //  Debug/Opt build. Those builds intern at least 6k more symbols than Check ones.
+        let additional_capacity = 2400;
+        let size_hint = values.size_hint().0 + additional_capacity;
         let mut conflicting_values: Vec<&[u8]> = Vec::new();
 
         let mut indices: HashTable<(&'static [u8], u32)> = HashTable::with_capacity(size_hint);

tests/mir-opt/dataflow-const-prop/union_field.read_other_field.DataflowConstProp.panic-abort.diff

@@ -0,0 +1,21 @@
+- // MIR for `read_other_field` before DataflowConstProp
++ // MIR for `read_other_field` after DataflowConstProp
+  
+  fn read_other_field() -> u8 {
+      let mut _0: u8;
+      let mut _1: U;
+      scope 1 {
+          debug u => _1;
+      }
+  
+      bb0: {
+          StorageLive(_1);
+          _1 = U { a: const 0_u8 };
+          (_1.0: u8) = const 1_u8;
+          (_1.1: u8) = const 5_u8;
+          _0 = copy (_1.0: u8);
+          StorageDead(_1);
+          return;
+      }
+  }
+  

tests/mir-opt/dataflow-const-prop/union_field.read_other_field.DataflowConstProp.panic-unwind.diff

@@ -0,0 +1,21 @@
+- // MIR for `read_other_field` before DataflowConstProp
++ // MIR for `read_other_field` after DataflowConstProp
+  
+  fn read_other_field() -> u8 {
+      let mut _0: u8;
+      let mut _1: U;
+      scope 1 {
+          debug u => _1;
+      }
+  
+      bb0: {
+          StorageLive(_1);
+          _1 = U { a: const 0_u8 };
+          (_1.0: u8) = const 1_u8;
+          (_1.1: u8) = const 5_u8;
+          _0 = copy (_1.0: u8);
+          StorageDead(_1);
+          return;
+      }
+  }
+  

tests/mir-opt/dataflow-const-prop/union_field.rs

@@ -0,0 +1,21 @@
+// EMIT_MIR_FOR_EACH_PANIC_STRATEGY
+// Writing to one union field must also invalidate what is known about the other fields,
+// since they share the same storage.
+
+//@ test-mir-pass: DataflowConstProp
+//@ compile-flags: --crate-type=lib
+
+union U {
+    a: u8,
+    b: u8,
+}
+
+// EMIT_MIR union_field.read_other_field.DataflowConstProp.diff
+pub fn read_other_field() -> u8 {
+    // CHECK-LABEL: fn read_other_field(
+    // CHECK: _0 = copy ({{_.*}}.0: u8);
+    let mut u = U { a: 0 };
+    u.a = 1;
+    u.b = 5;
+    unsafe { u.a }
+}

tests/mir-opt/jump_threading.rs

@@ -656,6 +656,42 @@ fn chained_conditions() -> u8 {
     }
 }
 
+union U {
+    a: u8,
+    b: u8,
+}
+
+fn union_field(x: u8) -> u8 {
+    // CHECK-LABEL: fn union_field(
+    // CHECK: debug u => [[u:_.*]];
+    // CHECK: bb0: {
+    // CHECK:     switchInt(copy ([[u]].0: u8)) -> [0: bb2, otherwise: bb1];
+    // CHECK: bb1: {
+    // CHECK:     goto -> bb3;
+    // CHECK: bb2: {
+    // CHECK:     goto -> bb3;
+    // CHECK: bb3: {
+    // CHECK:     ([[u]].1: u8) = const 5_u8;
+    // CHECK:     switchInt(copy ([[u]].0: u8)) -> [0: bb5, otherwise: bb4];
+
+    // Test for #161898, where the write to `u.b` did not invalidate what was known about `u.a`.
+    let mut u = U { a: x };
+    let r = unsafe {
+        match u.a {
+            0 => 100,
+            _ => 200,
+        }
+    };
+    u.b = 5;
+    let r2 = unsafe {
+        match u.a {
+            0 => 1,
+            _ => 2,
+        }
+    };
+    r + r2
+}
+
 fn main() {
     // CHECK-LABEL: fn main(
     too_complex(Ok(0));
@@ -673,6 +709,7 @@ fn main() {
     floats();
     bitwise_not();
     logical_not();
+    union_field(0);
 }
 
 // EMIT_MIR jump_threading.too_complex.JumpThreading.diff
@@ -693,3 +730,4 @@ fn main() {
 // EMIT_MIR jump_threading.bitwise_not.JumpThreading.diff
 // EMIT_MIR jump_threading.logical_not.JumpThreading.diff
 // EMIT_MIR jump_threading.chained_conditions.JumpThreading.diff
+// EMIT_MIR jump_threading.union_field.JumpThreading.diff

tests/mir-opt/jump_threading.union_field.JumpThreading.panic-abort.diff

@@ -0,0 +1,73 @@
+- // MIR for `union_field` before JumpThreading
++ // MIR for `union_field` after JumpThreading
+  
+  fn union_field(_1: u8) -> u8 {
+      debug x => _1;
+      let mut _0: u8;
+      let mut _2: U;
+      let mut _3: u8;
+      let mut _6: u8;
+      let mut _7: u8;
+      scope 1 {
+          debug u => _2;
+          let _4: u8;
+          scope 2 {
+              debug r => _4;
+              let _5: u8;
+              scope 3 {
+                  debug r2 => _5;
+              }
+          }
+      }
+  
+      bb0: {
+          StorageLive(_2);
+          StorageLive(_3);
+          _3 = copy _1;
+          _2 = U { a: move _3 };
+          StorageDead(_3);
+          StorageLive(_4);
+          switchInt(copy (_2.0: u8)) -> [0: bb2, otherwise: bb1];
+      }
+  
+      bb1: {
+          _4 = const 200_u8;
+          goto -> bb3;
+      }
+  
+      bb2: {
+          _4 = const 100_u8;
+          goto -> bb3;
+      }
+  
+      bb3: {
+          (_2.1: u8) = const 5_u8;
+          StorageLive(_5);
+          switchInt(copy (_2.0: u8)) -> [0: bb5, otherwise: bb4];
+      }
+  
+      bb4: {
+          _5 = const 2_u8;
+          goto -> bb6;
+      }
+  
+      bb5: {
+          _5 = const 1_u8;
+          goto -> bb6;
+      }
+  
+      bb6: {
+          StorageLive(_6);
+          _6 = copy _4;
+          StorageLive(_7);
+          _7 = copy _5;
+          _0 = Add(move _6, move _7);
+          StorageDead(_7);
+          StorageDead(_6);
+          StorageDead(_5);
+          StorageDead(_4);
+          StorageDead(_2);
+          return;
+      }
+  }
+