Auto merge of #70449 - ecstatic-morse:visit-body, r=oli-obk

Make `Visitor::visit_body` take a plain `&Body`

`ReadOnlyBodyAndCache` has replaced `&Body` in many parts of the code base that don't care about basic block predecessors. This includes the MIR `Visitor` trait, which I suspect resulted in many unnecessary changes in #64736. This reverts part of that PR to reduce the number of places where we need to pass a `ReadOnlyBodyAndCache`.

In the long term, we should either give `ReadOnlyBodyAndCache` more ergonomic name and replace all uses of `&mir::Body` with it at the cost of carrying an extra pointer everywhere, or use it only in places that actually need access to the predecessor cache. Perhaps there is an even nicer alternative.

r? @Nashenas88

src/librustc/mir/visit.rs

@@ -65,12 +65,12 @@ use rustc_span::Span;
 // variant argument) that does not require visiting, as in
 // `is_cleanup` above.
 
-macro_rules! body_cache_type {
-    (mut $a:lifetime, $tcx:lifetime) => {
+macro_rules! body_type {
+    (mut $tcx:lifetime) => {
         &mut BodyAndCache<$tcx>
     };
-    ($a:lifetime, $tcx:lifetime) => {
-        ReadOnlyBodyAndCache<$a, $tcx>
+    ($tcx:lifetime) => {
+        &Body<$tcx>
     };
 }
 
@@ -82,7 +82,7 @@ macro_rules! make_mir_visitor {
 
             fn visit_body(
                 &mut self,
-                body: body_cache_type!($($mutability)? '_, 'tcx)
+                body: body_type!($($mutability)? 'tcx)
             ) {
                 self.super_body(body);
             }
@@ -254,7 +254,7 @@ macro_rules! make_mir_visitor {
 
             fn super_body(
                 &mut self,
-                $($mutability)? body: body_cache_type!($($mutability)? '_, 'tcx)
+                $($mutability)? body: body_type!($($mutability)? 'tcx)
             ) {
                 let span = body.span;
                 if let Some(yield_ty) = &$($mutability)? body.yield_ty {
@@ -819,7 +819,7 @@ macro_rules! make_mir_visitor {
 
             fn visit_location(
                 &mut self,
-                body: body_cache_type!($($mutability)? '_, 'tcx),
+                body: body_type!($($mutability)? 'tcx),
                 location: Location
             ) {
                 let basic_block = & $($mutability)? body[location.block];

src/librustc_codegen_ssa/mir/analyze.rs

@@ -20,7 +20,7 @@ pub fn non_ssa_locals<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
     let mir = fx.mir;
     let mut analyzer = LocalAnalyzer::new(fx);
 
-    analyzer.visit_body(mir);
+    analyzer.visit_body(&mir);
 
     for (local, decl) in mir.local_decls.iter_enumerated() {
         let ty = fx.monomorphize(&decl.ty);

src/librustc_mir/borrow_check/borrow_set.rs

@@ -107,7 +107,7 @@ impl LocalsStateAtExit {
             LocalsStateAtExit::AllAreInvalidated
         } else {
             let mut has_storage_dead = HasStorageDead(BitSet::new_empty(body.local_decls.len()));
-            has_storage_dead.visit_body(body);
+            has_storage_dead.visit_body(&body);
             let mut has_storage_dead_or_moved = has_storage_dead.0;
             for move_out in &move_data.moves {
                 if let Some(index) = move_data.base_local(move_out.path) {

src/librustc_mir/borrow_check/diagnostics/conflict_errors.rs

@@ -1559,7 +1559,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
             }
         }
         let mut visitor = FakeReadCauseFinder { place, cause: None };
-        visitor.visit_body(self.body);
+        visitor.visit_body(&self.body);
         match visitor.cause {
             Some(FakeReadCause::ForMatchGuard) => Some("match guard"),
             Some(FakeReadCause::ForIndex) => Some("indexing expression"),

src/librustc_mir/borrow_check/invalidation.rs

@@ -37,7 +37,7 @@ pub(super) fn generate_invalidates<'tcx>(
             body: &body,
             dominators,
         };
-        ig.visit_body(body);
+        ig.visit_body(&body);
     }
 }
 

src/librustc_mir/borrow_check/mod.rs

@@ -299,8 +299,8 @@ fn do_mir_borrowck<'a, 'tcx>(
     }
 
     dataflow::visit_results(
-        &*body,
-        traversal::reverse_postorder(&*body).map(|(bb, _)| bb),
+        &body,
+        traversal::reverse_postorder(&body).map(|(bb, _)| bb),
         &results,
         &mut mbcx,
     );

src/librustc_mir/borrow_check/type_check/liveness/local_use_map.rs

@@ -81,7 +81,7 @@ impl LocalUseMap {
         live_locals.iter().for_each(|&local| locals_with_use_data[local] = true);
 
         LocalUseMapBuild { local_use_map: &mut local_use_map, elements, locals_with_use_data }
-            .visit_body(body);
+            .visit_body(&body);
 
         local_use_map
     }

src/librustc_mir/borrow_check/type_check/liveness/polonius.rs

@@ -101,7 +101,7 @@ pub(super) fn populate_access_facts(
             location_table,
             move_data,
         };
-        extractor.visit_body(body);
+        extractor.visit_body(&body);
 
         facts.var_dropped_at.extend(
             dropped_at.iter().map(|&(local, location)| (local, location_table.mid_index(location))),

src/librustc_mir/borrow_check/type_check/mod.rs

@@ -210,7 +210,7 @@ fn type_check_internal<'a, 'tcx, R>(
     );
     let errors_reported = {
         let mut verifier = TypeVerifier::new(&mut checker, *body, promoted);
-        verifier.visit_body(body);
+        verifier.visit_body(&body);
         verifier.errors_reported
     };
 
@@ -435,7 +435,7 @@ impl<'a, 'b, 'tcx> Visitor<'tcx> for TypeVerifier<'a, 'b, 'tcx> {
         }
     }
 
-    fn visit_body(&mut self, body: ReadOnlyBodyAndCache<'_, 'tcx>) {
+    fn visit_body(&mut self, body: &Body<'tcx>) {
         self.sanitize_type(&"return type", body.return_ty());
         for local_decl in &body.local_decls {
             self.sanitize_type(local_decl, local_decl.ty);
@@ -563,7 +563,7 @@ impl<'a, 'b, 'tcx> TypeVerifier<'a, 'b, 'tcx> {
 
         swap_constraints(self);
 
-        self.visit_body(promoted_body);
+        self.visit_body(&promoted_body);
 
         if !self.errors_reported {
             // if verifier failed, don't do further checks to avoid ICEs

src/librustc_mir/borrow_check/used_muts.rs

@@ -32,7 +32,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                 never_initialized_mut_locals: &mut never_initialized_mut_locals,
                 mbcx: self,
             };
-            visitor.visit_body(visitor.mbcx.body);
+            visitor.visit_body(&visitor.mbcx.body);
         }
 
         // Take the union of the existed `used_mut` set with those variables we've found were

src/librustc_mir/const_eval/eval_queries.rs

@@ -307,7 +307,7 @@ pub fn const_eval_raw_provider<'tcx>(
     );
 
     let res = ecx.load_mir(cid.instance.def, cid.promoted);
-    res.and_then(|body| eval_body_using_ecx(&mut ecx, cid, *body))
+    res.and_then(|body| eval_body_using_ecx(&mut ecx, cid, &body))
         .and_then(|place| {
             Ok(RawConst { alloc_id: place.ptr.assert_ptr().alloc_id, ty: place.layout.ty })
         })

src/librustc_mir/dataflow/impls/storage_liveness.rs

@@ -83,7 +83,7 @@ impl<'mir, 'tcx> MaybeRequiresStorage<'mir, 'tcx> {
     ) -> Self {
         MaybeRequiresStorage {
             body,
-            borrowed_locals: RefCell::new(ResultsRefCursor::new(*body, borrowed_locals)),
+            borrowed_locals: RefCell::new(ResultsRefCursor::new(&body, borrowed_locals)),
         }
     }
 }
@@ -250,7 +250,7 @@ impl<'mir, 'tcx> MaybeRequiresStorage<'mir, 'tcx> {
     /// Kill locals that are fully moved and have not been borrowed.
     fn check_for_move(&self, trans: &mut impl GenKill<Local>, loc: Location) {
         let mut visitor = MoveVisitor { trans, borrowed_locals: &self.borrowed_locals };
-        visitor.visit_location(self.body, loc);
+        visitor.visit_location(&self.body, loc);
     }
 }
 

src/librustc_mir/monomorphize/collector.rs

@@ -1162,7 +1162,7 @@ fn collect_neighbours<'tcx>(
     debug!("collect_neighbours: {:?}", instance.def_id());
     let body = tcx.instance_mir(instance.def);
 
-    MirNeighborCollector { tcx, body: &body, output, instance }.visit_body(body);
+    MirNeighborCollector { tcx, body: &body, output, instance }.visit_body(&body);
 }
 
 fn def_id_to_string(tcx: TyCtxt<'_>, def_id: DefId) -> String {

src/librustc_mir/transform/check_consts/validation.rs

@@ -183,7 +183,7 @@ impl Validator<'a, 'mir, 'tcx> {
             self.check_op_spanned(ops::Loop, body.span);
         }
 
-        self.visit_body(body);
+        self.visit_body(&body);
 
         // Ensure that the end result is `Sync` in a non-thread local `static`.
         let should_check_for_sync =

src/librustc_mir/transform/check_unsafety.rs

@@ -507,7 +507,7 @@ fn unsafety_check_result(tcx: TyCtxt<'_>, def_id: DefId) -> UnsafetyCheckResult
     // mir_built ensures that body has a computed cache, so we don't (and can't) attempt to
     // recompute it here.
     let body = body.unwrap_read_only();
-    checker.visit_body(body);
+    checker.visit_body(&body);
 
     check_unused_unsafe(tcx, def_id, &checker.used_unsafe, &mut checker.inherited_blocks);
     UnsafetyCheckResult {

src/librustc_mir/transform/const_prop.rs

@@ -778,7 +778,7 @@ impl CanConstProp {
                 trace!("local {:?} can't be const propagated because it's not a temporary", local);
             }
         }
-        cpv.visit_body(body);
+        cpv.visit_body(&body);
         cpv.can_const_prop
     }
 }

src/librustc_mir/transform/generator.rs

@@ -469,7 +469,7 @@ fn locals_live_across_suspend_points(
     // Find the MIR locals which do not use StorageLive/StorageDead statements.
     // The storage of these locals are always live.
     let mut ignored = StorageIgnored(BitSet::new_filled(body.local_decls.len()));
-    ignored.visit_body(body);
+    ignored.visit_body(&body);
 
     // Calculate the MIR locals which have been previously
     // borrowed (even if they are still active).

src/librustc_mir/transform/instcombine.rs

@@ -26,7 +26,7 @@ impl<'tcx> MirPass<'tcx> for InstCombine {
         let optimizations = {
             let read_only_cache = read_only!(body);
             let mut optimization_finder = OptimizationFinder::new(body, tcx);
-            optimization_finder.visit_body(read_only_cache);
+            optimization_finder.visit_body(&read_only_cache);
             optimization_finder.optimizations
         };
 

src/librustc_mir/transform/simplify.rs

@@ -309,7 +309,7 @@ impl<'tcx> MirPass<'tcx> for SimplifyLocals {
         let locals = {
             let read_only_cache = read_only!(body);
             let mut marker = DeclMarker { locals: BitSet::new_empty(body.local_decls.len()), body };
-            marker.visit_body(read_only_cache);
+            marker.visit_body(&read_only_cache);
             // Return pointer and arguments are always live
             marker.locals.insert(RETURN_PLACE);
             for arg in body.args_iter() {

src/librustc_mir/util/collect_writes.rs

@@ -1,7 +1,6 @@
 use rustc::mir::visit::PlaceContext;
 use rustc::mir::visit::Visitor;
-use rustc::mir::ReadOnlyBodyAndCache;
-use rustc::mir::{Local, Location};
+use rustc::mir::{Body, Local, Location};
 
 crate trait FindAssignments {
     // Finds all statements that assign directly to local (i.e., X = ...)
@@ -9,10 +8,10 @@ crate trait FindAssignments {
     fn find_assignments(&self, local: Local) -> Vec<Location>;
 }
 
-impl<'a, 'tcx> FindAssignments for ReadOnlyBodyAndCache<'a, 'tcx> {
+impl<'tcx> FindAssignments for Body<'tcx> {
     fn find_assignments(&self, local: Local) -> Vec<Location> {
         let mut visitor = FindLocalAssignmentVisitor { needle: local, locations: vec![] };
-        visitor.visit_body(*self);
+        visitor.visit_body(self);
         visitor.locations
     }
 }

src/librustc_mir/util/def_use.rs

@@ -38,7 +38,7 @@ impl DefUseAnalysis {
             var_debug_info_index: 0,
             in_var_debug_info: false,
         };
-        finder.visit_body(body);
+        finder.visit_body(&body);
         self.info = finder.info
     }