From c4c2d2c227440558d027d925a0a6baa2aa8a3ceb Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 23 Oct 2024 17:15:02 +1100 Subject: [PATCH 01/14] Reduce some visibilities. --- compiler/rustc_mir_transform/src/dataflow_const_prop.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_mir_transform/src/dataflow_const_prop.rs b/compiler/rustc_mir_transform/src/dataflow_const_prop.rs index 002216f50f2d1..3a28f29e92406 100644 --- a/compiler/rustc_mir_transform/src/dataflow_const_prop.rs +++ b/compiler/rustc_mir_transform/src/dataflow_const_prop.rs @@ -537,16 +537,16 @@ impl<'a, 'tcx> ConstAnalysis<'a, 'tcx> { } } -pub(crate) struct Patch<'tcx> { +struct Patch<'tcx> { tcx: TyCtxt<'tcx>, /// For a given MIR location, this stores the values of the operands used by that location. In /// particular, this is before the effect, such that the operands of `_1 = _1 + _2` are /// properly captured. (This may become UB soon, but it is currently emitted even by safe code.) - pub(crate) before_effect: FxHashMap<(Location, Place<'tcx>), Const<'tcx>>, + before_effect: FxHashMap<(Location, Place<'tcx>), Const<'tcx>>, /// Stores the assigned values for assignments where the Rvalue is constant. - pub(crate) assignments: FxHashMap>, + assignments: FxHashMap>, } impl<'tcx> Patch<'tcx> { From 52b90f7c16b275de761992fce19397f8db20c815 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 14 Oct 2024 16:15:33 +1100 Subject: [PATCH 02/14] In `MaybeRequiresStorage`, store `Results` instead of `ResultsCursor`. This requires a `clone` call in `check_for_move`, but makes `MaybeRequiresStorage` an immutable analysis, which enables the next commit. --- .../src/impls/storage_liveness.rs | 31 +++++++++---------- compiler/rustc_mir_transform/src/coroutine.rs | 9 +++--- 2 files changed, 19 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs b/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs index c5fd2a631ff74..fdbbab9ac4a68 100644 --- a/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs +++ b/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs @@ -5,7 +5,7 @@ use rustc_middle::mir::visit::{NonMutatingUseContext, PlaceContext, Visitor}; use rustc_middle::mir::*; use super::MaybeBorrowedLocals; -use crate::{Analysis, GenKill, ResultsCursor}; +use crate::{Analysis, GenKill, Results, ResultsCursor}; pub struct MaybeStorageLive<'a> { always_live_locals: Cow<'a, BitSet>, @@ -96,17 +96,19 @@ impl<'a, 'tcx> Analysis<'tcx> for MaybeStorageDead<'a> { } } -type BorrowedLocalsResults<'mir, 'tcx> = ResultsCursor<'mir, 'tcx, MaybeBorrowedLocals>; - /// Dataflow analysis that determines whether each local requires storage at a /// given location; i.e. whether its storage can go away without being observed. pub struct MaybeRequiresStorage<'mir, 'tcx> { - borrowed_locals: BorrowedLocalsResults<'mir, 'tcx>, + body: &'mir Body<'tcx>, + borrowed_locals: Results<'tcx, MaybeBorrowedLocals>, } impl<'mir, 'tcx> MaybeRequiresStorage<'mir, 'tcx> { - pub fn new(borrowed_locals: BorrowedLocalsResults<'mir, 'tcx>) -> Self { - MaybeRequiresStorage { borrowed_locals } + pub fn new( + body: &'mir Body<'tcx>, + borrowed_locals: Results<'tcx, MaybeBorrowedLocals>, + ) -> Self { + MaybeRequiresStorage { body, borrowed_locals } } } @@ -135,7 +137,7 @@ impl<'tcx> Analysis<'tcx> for MaybeRequiresStorage<'_, 'tcx> { loc: Location, ) { // If a place is borrowed in a statement, it needs storage for that statement. - self.borrowed_locals.mut_analysis().apply_statement_effect(trans, stmt, loc); + self.borrowed_locals.analysis.apply_statement_effect(trans, stmt, loc); match &stmt.kind { StatementKind::StorageDead(l) => trans.kill(*l), @@ -179,10 +181,7 @@ impl<'tcx> Analysis<'tcx> for MaybeRequiresStorage<'_, 'tcx> { loc: Location, ) { // If a place is borrowed in a terminator, it needs storage for that terminator. - self.borrowed_locals - .mut_analysis() - .transfer_function(trans) - .visit_terminator(terminator, loc); + self.borrowed_locals.analysis.transfer_function(trans).visit_terminator(terminator, loc); match &terminator.kind { TerminatorKind::Call { destination, .. } => { @@ -283,15 +282,15 @@ impl<'tcx> Analysis<'tcx> for MaybeRequiresStorage<'_, 'tcx> { impl<'tcx> MaybeRequiresStorage<'_, 'tcx> { /// Kill locals that are fully moved and have not been borrowed. - fn check_for_move(&mut self, trans: &mut >::Domain, loc: Location) { - let body = self.borrowed_locals.body(); - let mut visitor = MoveVisitor { trans, borrowed_locals: &mut self.borrowed_locals }; - visitor.visit_location(body, loc); + fn check_for_move(&self, trans: &mut >::Domain, loc: Location) { + let borrowed_locals = self.borrowed_locals.clone().into_results_cursor(self.body); + let mut visitor = MoveVisitor { trans, borrowed_locals }; + visitor.visit_location(self.body, loc); } } struct MoveVisitor<'a, 'mir, 'tcx> { - borrowed_locals: &'a mut BorrowedLocalsResults<'mir, 'tcx>, + borrowed_locals: ResultsCursor<'mir, 'tcx, MaybeBorrowedLocals>, trans: &'a mut BitSet, } diff --git a/compiler/rustc_mir_transform/src/coroutine.rs b/compiler/rustc_mir_transform/src/coroutine.rs index 0a413d68020dc..90f9c57fbfd60 100644 --- a/compiler/rustc_mir_transform/src/coroutine.rs +++ b/compiler/rustc_mir_transform/src/coroutine.rs @@ -679,11 +679,10 @@ fn locals_live_across_suspend_points<'tcx>( // Calculate the MIR locals that we actually need to keep storage around // for. - let mut requires_storage_cursor = - MaybeRequiresStorage::new(borrowed_locals_results.into_results_cursor(body)) - .into_engine(tcx, body) - .iterate_to_fixpoint() - .into_results_cursor(body); + let mut requires_storage_cursor = MaybeRequiresStorage::new(body, borrowed_locals_results) + .into_engine(tcx, body) + .iterate_to_fixpoint() + .into_results_cursor(body); // Calculate the liveness of MIR locals ignoring borrows. let mut liveness = MaybeLiveLocals From afbffe8ed3994df14ea8656fae0a942bbbbaf8da Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 14 Oct 2024 16:26:15 +1100 Subject: [PATCH 03/14] Make analyses and visitables immutable. Much nicer. --- compiler/rustc_borrowck/src/dataflow.rs | 16 +++++------ .../src/check_consts/resolver.rs | 6 ++-- .../src/framework/cursor.rs | 11 ++------ .../src/framework/direction.rs | 18 ++++++------ .../src/framework/engine.rs | 6 ++-- .../rustc_mir_dataflow/src/framework/mod.rs | 12 ++++---- .../rustc_mir_dataflow/src/framework/tests.rs | 8 +++--- .../src/framework/visitor.rs | 16 +++++------ .../src/impls/borrowed_locals.rs | 4 +-- .../src/impls/initialized.rs | 28 +++++++++---------- .../rustc_mir_dataflow/src/impls/liveness.rs | 12 ++++---- .../src/impls/storage_liveness.rs | 19 +++++-------- .../rustc_mir_dataflow/src/value_analysis.rs | 8 +++--- 13 files changed, 77 insertions(+), 87 deletions(-) diff --git a/compiler/rustc_borrowck/src/dataflow.rs b/compiler/rustc_borrowck/src/dataflow.rs index 89ff12c1479ae..15002ddff495b 100644 --- a/compiler/rustc_borrowck/src/dataflow.rs +++ b/compiler/rustc_borrowck/src/dataflow.rs @@ -44,7 +44,7 @@ impl<'a, 'tcx> ResultsVisitable<'tcx> for BorrowckResults<'a, 'tcx> { } fn reconstruct_before_statement_effect( - &mut self, + &self, state: &mut Self::Domain, stmt: &mir::Statement<'tcx>, loc: Location, @@ -55,7 +55,7 @@ impl<'a, 'tcx> ResultsVisitable<'tcx> for BorrowckResults<'a, 'tcx> { } fn reconstruct_statement_effect( - &mut self, + &self, state: &mut Self::Domain, stmt: &mir::Statement<'tcx>, loc: Location, @@ -66,7 +66,7 @@ impl<'a, 'tcx> ResultsVisitable<'tcx> for BorrowckResults<'a, 'tcx> { } fn reconstruct_before_terminator_effect( - &mut self, + &self, state: &mut Self::Domain, term: &mir::Terminator<'tcx>, loc: Location, @@ -77,7 +77,7 @@ impl<'a, 'tcx> ResultsVisitable<'tcx> for BorrowckResults<'a, 'tcx> { } fn reconstruct_terminator_effect( - &mut self, + &self, state: &mut Self::Domain, term: &mir::Terminator<'tcx>, loc: Location, @@ -511,7 +511,7 @@ impl<'tcx> rustc_mir_dataflow::Analysis<'tcx> for Borrows<'_, 'tcx> { } fn apply_before_statement_effect( - &mut self, + &self, trans: &mut Self::Domain, _statement: &mir::Statement<'tcx>, location: Location, @@ -520,7 +520,7 @@ impl<'tcx> rustc_mir_dataflow::Analysis<'tcx> for Borrows<'_, 'tcx> { } fn apply_statement_effect( - &mut self, + &self, trans: &mut Self::Domain, stmt: &mir::Statement<'tcx>, location: Location, @@ -568,7 +568,7 @@ impl<'tcx> rustc_mir_dataflow::Analysis<'tcx> for Borrows<'_, 'tcx> { } fn apply_before_terminator_effect( - &mut self, + &self, trans: &mut Self::Domain, _terminator: &mir::Terminator<'tcx>, location: Location, @@ -577,7 +577,7 @@ impl<'tcx> rustc_mir_dataflow::Analysis<'tcx> for Borrows<'_, 'tcx> { } fn apply_terminator_effect<'mir>( - &mut self, + &self, trans: &mut Self::Domain, terminator: &'mir mir::Terminator<'tcx>, _location: Location, diff --git a/compiler/rustc_const_eval/src/check_consts/resolver.rs b/compiler/rustc_const_eval/src/check_consts/resolver.rs index 74eb6b37fbbad..b7f5613dda762 100644 --- a/compiler/rustc_const_eval/src/check_consts/resolver.rs +++ b/compiler/rustc_const_eval/src/check_consts/resolver.rs @@ -330,7 +330,7 @@ where } fn apply_statement_effect( - &mut self, + &self, state: &mut Self::Domain, statement: &mir::Statement<'tcx>, location: Location, @@ -339,7 +339,7 @@ where } fn apply_terminator_effect<'mir>( - &mut self, + &self, state: &mut Self::Domain, terminator: &'mir mir::Terminator<'tcx>, location: Location, @@ -349,7 +349,7 @@ where } fn apply_call_return_effect( - &mut self, + &self, state: &mut Self::Domain, block: BasicBlock, return_places: CallReturnPlaces<'_, 'tcx>, diff --git a/compiler/rustc_mir_dataflow/src/framework/cursor.rs b/compiler/rustc_mir_dataflow/src/framework/cursor.rs index 5ebb343f4e195..bdbfd7d86915c 100644 --- a/compiler/rustc_mir_dataflow/src/framework/cursor.rs +++ b/compiler/rustc_mir_dataflow/src/framework/cursor.rs @@ -93,11 +93,6 @@ where &self.results.analysis } - /// Returns the `Analysis` used to generate the underlying `Results`. - pub fn mut_analysis(&mut self) -> &mut A { - &mut self.results.analysis - } - /// Resets the cursor to hold the entry set for the given basic block. /// /// For forward dataflow analyses, this is the dataflow state prior to the first statement. @@ -199,7 +194,7 @@ where let target_effect_index = effect.at_index(target.statement_index); A::Direction::apply_effects_in_range( - &mut self.results.analysis, + &self.results.analysis, &mut self.state, target.block, block_data, @@ -214,8 +209,8 @@ where /// /// This can be used, e.g., to apply the call return effect directly to the cursor without /// creating an extra copy of the dataflow state. - pub fn apply_custom_effect(&mut self, f: impl FnOnce(&mut A, &mut A::Domain)) { - f(&mut self.results.analysis, &mut self.state); + pub fn apply_custom_effect(&mut self, f: impl FnOnce(&A, &mut A::Domain)) { + f(&self.results.analysis, &mut self.state); self.state_needs_reset = true; } } diff --git a/compiler/rustc_mir_dataflow/src/framework/direction.rs b/compiler/rustc_mir_dataflow/src/framework/direction.rs index 3e01f0512c46f..7458e8379b1cb 100644 --- a/compiler/rustc_mir_dataflow/src/framework/direction.rs +++ b/compiler/rustc_mir_dataflow/src/framework/direction.rs @@ -16,7 +16,7 @@ pub trait Direction { /// /// `effects.start()` must precede or equal `effects.end()` in this direction. fn apply_effects_in_range<'tcx, A>( - analysis: &mut A, + analysis: &A, state: &mut A::Domain, block: BasicBlock, block_data: &mir::BasicBlockData<'tcx>, @@ -25,7 +25,7 @@ pub trait Direction { A: Analysis<'tcx>; fn apply_effects_in_block<'mir, 'tcx, A>( - analysis: &mut A, + analysis: &A, state: &mut A::Domain, block: BasicBlock, block_data: &'mir mir::BasicBlockData<'tcx>, @@ -43,7 +43,7 @@ pub trait Direction { R: ResultsVisitable<'tcx, Domain = D>; fn join_state_into_successors_of<'tcx, A>( - analysis: &mut A, + analysis: &A, body: &mir::Body<'tcx>, exit_state: &mut A::Domain, block: BasicBlock, @@ -60,7 +60,7 @@ impl Direction for Backward { const IS_FORWARD: bool = false; fn apply_effects_in_block<'mir, 'tcx, A>( - analysis: &mut A, + analysis: &A, state: &mut A::Domain, block: BasicBlock, block_data: &'mir mir::BasicBlockData<'tcx>, @@ -81,7 +81,7 @@ impl Direction for Backward { } fn apply_effects_in_range<'tcx, A>( - analysis: &mut A, + analysis: &A, state: &mut A::Domain, block: BasicBlock, block_data: &mir::BasicBlockData<'tcx>, @@ -190,7 +190,7 @@ impl Direction for Backward { } fn join_state_into_successors_of<'tcx, A>( - analysis: &mut A, + analysis: &A, body: &mir::Body<'tcx>, exit_state: &mut A::Domain, bb: BasicBlock, @@ -297,7 +297,7 @@ impl Direction for Forward { const IS_FORWARD: bool = true; fn apply_effects_in_block<'mir, 'tcx, A>( - analysis: &mut A, + analysis: &A, state: &mut A::Domain, block: BasicBlock, block_data: &'mir mir::BasicBlockData<'tcx>, @@ -317,7 +317,7 @@ impl Direction for Forward { } fn apply_effects_in_range<'tcx, A>( - analysis: &mut A, + analysis: &A, state: &mut A::Domain, block: BasicBlock, block_data: &mir::BasicBlockData<'tcx>, @@ -421,7 +421,7 @@ impl Direction for Forward { } fn join_state_into_successors_of<'tcx, A>( - analysis: &mut A, + analysis: &A, _body: &mir::Body<'tcx>, exit_state: &mut A::Domain, bb: BasicBlock, diff --git a/compiler/rustc_mir_dataflow/src/framework/engine.rs b/compiler/rustc_mir_dataflow/src/framework/engine.rs index cbd1083d037d2..37eae96eddf6c 100644 --- a/compiler/rustc_mir_dataflow/src/framework/engine.rs +++ b/compiler/rustc_mir_dataflow/src/framework/engine.rs @@ -116,7 +116,7 @@ where where A::Domain: DebugWithContext, { - let Engine { mut analysis, body, mut entry_sets, tcx, pass_name } = self; + let Engine { analysis, body, mut entry_sets, tcx, pass_name } = self; let mut dirty_queue: WorkQueue = WorkQueue::with_none(body.basic_blocks.len()); @@ -146,10 +146,10 @@ where // Apply the block transfer function, using the cached one if it exists. let edges = - A::Direction::apply_effects_in_block(&mut analysis, &mut state, bb, bb_data); + A::Direction::apply_effects_in_block(&analysis, &mut state, bb, bb_data); A::Direction::join_state_into_successors_of( - &mut analysis, + &analysis, body, &mut state, bb, diff --git a/compiler/rustc_mir_dataflow/src/framework/mod.rs b/compiler/rustc_mir_dataflow/src/framework/mod.rs index 959f1ea534090..90f9f88352537 100644 --- a/compiler/rustc_mir_dataflow/src/framework/mod.rs +++ b/compiler/rustc_mir_dataflow/src/framework/mod.rs @@ -136,7 +136,7 @@ pub trait Analysis<'tcx> { /// Updates the current dataflow state with the effect of evaluating a statement. fn apply_statement_effect( - &mut self, + &self, state: &mut Self::Domain, statement: &mir::Statement<'tcx>, location: Location, @@ -149,7 +149,7 @@ pub trait Analysis<'tcx> { /// *part* of the effect of a statement (e.g. for two-phase borrows). As a general rule, /// analyses should not implement this without also implementing `apply_statement_effect`. fn apply_before_statement_effect( - &mut self, + &self, _state: &mut Self::Domain, _statement: &mir::Statement<'tcx>, _location: Location, @@ -163,7 +163,7 @@ pub trait Analysis<'tcx> { /// `InitializedPlaces` analyses, the return place for a function call is not marked as /// initialized here. fn apply_terminator_effect<'mir>( - &mut self, + &self, _state: &mut Self::Domain, terminator: &'mir mir::Terminator<'tcx>, _location: Location, @@ -178,7 +178,7 @@ pub trait Analysis<'tcx> { /// *part* of the effect of a terminator (e.g. for two-phase borrows). As a general rule, /// analyses should not implement this without also implementing `apply_terminator_effect`. fn apply_before_terminator_effect( - &mut self, + &self, _state: &mut Self::Domain, _terminator: &mir::Terminator<'tcx>, _location: Location, @@ -193,7 +193,7 @@ pub trait Analysis<'tcx> { /// This is separate from `apply_terminator_effect` to properly track state across unwind /// edges. fn apply_call_return_effect( - &mut self, + &self, _state: &mut Self::Domain, _block: BasicBlock, _return_places: CallReturnPlaces<'_, 'tcx>, @@ -214,7 +214,7 @@ pub trait Analysis<'tcx> { /// engine doesn't need to clone the exit state for a block unless /// `SwitchIntEdgeEffects::apply` is actually called. fn apply_switch_int_edge_effects( - &mut self, + &self, _block: BasicBlock, _discr: &mir::Operand<'tcx>, _apply_edge_effects: &mut impl SwitchIntEdgeEffects, diff --git a/compiler/rustc_mir_dataflow/src/framework/tests.rs b/compiler/rustc_mir_dataflow/src/framework/tests.rs index 14fb6dfb50c80..1aa457e123b55 100644 --- a/compiler/rustc_mir_dataflow/src/framework/tests.rs +++ b/compiler/rustc_mir_dataflow/src/framework/tests.rs @@ -169,7 +169,7 @@ impl<'tcx, D: Direction> Analysis<'tcx> for MockAnalysis<'tcx, D> { } fn apply_statement_effect( - &mut self, + &self, state: &mut Self::Domain, _statement: &mir::Statement<'tcx>, location: Location, @@ -179,7 +179,7 @@ impl<'tcx, D: Direction> Analysis<'tcx> for MockAnalysis<'tcx, D> { } fn apply_before_statement_effect( - &mut self, + &self, state: &mut Self::Domain, _statement: &mir::Statement<'tcx>, location: Location, @@ -189,7 +189,7 @@ impl<'tcx, D: Direction> Analysis<'tcx> for MockAnalysis<'tcx, D> { } fn apply_terminator_effect<'mir>( - &mut self, + &self, state: &mut Self::Domain, terminator: &'mir mir::Terminator<'tcx>, location: Location, @@ -200,7 +200,7 @@ impl<'tcx, D: Direction> Analysis<'tcx> for MockAnalysis<'tcx, D> { } fn apply_before_terminator_effect( - &mut self, + &self, state: &mut Self::Domain, _terminator: &mir::Terminator<'tcx>, location: Location, diff --git a/compiler/rustc_mir_dataflow/src/framework/visitor.rs b/compiler/rustc_mir_dataflow/src/framework/visitor.rs index 3d6b008a6846d..6be2581265c3a 100644 --- a/compiler/rustc_mir_dataflow/src/framework/visitor.rs +++ b/compiler/rustc_mir_dataflow/src/framework/visitor.rs @@ -99,28 +99,28 @@ pub trait ResultsVisitable<'tcx> { fn reset_to_block_entry(&self, state: &mut Self::Domain, block: BasicBlock); fn reconstruct_before_statement_effect( - &mut self, + &self, state: &mut Self::Domain, statement: &mir::Statement<'tcx>, location: Location, ); fn reconstruct_statement_effect( - &mut self, + &self, state: &mut Self::Domain, statement: &mir::Statement<'tcx>, location: Location, ); fn reconstruct_before_terminator_effect( - &mut self, + &self, state: &mut Self::Domain, terminator: &mir::Terminator<'tcx>, location: Location, ); fn reconstruct_terminator_effect( - &mut self, + &self, state: &mut Self::Domain, terminator: &mir::Terminator<'tcx>, location: Location, @@ -143,7 +143,7 @@ where } fn reconstruct_before_statement_effect( - &mut self, + &self, state: &mut Self::Domain, stmt: &mir::Statement<'tcx>, loc: Location, @@ -152,7 +152,7 @@ where } fn reconstruct_statement_effect( - &mut self, + &self, state: &mut Self::Domain, stmt: &mir::Statement<'tcx>, loc: Location, @@ -161,7 +161,7 @@ where } fn reconstruct_before_terminator_effect( - &mut self, + &self, state: &mut Self::Domain, term: &mir::Terminator<'tcx>, loc: Location, @@ -170,7 +170,7 @@ where } fn reconstruct_terminator_effect( - &mut self, + &self, state: &mut Self::Domain, term: &mir::Terminator<'tcx>, loc: Location, diff --git a/compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs b/compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs index 859019fd1f6ee..80def9e1b4ae5 100644 --- a/compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs +++ b/compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs @@ -34,7 +34,7 @@ impl<'tcx> Analysis<'tcx> for MaybeBorrowedLocals { } fn apply_statement_effect( - &mut self, + &self, trans: &mut Self::Domain, statement: &Statement<'tcx>, location: Location, @@ -43,7 +43,7 @@ impl<'tcx> Analysis<'tcx> for MaybeBorrowedLocals { } fn apply_terminator_effect<'mir>( - &mut self, + &self, trans: &mut Self::Domain, terminator: &'mir Terminator<'tcx>, location: Location, diff --git a/compiler/rustc_mir_dataflow/src/impls/initialized.rs b/compiler/rustc_mir_dataflow/src/impls/initialized.rs index 9bb50d1e056bd..904aeb85bd023 100644 --- a/compiler/rustc_mir_dataflow/src/impls/initialized.rs +++ b/compiler/rustc_mir_dataflow/src/impls/initialized.rs @@ -328,7 +328,7 @@ impl<'tcx> Analysis<'tcx> for MaybeInitializedPlaces<'_, 'tcx> { } fn apply_statement_effect( - &mut self, + &self, trans: &mut Self::Domain, statement: &mir::Statement<'tcx>, location: Location, @@ -352,7 +352,7 @@ impl<'tcx> Analysis<'tcx> for MaybeInitializedPlaces<'_, 'tcx> { } fn apply_terminator_effect<'mir>( - &mut self, + &self, state: &mut Self::Domain, terminator: &'mir mir::Terminator<'tcx>, location: Location, @@ -372,7 +372,7 @@ impl<'tcx> Analysis<'tcx> for MaybeInitializedPlaces<'_, 'tcx> { } fn apply_call_return_effect( - &mut self, + &self, trans: &mut Self::Domain, _block: mir::BasicBlock, return_places: CallReturnPlaces<'_, 'tcx>, @@ -391,7 +391,7 @@ impl<'tcx> Analysis<'tcx> for MaybeInitializedPlaces<'_, 'tcx> { } fn apply_switch_int_edge_effects( - &mut self, + &self, block: mir::BasicBlock, discr: &mir::Operand<'tcx>, edge_effects: &mut impl SwitchIntEdgeEffects, @@ -457,7 +457,7 @@ impl<'tcx> Analysis<'tcx> for MaybeUninitializedPlaces<'_, 'tcx> { } fn apply_statement_effect( - &mut self, + &self, trans: &mut Self::Domain, _statement: &mir::Statement<'tcx>, location: Location, @@ -471,7 +471,7 @@ impl<'tcx> Analysis<'tcx> for MaybeUninitializedPlaces<'_, 'tcx> { } fn apply_terminator_effect<'mir>( - &mut self, + &self, trans: &mut Self::Domain, terminator: &'mir mir::Terminator<'tcx>, location: Location, @@ -489,7 +489,7 @@ impl<'tcx> Analysis<'tcx> for MaybeUninitializedPlaces<'_, 'tcx> { } fn apply_call_return_effect( - &mut self, + &self, trans: &mut Self::Domain, _block: mir::BasicBlock, return_places: CallReturnPlaces<'_, 'tcx>, @@ -508,7 +508,7 @@ impl<'tcx> Analysis<'tcx> for MaybeUninitializedPlaces<'_, 'tcx> { } fn apply_switch_int_edge_effects( - &mut self, + &self, block: mir::BasicBlock, discr: &mir::Operand<'tcx>, edge_effects: &mut impl SwitchIntEdgeEffects, @@ -576,7 +576,7 @@ impl<'a, 'tcx> Analysis<'tcx> for DefinitelyInitializedPlaces<'a, 'tcx> { } fn apply_statement_effect( - &mut self, + &self, trans: &mut Self::Domain, _statement: &mir::Statement<'tcx>, location: Location, @@ -587,7 +587,7 @@ impl<'a, 'tcx> Analysis<'tcx> for DefinitelyInitializedPlaces<'a, 'tcx> { } fn apply_terminator_effect<'mir>( - &mut self, + &self, trans: &mut Self::Domain, terminator: &'mir mir::Terminator<'tcx>, location: Location, @@ -599,7 +599,7 @@ impl<'a, 'tcx> Analysis<'tcx> for DefinitelyInitializedPlaces<'a, 'tcx> { } fn apply_call_return_effect( - &mut self, + &self, trans: &mut Self::Domain, _block: mir::BasicBlock, return_places: CallReturnPlaces<'_, 'tcx>, @@ -638,7 +638,7 @@ impl<'tcx> Analysis<'tcx> for EverInitializedPlaces<'_, 'tcx> { #[instrument(skip(self, trans), level = "debug")] fn apply_statement_effect( - &mut self, + &self, trans: &mut Self::Domain, stmt: &mir::Statement<'tcx>, location: Location, @@ -666,7 +666,7 @@ impl<'tcx> Analysis<'tcx> for EverInitializedPlaces<'_, 'tcx> { #[instrument(skip(self, trans, terminator), level = "debug")] fn apply_terminator_effect<'mir>( - &mut self, + &self, trans: &mut Self::Domain, terminator: &'mir mir::Terminator<'tcx>, location: Location, @@ -688,7 +688,7 @@ impl<'tcx> Analysis<'tcx> for EverInitializedPlaces<'_, 'tcx> { } fn apply_call_return_effect( - &mut self, + &self, trans: &mut Self::Domain, block: mir::BasicBlock, _return_places: CallReturnPlaces<'_, 'tcx>, diff --git a/compiler/rustc_mir_dataflow/src/impls/liveness.rs b/compiler/rustc_mir_dataflow/src/impls/liveness.rs index e06c1f2bb49e3..b825bd6093647 100644 --- a/compiler/rustc_mir_dataflow/src/impls/liveness.rs +++ b/compiler/rustc_mir_dataflow/src/impls/liveness.rs @@ -41,7 +41,7 @@ impl<'tcx> Analysis<'tcx> for MaybeLiveLocals { } fn apply_statement_effect( - &mut self, + &self, trans: &mut Self::Domain, statement: &mir::Statement<'tcx>, location: Location, @@ -50,7 +50,7 @@ impl<'tcx> Analysis<'tcx> for MaybeLiveLocals { } fn apply_terminator_effect<'mir>( - &mut self, + &self, trans: &mut Self::Domain, terminator: &'mir mir::Terminator<'tcx>, location: Location, @@ -60,7 +60,7 @@ impl<'tcx> Analysis<'tcx> for MaybeLiveLocals { } fn apply_call_return_effect( - &mut self, + &self, trans: &mut Self::Domain, _block: mir::BasicBlock, return_places: CallReturnPlaces<'_, 'tcx>, @@ -233,7 +233,7 @@ impl<'a, 'tcx> Analysis<'tcx> for MaybeTransitiveLiveLocals<'a> { } fn apply_statement_effect( - &mut self, + &self, trans: &mut Self::Domain, statement: &mir::Statement<'tcx>, location: Location, @@ -268,7 +268,7 @@ impl<'a, 'tcx> Analysis<'tcx> for MaybeTransitiveLiveLocals<'a> { } fn apply_terminator_effect<'mir>( - &mut self, + &self, trans: &mut Self::Domain, terminator: &'mir mir::Terminator<'tcx>, location: Location, @@ -278,7 +278,7 @@ impl<'a, 'tcx> Analysis<'tcx> for MaybeTransitiveLiveLocals<'a> { } fn apply_call_return_effect( - &mut self, + &self, trans: &mut Self::Domain, _block: mir::BasicBlock, return_places: CallReturnPlaces<'_, 'tcx>, diff --git a/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs b/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs index fdbbab9ac4a68..5afd174060d7f 100644 --- a/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs +++ b/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs @@ -39,7 +39,7 @@ impl<'a, 'tcx> Analysis<'tcx> for MaybeStorageLive<'a> { } fn apply_statement_effect( - &mut self, + &self, trans: &mut Self::Domain, stmt: &Statement<'tcx>, _: Location, @@ -83,7 +83,7 @@ impl<'a, 'tcx> Analysis<'tcx> for MaybeStorageDead<'a> { } fn apply_statement_effect( - &mut self, + &self, trans: &mut Self::Domain, stmt: &Statement<'tcx>, _: Location, @@ -131,7 +131,7 @@ impl<'tcx> Analysis<'tcx> for MaybeRequiresStorage<'_, 'tcx> { } fn apply_before_statement_effect( - &mut self, + &self, trans: &mut Self::Domain, stmt: &Statement<'tcx>, loc: Location, @@ -163,19 +163,14 @@ impl<'tcx> Analysis<'tcx> for MaybeRequiresStorage<'_, 'tcx> { } } - fn apply_statement_effect( - &mut self, - trans: &mut Self::Domain, - _: &Statement<'tcx>, - loc: Location, - ) { + fn apply_statement_effect(&self, trans: &mut Self::Domain, _: &Statement<'tcx>, loc: Location) { // If we move from a place then it only stops needing storage *after* // that statement. self.check_for_move(trans, loc); } fn apply_before_terminator_effect( - &mut self, + &self, trans: &mut Self::Domain, terminator: &Terminator<'tcx>, loc: Location, @@ -230,7 +225,7 @@ impl<'tcx> Analysis<'tcx> for MaybeRequiresStorage<'_, 'tcx> { } fn apply_terminator_effect<'t>( - &mut self, + &self, trans: &mut Self::Domain, terminator: &'t Terminator<'tcx>, loc: Location, @@ -271,7 +266,7 @@ impl<'tcx> Analysis<'tcx> for MaybeRequiresStorage<'_, 'tcx> { } fn apply_call_return_effect( - &mut self, + &self, trans: &mut Self::Domain, _block: BasicBlock, return_places: CallReturnPlaces<'_, 'tcx>, diff --git a/compiler/rustc_mir_dataflow/src/value_analysis.rs b/compiler/rustc_mir_dataflow/src/value_analysis.rs index faee40faa3f55..b4619b3e22e05 100644 --- a/compiler/rustc_mir_dataflow/src/value_analysis.rs +++ b/compiler/rustc_mir_dataflow/src/value_analysis.rs @@ -353,7 +353,7 @@ impl<'tcx, T: ValueAnalysis<'tcx>> Analysis<'tcx> for ValueAnalysisWrapper { } fn apply_statement_effect( - &mut self, + &self, state: &mut Self::Domain, statement: &Statement<'tcx>, _location: Location, @@ -364,7 +364,7 @@ impl<'tcx, T: ValueAnalysis<'tcx>> Analysis<'tcx> for ValueAnalysisWrapper { } fn apply_terminator_effect<'mir>( - &mut self, + &self, state: &mut Self::Domain, terminator: &'mir Terminator<'tcx>, _location: Location, @@ -377,7 +377,7 @@ impl<'tcx, T: ValueAnalysis<'tcx>> Analysis<'tcx> for ValueAnalysisWrapper { } fn apply_call_return_effect( - &mut self, + &self, state: &mut Self::Domain, _block: BasicBlock, return_places: CallReturnPlaces<'_, 'tcx>, @@ -388,7 +388,7 @@ impl<'tcx, T: ValueAnalysis<'tcx>> Analysis<'tcx> for ValueAnalysisWrapper { } fn apply_switch_int_edge_effects( - &mut self, + &self, _block: BasicBlock, _discr: &Operand<'tcx>, _apply_edge_effects: &mut impl SwitchIntEdgeEffects, From d990c004942ca4a8caa80f3f9be34ad476dc79c6 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 22 Oct 2024 10:48:30 +1100 Subject: [PATCH 04/14] Make results immutable. Because most of them are. The one exception is `ConstAnalysis`, which gains a `RefCell` to provide interior mutability. This is a bit annoying, but I think it's worth inconveniencing the one analysis that requires mutability in exchange for making the interface simpler for all the other analyses. --- compiler/rustc_borrowck/src/lib.rs | 10 +-- .../src/framework/cursor.rs | 5 -- .../src/framework/direction.rs | 6 +- .../src/framework/engine.rs | 2 +- .../src/framework/graphviz.rs | 18 ++--- .../src/framework/visitor.rs | 10 +-- compiler/rustc_mir_dataflow/src/points.rs | 8 +- compiler/rustc_mir_transform/src/coroutine.rs | 4 +- .../src/dataflow_const_prop.rs | 74 ++++++++++++------- 9 files changed, 74 insertions(+), 63 deletions(-) diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index 698fdafc93616..234061ac7a279 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -341,7 +341,7 @@ fn do_mir_borrowck<'tcx>( // Compute and report region errors, if any. mbcx.report_region_errors(nll_errors); - let mut results = BorrowckResults { + let results = BorrowckResults { ever_inits: flow_ever_inits, uninits: flow_uninits, borrows: flow_borrows, @@ -350,7 +350,7 @@ fn do_mir_borrowck<'tcx>( rustc_mir_dataflow::visit_results( body, traversal::reverse_postorder(body).map(|(bb, _)| bb), - &mut results, + &results, &mut mbcx, ); @@ -602,7 +602,7 @@ impl<'a, 'tcx, R> rustc_mir_dataflow::ResultsVisitor<'a, 'tcx, R> fn visit_statement_before_primary_effect( &mut self, - _results: &mut R, + _results: &R, state: &BorrowckDomain<'a, 'tcx>, stmt: &'a Statement<'tcx>, location: Location, @@ -672,7 +672,7 @@ impl<'a, 'tcx, R> rustc_mir_dataflow::ResultsVisitor<'a, 'tcx, R> fn visit_terminator_before_primary_effect( &mut self, - _results: &mut R, + _results: &R, state: &BorrowckDomain<'a, 'tcx>, term: &'a Terminator<'tcx>, loc: Location, @@ -785,7 +785,7 @@ impl<'a, 'tcx, R> rustc_mir_dataflow::ResultsVisitor<'a, 'tcx, R> fn visit_terminator_after_primary_effect( &mut self, - _results: &mut R, + _results: &R, state: &BorrowckDomain<'a, 'tcx>, term: &'a Terminator<'tcx>, loc: Location, diff --git a/compiler/rustc_mir_dataflow/src/framework/cursor.rs b/compiler/rustc_mir_dataflow/src/framework/cursor.rs index bdbfd7d86915c..2d091885dead9 100644 --- a/compiler/rustc_mir_dataflow/src/framework/cursor.rs +++ b/compiler/rustc_mir_dataflow/src/framework/cursor.rs @@ -83,11 +83,6 @@ where &self.results } - /// Returns the underlying `Results`. - pub fn mut_results(&mut self) -> &mut Results<'tcx, A> { - &mut self.results - } - /// Returns the `Analysis` used to generate the underlying `Results`. pub fn analysis(&self) -> &A { &self.results.analysis diff --git a/compiler/rustc_mir_dataflow/src/framework/direction.rs b/compiler/rustc_mir_dataflow/src/framework/direction.rs index 7458e8379b1cb..5b7ba899c0133 100644 --- a/compiler/rustc_mir_dataflow/src/framework/direction.rs +++ b/compiler/rustc_mir_dataflow/src/framework/direction.rs @@ -37,7 +37,7 @@ pub trait Direction { state: &mut D, block: BasicBlock, block_data: &'mir mir::BasicBlockData<'tcx>, - results: &mut R, + results: &R, vis: &mut impl ResultsVisitor<'mir, 'tcx, R, Domain = D>, ) where R: ResultsVisitable<'tcx, Domain = D>; @@ -161,7 +161,7 @@ impl Direction for Backward { state: &mut D, block: BasicBlock, block_data: &'mir mir::BasicBlockData<'tcx>, - results: &mut R, + results: &R, vis: &mut impl ResultsVisitor<'mir, 'tcx, R, Domain = D>, ) where R: ResultsVisitable<'tcx, Domain = D>, @@ -393,7 +393,7 @@ impl Direction for Forward { state: &mut F, block: BasicBlock, block_data: &'mir mir::BasicBlockData<'tcx>, - results: &mut R, + results: &R, vis: &mut impl ResultsVisitor<'mir, 'tcx, R, Domain = F>, ) where R: ResultsVisitable<'tcx, Domain = F>, diff --git a/compiler/rustc_mir_dataflow/src/framework/engine.rs b/compiler/rustc_mir_dataflow/src/framework/engine.rs index 37eae96eddf6c..890f2b53dcc30 100644 --- a/compiler/rustc_mir_dataflow/src/framework/engine.rs +++ b/compiler/rustc_mir_dataflow/src/framework/engine.rs @@ -52,7 +52,7 @@ where } pub fn visit_with<'mir>( - &mut self, + &self, body: &'mir mir::Body<'tcx>, blocks: impl IntoIterator, vis: &mut impl ResultsVisitor<'mir, 'tcx, Self, Domain = A::Domain>, diff --git a/compiler/rustc_mir_dataflow/src/framework/graphviz.rs b/compiler/rustc_mir_dataflow/src/framework/graphviz.rs index 96a70f4fa5b0c..82cdf1f2c715a 100644 --- a/compiler/rustc_mir_dataflow/src/framework/graphviz.rs +++ b/compiler/rustc_mir_dataflow/src/framework/graphviz.rs @@ -406,12 +406,8 @@ where w: &mut impl io::Write, block: BasicBlock, ) -> io::Result<()> { - let diffs = StateDiffCollector::run( - self.results.body(), - block, - self.results.mut_results(), - self.style, - ); + let diffs = + StateDiffCollector::run(self.results.body(), block, self.results.results(), self.style); let mut diffs_before = diffs.before.map(|v| v.into_iter()); let mut diffs_after = diffs.after.into_iter(); @@ -521,7 +517,7 @@ impl StateDiffCollector { fn run<'tcx, A>( body: &mir::Body<'tcx>, block: BasicBlock, - results: &mut Results<'tcx, A>, + results: &Results<'tcx, A>, style: OutputStyle, ) -> Self where @@ -560,7 +556,7 @@ where fn visit_statement_before_primary_effect( &mut self, - results: &mut Results<'tcx, A>, + results: &Results<'tcx, A>, state: &Self::Domain, _statement: &mir::Statement<'tcx>, _location: Location, @@ -573,7 +569,7 @@ where fn visit_statement_after_primary_effect( &mut self, - results: &mut Results<'tcx, A>, + results: &Results<'tcx, A>, state: &Self::Domain, _statement: &mir::Statement<'tcx>, _location: Location, @@ -584,7 +580,7 @@ where fn visit_terminator_before_primary_effect( &mut self, - results: &mut Results<'tcx, A>, + results: &Results<'tcx, A>, state: &Self::Domain, _terminator: &mir::Terminator<'tcx>, _location: Location, @@ -597,7 +593,7 @@ where fn visit_terminator_after_primary_effect( &mut self, - results: &mut Results<'tcx, A>, + results: &Results<'tcx, A>, state: &Self::Domain, _terminator: &mir::Terminator<'tcx>, _location: Location, diff --git a/compiler/rustc_mir_dataflow/src/framework/visitor.rs b/compiler/rustc_mir_dataflow/src/framework/visitor.rs index 6be2581265c3a..767099616774b 100644 --- a/compiler/rustc_mir_dataflow/src/framework/visitor.rs +++ b/compiler/rustc_mir_dataflow/src/framework/visitor.rs @@ -7,7 +7,7 @@ use super::{Analysis, Direction, Results}; pub fn visit_results<'mir, 'tcx, D, R>( body: &'mir mir::Body<'tcx>, blocks: impl IntoIterator, - results: &mut R, + results: &R, vis: &mut impl ResultsVisitor<'mir, 'tcx, R, Domain = D>, ) where R: ResultsVisitable<'tcx, Domain = D>, @@ -37,7 +37,7 @@ pub trait ResultsVisitor<'mir, 'tcx, R> { /// its `statement_effect`. fn visit_statement_before_primary_effect( &mut self, - _results: &mut R, + _results: &R, _state: &Self::Domain, _statement: &'mir mir::Statement<'tcx>, _location: Location, @@ -48,7 +48,7 @@ pub trait ResultsVisitor<'mir, 'tcx, R> { /// statement applied to `state`. fn visit_statement_after_primary_effect( &mut self, - _results: &mut R, + _results: &R, _state: &Self::Domain, _statement: &'mir mir::Statement<'tcx>, _location: Location, @@ -59,7 +59,7 @@ pub trait ResultsVisitor<'mir, 'tcx, R> { /// its `terminator_effect`. fn visit_terminator_before_primary_effect( &mut self, - _results: &mut R, + _results: &R, _state: &Self::Domain, _terminator: &'mir mir::Terminator<'tcx>, _location: Location, @@ -72,7 +72,7 @@ pub trait ResultsVisitor<'mir, 'tcx, R> { /// The `call_return_effect` (if one exists) will *not* be applied to `state`. fn visit_terminator_after_primary_effect( &mut self, - _results: &mut R, + _results: &R, _state: &Self::Domain, _terminator: &'mir mir::Terminator<'tcx>, _location: Location, diff --git a/compiler/rustc_mir_dataflow/src/points.rs b/compiler/rustc_mir_dataflow/src/points.rs index 73abb669a1117..895b4effbbc06 100644 --- a/compiler/rustc_mir_dataflow/src/points.rs +++ b/compiler/rustc_mir_dataflow/src/points.rs @@ -98,7 +98,7 @@ rustc_index::newtype_index! { pub fn save_as_intervals<'tcx, N, R>( elements: &DenseLocationMap, body: &mir::Body<'tcx>, - mut results: R, + results: R, ) -> SparseIntervalMatrix where N: Idx, @@ -109,7 +109,7 @@ where visit_results( body, body.basic_blocks.reverse_postorder().iter().copied(), - &mut results, + &results, &mut visitor, ); visitor.values @@ -128,7 +128,7 @@ where fn visit_statement_after_primary_effect( &mut self, - _results: &mut R, + _results: &R, state: &Self::Domain, _statement: &'mir mir::Statement<'tcx>, location: Location, @@ -142,7 +142,7 @@ where fn visit_terminator_after_primary_effect( &mut self, - _results: &mut R, + _results: &R, state: &Self::Domain, _terminator: &'mir mir::Terminator<'tcx>, location: Location, diff --git a/compiler/rustc_mir_transform/src/coroutine.rs b/compiler/rustc_mir_transform/src/coroutine.rs index 90f9c57fbfd60..43ebbf2f4d8dd 100644 --- a/compiler/rustc_mir_transform/src/coroutine.rs +++ b/compiler/rustc_mir_transform/src/coroutine.rs @@ -888,7 +888,7 @@ impl<'a, 'tcx, R> rustc_mir_dataflow::ResultsVisitor<'a, 'tcx, R> fn visit_statement_before_primary_effect( &mut self, - _results: &mut R, + _results: &R, state: &Self::Domain, _statement: &'a Statement<'tcx>, loc: Location, @@ -898,7 +898,7 @@ impl<'a, 'tcx, R> rustc_mir_dataflow::ResultsVisitor<'a, 'tcx, R> fn visit_terminator_before_primary_effect( &mut self, - _results: &mut R, + _results: &R, state: &Self::Domain, _terminator: &'a Terminator<'tcx>, loc: Location, diff --git a/compiler/rustc_mir_transform/src/dataflow_const_prop.rs b/compiler/rustc_mir_transform/src/dataflow_const_prop.rs index 3a28f29e92406..2636f1c54767c 100644 --- a/compiler/rustc_mir_transform/src/dataflow_const_prop.rs +++ b/compiler/rustc_mir_transform/src/dataflow_const_prop.rs @@ -2,6 +2,8 @@ //! //! Currently, this pass only propagates scalar values. +use std::cell::RefCell; + use rustc_const_eval::const_eval::{DummyMachine, throw_machine_stop_str}; use rustc_const_eval::interpret::{ ImmTy, Immediate, InterpCx, OpTy, PlaceTy, Projectable, interp_ok, @@ -73,7 +75,7 @@ struct ConstAnalysis<'a, 'tcx> { map: Map<'tcx>, tcx: TyCtxt<'tcx>, local_decls: &'a LocalDecls<'tcx>, - ecx: InterpCx<'tcx, DummyMachine>, + ecx: RefCell>, param_env: ty::ParamEnv<'tcx>, } @@ -237,6 +239,7 @@ impl<'tcx> ValueAnalysis<'tcx> for ConstAnalysis<'_, 'tcx> { match self.eval_operand(operand, state) { FlatSet::Elem(op) => self .ecx + .borrow() .int_to_int_or_float(&op, layout) .discard_err() .map_or(FlatSet::Top, |result| self.wrap_immediate(*result)), @@ -251,6 +254,7 @@ impl<'tcx> ValueAnalysis<'tcx> for ConstAnalysis<'_, 'tcx> { match self.eval_operand(operand, state) { FlatSet::Elem(op) => self .ecx + .borrow() .float_to_float_or_int(&op, layout) .discard_err() .map_or(FlatSet::Top, |result| self.wrap_immediate(*result)), @@ -274,6 +278,7 @@ impl<'tcx> ValueAnalysis<'tcx> for ConstAnalysis<'_, 'tcx> { Rvalue::UnaryOp(op, operand) => match self.eval_operand(operand, state) { FlatSet::Elem(value) => self .ecx + .borrow() .unary_op(*op, &value) .discard_err() .map_or(FlatSet::Top, |val| self.wrap_immediate(*val)), @@ -287,11 +292,10 @@ impl<'tcx> ValueAnalysis<'tcx> for ConstAnalysis<'_, 'tcx> { let val = match null_op { NullOp::SizeOf if layout.is_sized() => layout.size.bytes(), NullOp::AlignOf if layout.is_sized() => layout.align.abi.bytes(), - NullOp::OffsetOf(fields) => self - .ecx - .tcx - .offset_of_subfield(self.ecx.param_env(), layout, fields.iter()) - .bytes(), + NullOp::OffsetOf(fields) => { + let ecx = self.ecx.borrow(); + ecx.tcx.offset_of_subfield(ecx.param_env(), layout, fields.iter()).bytes() + } _ => return ValueOrPlace::Value(FlatSet::Top), }; FlatSet::Elem(Scalar::from_target_usize(val, &self.tcx)) @@ -343,7 +347,7 @@ impl<'a, 'tcx> ConstAnalysis<'a, 'tcx> { map, tcx, local_decls: &body.local_decls, - ecx: InterpCx::new(tcx, DUMMY_SP, param_env, DummyMachine), + ecx: RefCell::new(InterpCx::new(tcx, DUMMY_SP, param_env, DummyMachine)), param_env, } } @@ -369,8 +373,11 @@ impl<'a, 'tcx> ConstAnalysis<'a, 'tcx> { } } Operand::Constant(box constant) => { - if let Some(constant) = - self.ecx.eval_mir_constant(&constant.const_, constant.span, None).discard_err() + if let Some(constant) = self + .ecx + .borrow() + .eval_mir_constant(&constant.const_, constant.span, None) + .discard_err() { self.assign_constant(state, place, constant, &[]); } @@ -400,7 +407,9 @@ impl<'a, 'tcx> ConstAnalysis<'a, 'tcx> { return; } } - operand = if let Some(operand) = self.ecx.project(&operand, proj_elem).discard_err() { + operand = if let Some(operand) = + self.ecx.borrow().project(&operand, proj_elem).discard_err() + { operand } else { return; @@ -411,24 +420,32 @@ impl<'a, 'tcx> ConstAnalysis<'a, 'tcx> { place, operand, &mut |elem, op| match elem { - TrackElem::Field(idx) => self.ecx.project_field(op, idx.as_usize()).discard_err(), - TrackElem::Variant(idx) => self.ecx.project_downcast(op, idx).discard_err(), + TrackElem::Field(idx) => { + self.ecx.borrow().project_field(op, idx.as_usize()).discard_err() + } + TrackElem::Variant(idx) => { + self.ecx.borrow().project_downcast(op, idx).discard_err() + } TrackElem::Discriminant => { - let variant = self.ecx.read_discriminant(op).discard_err()?; - let discr_value = - self.ecx.discriminant_for_variant(op.layout.ty, variant).discard_err()?; + let variant = self.ecx.borrow().read_discriminant(op).discard_err()?; + let discr_value = self + .ecx + .borrow() + .discriminant_for_variant(op.layout.ty, variant) + .discard_err()?; Some(discr_value.into()) } TrackElem::DerefLen => { - let op: OpTy<'_> = self.ecx.deref_pointer(op).discard_err()?.into(); - let len_usize = op.len(&self.ecx).discard_err()?; + let ecx = self.ecx.borrow(); + let op: OpTy<'_> = ecx.deref_pointer(op).discard_err()?.into(); + let len_usize = op.len(&ecx).discard_err()?; let layout = self.tcx.layout_of(self.param_env.and(self.tcx.types.usize)).unwrap(); Some(ImmTy::from_uint(len_usize, layout).into()) } }, &mut |place, op| { - if let Some(imm) = self.ecx.read_immediate_raw(op).discard_err() + if let Some(imm) = self.ecx.borrow().read_immediate_raw(op).discard_err() && let Some(imm) = imm.right() { let elem = self.wrap_immediate(*imm); @@ -452,7 +469,7 @@ impl<'a, 'tcx> ConstAnalysis<'a, 'tcx> { (FlatSet::Bottom, _) | (_, FlatSet::Bottom) => (FlatSet::Bottom, FlatSet::Bottom), // Both sides are known, do the actual computation. (FlatSet::Elem(left), FlatSet::Elem(right)) => { - match self.ecx.binary_op(op, &left, &right).discard_err() { + match self.ecx.borrow().binary_op(op, &left, &right).discard_err() { // Ideally this would return an Immediate, since it's sometimes // a pair and sometimes not. But as a hack we always return a pair // and just make the 2nd component `Bottom` when it does not exist. @@ -523,8 +540,11 @@ impl<'a, 'tcx> ConstAnalysis<'a, 'tcx> { return None; } let enum_ty_layout = self.tcx.layout_of(self.param_env.and(enum_ty)).ok()?; - let discr_value = - self.ecx.discriminant_for_variant(enum_ty_layout.ty, variant_index).discard_err()?; + let discr_value = self + .ecx + .borrow() + .discriminant_for_variant(enum_ty_layout.ty, variant_index) + .discard_err()?; Some(discr_value.to_scalar()) } @@ -734,7 +754,7 @@ impl<'mir, 'tcx> #[instrument(level = "trace", skip(self, results, statement))] fn visit_statement_before_primary_effect( &mut self, - results: &mut Results<'tcx, ValueAnalysisWrapper>>, + results: &Results<'tcx, ValueAnalysisWrapper>>, state: &Self::Domain, statement: &'mir Statement<'tcx>, location: Location, @@ -744,7 +764,7 @@ impl<'mir, 'tcx> OperandCollector { state, visitor: self, - ecx: &mut results.analysis.0.ecx, + ecx: &mut results.analysis.0.ecx.borrow_mut(), map: &results.analysis.0.map, } .visit_rvalue(rvalue, location); @@ -756,7 +776,7 @@ impl<'mir, 'tcx> #[instrument(level = "trace", skip(self, results, statement))] fn visit_statement_after_primary_effect( &mut self, - results: &mut Results<'tcx, ValueAnalysisWrapper>>, + results: &Results<'tcx, ValueAnalysisWrapper>>, state: &Self::Domain, statement: &'mir Statement<'tcx>, location: Location, @@ -767,7 +787,7 @@ impl<'mir, 'tcx> } StatementKind::Assign(box (place, _)) => { if let Some(value) = self.try_make_constant( - &mut results.analysis.0.ecx, + &mut results.analysis.0.ecx.borrow_mut(), place, state, &results.analysis.0.map, @@ -781,7 +801,7 @@ impl<'mir, 'tcx> fn visit_terminator_before_primary_effect( &mut self, - results: &mut Results<'tcx, ValueAnalysisWrapper>>, + results: &Results<'tcx, ValueAnalysisWrapper>>, state: &Self::Domain, terminator: &'mir Terminator<'tcx>, location: Location, @@ -789,7 +809,7 @@ impl<'mir, 'tcx> OperandCollector { state, visitor: self, - ecx: &mut results.analysis.0.ecx, + ecx: &mut results.analysis.0.ecx.borrow_mut(), map: &results.analysis.0.map, } .visit_terminator(terminator, location); From dd24e0ea1c9f83c27d7d62dab74b6cd467c6df52 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 23 Oct 2024 14:33:55 +1100 Subject: [PATCH 05/14] Rename `BlockFormatter::results` as `BlockFormatter::cursor`. Because it's a `ResultsCursor`, not a `Results`. --- .../src/framework/graphviz.rs | 50 +++++++++---------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/compiler/rustc_mir_dataflow/src/framework/graphviz.rs b/compiler/rustc_mir_dataflow/src/framework/graphviz.rs index 82cdf1f2c715a..8a3510d01591d 100644 --- a/compiler/rustc_mir_dataflow/src/framework/graphviz.rs +++ b/compiler/rustc_mir_dataflow/src/framework/graphviz.rs @@ -96,13 +96,13 @@ where // the value with `None`, move it into the results cursor, move it // back out, and return it to the refcell wrapped in `Some`. let mut fmt = BlockFormatter { - results: results.take().unwrap().into_results_cursor(self.body), + cursor: results.take().unwrap().into_results_cursor(self.body), style: self.style, bg: Background::Light, }; fmt.write_node_label(&mut label, *block).unwrap(); - Some(fmt.results.into_results()) + Some(fmt.cursor.into_results()) }); dot::LabelText::html(String::from_utf8(label).unwrap()) } @@ -155,7 +155,7 @@ struct BlockFormatter<'mir, 'tcx, A> where A: Analysis<'tcx>, { - results: ResultsCursor<'mir, 'tcx, A>, + cursor: ResultsCursor<'mir, 'tcx, A>, bg: Background, style: OutputStyle, } @@ -219,8 +219,8 @@ where // C: State at start of block self.bg = Background::Light; - self.results.seek_to_block_start(block); - let block_start_state = self.results.get().clone(); + self.cursor.seek_to_block_start(block); + let block_start_state = self.cursor.get().clone(); self.write_row_with_full_state(w, "", "(on start)")?; // D + E: Statement and terminator transfer functions @@ -228,12 +228,12 @@ where // F: State at end of block - let terminator = self.results.body()[block].terminator(); + let terminator = self.cursor.body()[block].terminator(); // Write the full dataflow state immediately after the terminator if it differs from the // state at block entry. - self.results.seek_to_block_end(block); - if self.results.get() != &block_start_state || A::Direction::IS_BACKWARD { + self.cursor.seek_to_block_end(block); + if self.cursor.get() != &block_start_state || A::Direction::IS_BACKWARD { let after_terminator_name = match terminator.kind { mir::TerminatorKind::Call { target: Some(_), .. } => "(on unwind)", _ => "(on end)", @@ -250,8 +250,8 @@ where match terminator.kind { mir::TerminatorKind::Call { destination, .. } => { self.write_row(w, "", "(on successful return)", |this, w, fmt| { - let state_on_unwind = this.results.get().clone(); - this.results.apply_custom_effect(|analysis, state| { + let state_on_unwind = this.cursor.get().clone(); + this.cursor.apply_custom_effect(|analysis, state| { analysis.apply_call_return_effect( state, block, @@ -265,9 +265,9 @@ where colspan = this.style.num_state_columns(), fmt = fmt, diff = diff_pretty( - this.results.get(), + this.cursor.get(), &state_on_unwind, - this.results.analysis() + this.cursor.analysis() ), ) })?; @@ -275,8 +275,8 @@ where mir::TerminatorKind::Yield { resume, resume_arg, .. } => { self.write_row(w, "", "(on yield resume)", |this, w, fmt| { - let state_on_coroutine_drop = this.results.get().clone(); - this.results.apply_custom_effect(|analysis, state| { + let state_on_coroutine_drop = this.cursor.get().clone(); + this.cursor.apply_custom_effect(|analysis, state| { analysis.apply_call_return_effect( state, resume, @@ -290,9 +290,9 @@ where colspan = this.style.num_state_columns(), fmt = fmt, diff = diff_pretty( - this.results.get(), + this.cursor.get(), &state_on_coroutine_drop, - this.results.analysis() + this.cursor.analysis() ), ) })?; @@ -302,8 +302,8 @@ where if !targets.is_empty() => { self.write_row(w, "", "(on successful return)", |this, w, fmt| { - let state_on_unwind = this.results.get().clone(); - this.results.apply_custom_effect(|analysis, state| { + let state_on_unwind = this.cursor.get().clone(); + this.cursor.apply_custom_effect(|analysis, state| { analysis.apply_call_return_effect( state, block, @@ -317,9 +317,9 @@ where colspan = this.style.num_state_columns(), fmt = fmt, diff = diff_pretty( - this.results.get(), + this.cursor.get(), &state_on_unwind, - this.results.analysis() + this.cursor.analysis() ), ) })?; @@ -407,7 +407,7 @@ where block: BasicBlock, ) -> io::Result<()> { let diffs = - StateDiffCollector::run(self.results.body(), block, self.results.results(), self.style); + StateDiffCollector::run(self.cursor.body(), block, self.cursor.results(), self.style); let mut diffs_before = diffs.before.map(|v| v.into_iter()); let mut diffs_after = diffs.after.into_iter(); @@ -416,7 +416,7 @@ where if A::Direction::IS_FORWARD { it.next().unwrap() } else { it.next_back().unwrap() } }; - for (i, statement) in self.results.body()[block].statements.iter().enumerate() { + for (i, statement) in self.cursor.body()[block].statements.iter().enumerate() { let statement_str = format!("{statement:?}"); let index_str = format!("{i}"); @@ -438,7 +438,7 @@ where assert!(diffs_after.is_empty()); assert!(diffs_before.as_ref().map_or(true, ExactSizeIterator::is_empty)); - let terminator = self.results.body()[block].terminator(); + let terminator = self.cursor.body()[block].terminator(); let mut terminator_str = String::new(); terminator.kind.fmt_head(&mut terminator_str).unwrap(); @@ -488,8 +488,8 @@ where mir: &str, ) -> io::Result<()> { self.write_row(w, i, mir, |this, w, fmt| { - let state = this.results.get(); - let analysis = this.results.analysis(); + let state = this.cursor.get(); + let analysis = this.cursor.analysis(); // FIXME: The full state vector can be quite long. It would be nice to split on commas // and use some text wrapping algorithm. From a6fad98c995387b1cde25255bfe1384c28572e1f Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 23 Oct 2024 14:34:26 +1100 Subject: [PATCH 06/14] Simplify `graphviz::Formatter`. `Formatter` currently has a `RefCell>` field. This is so the `Results` can be temporarily taken and put into a `ResultsCursor` that is used by `BlockFormatter`, and then put back, which is messy. This commit changes `Formatter` to have a `RefCell` and `BlockFormatter` to have a `&mut ResultsCursor`, which greatly simplifies the code at the `Formatter`/`BlockFormatter` interaction point in `Formatter::node_label`. The commit also: - documents the reason for the `RefCell`; - adds a `Formatter::body` method, replacing the `Formatter::body` field. --- .../src/framework/graphviz.rs | 53 +++++++++---------- 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/compiler/rustc_mir_dataflow/src/framework/graphviz.rs b/compiler/rustc_mir_dataflow/src/framework/graphviz.rs index 8a3510d01591d..599b92867d7b9 100644 --- a/compiler/rustc_mir_dataflow/src/framework/graphviz.rs +++ b/compiler/rustc_mir_dataflow/src/framework/graphviz.rs @@ -32,8 +32,11 @@ pub(crate) struct Formatter<'mir, 'tcx, A> where A: Analysis<'tcx>, { - body: &'mir Body<'tcx>, - results: RefCell>>, + // The `RefCell` is used because `::node_label` + // takes `&self`, but it needs to modify the cursor. This is also the + // reason for the `Formatter`/`BlockFormatter` split; `BlockFormatter` has + // the operations that involve the mutation, i.e. within the `borrow_mut`. + cursor: RefCell>, style: OutputStyle, reachable: BitSet, } @@ -48,11 +51,15 @@ where style: OutputStyle, ) -> Self { let reachable = mir::traversal::reachable_as_bitset(body); - Formatter { body, results: Some(results).into(), style, reachable } + Formatter { cursor: results.into_results_cursor(body).into(), style, reachable } + } + + fn body(&self) -> &'mir Body<'tcx> { + self.cursor.borrow().body() } pub(crate) fn into_results(self) -> Results<'tcx, A> { - self.results.into_inner().unwrap() + self.cursor.into_inner().into_results() } } @@ -81,7 +88,7 @@ where type Edge = CfgEdge; fn graph_id(&self) -> dot::Id<'_> { - let name = graphviz_safe_def_name(self.body.source.def_id()); + let name = graphviz_safe_def_name(self.body().source.def_id()); dot::Id::new(format!("graph_for_def_id_{name}")).unwrap() } @@ -91,19 +98,11 @@ where fn node_label(&self, block: &Self::Node) -> dot::LabelText<'_> { let mut label = Vec::new(); - self.results.replace_with(|results| { - // `Formatter::result` is a `RefCell>` so we can replace - // the value with `None`, move it into the results cursor, move it - // back out, and return it to the refcell wrapped in `Some`. - let mut fmt = BlockFormatter { - cursor: results.take().unwrap().into_results_cursor(self.body), - style: self.style, - bg: Background::Light, - }; + let mut cursor = self.cursor.borrow_mut(); + let mut fmt = + BlockFormatter { cursor: &mut cursor, style: self.style, bg: Background::Light }; + fmt.write_node_label(&mut label, *block).unwrap(); - fmt.write_node_label(&mut label, *block).unwrap(); - Some(fmt.cursor.into_results()) - }); dot::LabelText::html(String::from_utf8(label).unwrap()) } @@ -112,12 +111,12 @@ where } fn edge_label(&self, e: &Self::Edge) -> dot::LabelText<'_> { - let label = &self.body[e.source].terminator().kind.fmt_successor_labels()[e.index]; + let label = &self.body()[e.source].terminator().kind.fmt_successor_labels()[e.index]; dot::LabelText::label(label.clone()) } } -impl<'mir, 'tcx, A> dot::GraphWalk<'mir> for Formatter<'mir, 'tcx, A> +impl<'tcx, A> dot::GraphWalk<'_> for Formatter<'_, 'tcx, A> where A: Analysis<'tcx>, { @@ -125,7 +124,7 @@ where type Edge = CfgEdge; fn nodes(&self) -> dot::Nodes<'_, Self::Node> { - self.body + self.body() .basic_blocks .indices() .filter(|&idx| self.reachable.contains(idx)) @@ -134,10 +133,10 @@ where } fn edges(&self) -> dot::Edges<'_, Self::Edge> { - self.body - .basic_blocks + let body = self.body(); + body.basic_blocks .indices() - .flat_map(|bb| dataflow_successors(self.body, bb)) + .flat_map(|bb| dataflow_successors(body, bb)) .collect::>() .into() } @@ -147,20 +146,20 @@ where } fn target(&self, edge: &Self::Edge) -> Self::Node { - self.body[edge.source].terminator().successors().nth(edge.index).unwrap() + self.body()[edge.source].terminator().successors().nth(edge.index).unwrap() } } -struct BlockFormatter<'mir, 'tcx, A> +struct BlockFormatter<'a, 'mir, 'tcx, A> where A: Analysis<'tcx>, { - cursor: ResultsCursor<'mir, 'tcx, A>, + cursor: &'a mut ResultsCursor<'mir, 'tcx, A>, bg: Background, style: OutputStyle, } -impl<'mir, 'tcx, A> BlockFormatter<'mir, 'tcx, A> +impl<'tcx, A> BlockFormatter<'_, '_, 'tcx, A> where A: Analysis<'tcx>, A::Domain: DebugWithContext, From 6a29ce846bae856955662d926ce044edb480a1c5 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 23 Oct 2024 16:53:22 +1100 Subject: [PATCH 07/14] Return label from `write_node_label`. Instead of mutating an empy label. Because it's conceptually simpler. --- .../rustc_mir_dataflow/src/framework/graphviz.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_mir_dataflow/src/framework/graphviz.rs b/compiler/rustc_mir_dataflow/src/framework/graphviz.rs index 599b92867d7b9..56b0f2fb785b3 100644 --- a/compiler/rustc_mir_dataflow/src/framework/graphviz.rs +++ b/compiler/rustc_mir_dataflow/src/framework/graphviz.rs @@ -97,11 +97,10 @@ where } fn node_label(&self, block: &Self::Node) -> dot::LabelText<'_> { - let mut label = Vec::new(); let mut cursor = self.cursor.borrow_mut(); let mut fmt = BlockFormatter { cursor: &mut cursor, style: self.style, bg: Background::Light }; - fmt.write_node_label(&mut label, *block).unwrap(); + let label = fmt.write_node_label(*block).unwrap(); dot::LabelText::html(String::from_utf8(label).unwrap()) } @@ -172,7 +171,9 @@ where bg } - fn write_node_label(&mut self, w: &mut impl io::Write, block: BasicBlock) -> io::Result<()> { + fn write_node_label(&mut self, block: BasicBlock) -> io::Result> { + use std::io::Write; + // Sample output: // +-+-----------------------------------------------+ // A | bb4 | @@ -199,6 +200,9 @@ where // attributes. Make sure to test the output before trying to remove the redundancy. // Notably, `align` was found to have no effect when applied only to . + let mut v = vec![]; + let w = &mut v; + let table_fmt = concat!( " border=\"1\"", " cellborder=\"1\"", @@ -327,7 +331,9 @@ where _ => {} }; - write!(w, "
") + write!(w, "")?; + + Ok(v) } fn write_block_header_simple( From f2ac970b167d1a9da1f72f25d6cae7ad9de9bac1 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 23 Oct 2024 19:29:20 +1100 Subject: [PATCH 08/14] Remove `Analysis::into_engine`. This is a standard pattern: ``` MyAnalysis.into_engine(tcx, body).iterate_to_fixpoint() ``` In fact, `into_engine` and `iterate_to_fixpoint` are always called in pairs, but sometimes with a builder-style `pass_name` calls between them. But a builder-style interface is overkill here. This has been bugging me a for a while. This commit: - Merges `Engine::new` and `Engine::iterate_to_fixpoint`. This removes the need for `Engine` to have fields, leaving it as a trivial type that the next commit will remove. - Renames `Analysis::into_engine` as `Analysis::iterate_to_fixpoint`, gives it an extra argument for the optional pass name, and makes it call `Engine::iterate_to_fixpoint` instead of `Engine::new`. This turns the pattern from above into this: ``` MyAnalysis.iterate_to_fixpoint(tcx, body, None) ``` which is shorter at every call site, and there's less plumbing required to support it. --- compiler/rustc_borrowck/src/lib.rs | 31 ++++++------ .../src/check_consts/check.rs | 12 ++--- .../src/framework/engine.rs | 47 +++++-------------- .../rustc_mir_dataflow/src/framework/mod.rs | 33 +++++++------ compiler/rustc_mir_dataflow/src/rustc_peek.rs | 15 +++--- compiler/rustc_mir_transform/src/coroutine.rs | 16 ++----- .../src/dataflow_const_prop.rs | 2 +- .../src/dead_store_elimination.rs | 3 +- compiler/rustc_mir_transform/src/dest_prop.rs | 5 +- .../src/elaborate_drops.rs | 8 +--- compiler/rustc_mir_transform/src/lint.rs | 6 +-- compiler/rustc_mir_transform/src/ref_prop.rs | 3 +- .../src/remove_uninit_drops.rs | 4 +- .../clippy_utils/src/mir/possible_borrower.rs | 4 +- 14 files changed, 72 insertions(+), 117 deletions(-) diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index 234061ac7a279..3d4fc05a8cafa 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -193,9 +193,7 @@ fn do_mir_borrowck<'tcx>( .map(|(idx, body)| (idx, MoveData::gather_moves(body, tcx, |_| true))); let mut flow_inits = MaybeInitializedPlaces::new(tcx, body, &move_data) - .into_engine(tcx, body) - .pass_name("borrowck") - .iterate_to_fixpoint() + .iterate_to_fixpoint(tcx, body, Some("borrowck")) .into_results_cursor(body); let locals_are_invalidated_at_exit = tcx.hir().body_owner_kind(def).is_fn_or_closure(); @@ -243,18 +241,21 @@ fn do_mir_borrowck<'tcx>( // usage significantly on some benchmarks. drop(flow_inits); - let flow_borrows = Borrows::new(tcx, body, ®ioncx, &borrow_set) - .into_engine(tcx, body) - .pass_name("borrowck") - .iterate_to_fixpoint(); - let flow_uninits = MaybeUninitializedPlaces::new(tcx, body, &move_data) - .into_engine(tcx, body) - .pass_name("borrowck") - .iterate_to_fixpoint(); - let flow_ever_inits = EverInitializedPlaces::new(body, &move_data) - .into_engine(tcx, body) - .pass_name("borrowck") - .iterate_to_fixpoint(); + let flow_borrows = Borrows::new(tcx, body, ®ioncx, &borrow_set).iterate_to_fixpoint( + tcx, + body, + Some("borrowck"), + ); + let flow_uninits = MaybeUninitializedPlaces::new(tcx, body, &move_data).iterate_to_fixpoint( + tcx, + body, + Some("borrowck"), + ); + let flow_ever_inits = EverInitializedPlaces::new(body, &move_data).iterate_to_fixpoint( + tcx, + body, + Some("borrowck"), + ); let movable_coroutine = // The first argument is the coroutine type passed by value diff --git a/compiler/rustc_const_eval/src/check_consts/check.rs b/compiler/rustc_const_eval/src/check_consts/check.rs index 463a66d4e2e46..4d6f6715b9de3 100644 --- a/compiler/rustc_const_eval/src/check_consts/check.rs +++ b/compiler/rustc_const_eval/src/check_consts/check.rs @@ -63,8 +63,7 @@ impl<'mir, 'tcx> Qualifs<'mir, 'tcx> { let ConstCx { tcx, body, .. } = *ccx; FlowSensitiveAnalysis::new(NeedsDrop, ccx) - .into_engine(tcx, body) - .iterate_to_fixpoint() + .iterate_to_fixpoint(tcx, body, None) .into_results_cursor(body) }); @@ -93,8 +92,7 @@ impl<'mir, 'tcx> Qualifs<'mir, 'tcx> { let ConstCx { tcx, body, .. } = *ccx; FlowSensitiveAnalysis::new(NeedsNonConstDrop, ccx) - .into_engine(tcx, body) - .iterate_to_fixpoint() + .iterate_to_fixpoint(tcx, body, None) .into_results_cursor(body) }); @@ -123,8 +121,7 @@ impl<'mir, 'tcx> Qualifs<'mir, 'tcx> { let ConstCx { tcx, body, .. } = *ccx; FlowSensitiveAnalysis::new(HasMutInterior, ccx) - .into_engine(tcx, body) - .iterate_to_fixpoint() + .iterate_to_fixpoint(tcx, body, None) .into_results_cursor(body) }); @@ -239,8 +236,7 @@ impl<'mir, 'tcx> Checker<'mir, 'tcx> { let always_live_locals = &always_storage_live_locals(&ccx.body); let mut maybe_storage_live = MaybeStorageLive::new(Cow::Borrowed(always_live_locals)) - .into_engine(ccx.tcx, &ccx.body) - .iterate_to_fixpoint() + .iterate_to_fixpoint(ccx.tcx, &ccx.body, None) .into_results_cursor(&ccx.body); // And then check all `Return` in the MIR, and if a local is "maybe live" at a diff --git a/compiler/rustc_mir_dataflow/src/framework/engine.rs b/compiler/rustc_mir_dataflow/src/framework/engine.rs index 890f2b53dcc30..4b4a5b366cbda 100644 --- a/compiler/rustc_mir_dataflow/src/framework/engine.rs +++ b/compiler/rustc_mir_dataflow/src/framework/engine.rs @@ -71,25 +71,21 @@ where } /// A solver for dataflow problems. -pub struct Engine<'mir, 'tcx, A> -where - A: Analysis<'tcx>, -{ - tcx: TyCtxt<'tcx>, - body: &'mir mir::Body<'tcx>, - entry_sets: IndexVec, - pass_name: Option<&'static str>, - analysis: A, -} +pub struct Engine; -impl<'mir, 'tcx, A, D> Engine<'mir, 'tcx, A> -where - A: Analysis<'tcx, Domain = D>, - D: Clone + JoinSemiLattice, -{ +impl Engine { /// Creates a new `Engine` to solve a dataflow problem with an arbitrary transfer /// function. - pub(crate) fn new(tcx: TyCtxt<'tcx>, body: &'mir mir::Body<'tcx>, analysis: A) -> Self { + pub(crate) fn iterate_to_fixpoint<'mir, 'tcx, A>( + tcx: TyCtxt<'tcx>, + body: &'mir mir::Body<'tcx>, + analysis: A, + pass_name: Option<&'static str>, + ) -> Results<'tcx, A> + where + A: Analysis<'tcx>, + A::Domain: DebugWithContext + Clone + JoinSemiLattice, + { let mut entry_sets = IndexVec::from_fn_n(|_| analysis.bottom_value(body), body.basic_blocks.len()); analysis.initialize_start_block(body, &mut entry_sets[mir::START_BLOCK]); @@ -99,25 +95,6 @@ where bug!("`initialize_start_block` is not yet supported for backward dataflow analyses"); } - Engine { analysis, tcx, body, pass_name: None, entry_sets } - } - - /// Adds an identifier to the graphviz output for this particular run of a dataflow analysis. - /// - /// Some analyses are run multiple times in the compilation pipeline. Give them a `pass_name` - /// to differentiate them. Otherwise, only the results for the latest run will be saved. - pub fn pass_name(mut self, name: &'static str) -> Self { - self.pass_name = Some(name); - self - } - - /// Computes the fixpoint for this dataflow problem and returns it. - pub fn iterate_to_fixpoint(self) -> Results<'tcx, A> - where - A::Domain: DebugWithContext, - { - let Engine { analysis, body, mut entry_sets, tcx, pass_name } = self; - let mut dirty_queue: WorkQueue = WorkQueue::with_none(body.basic_blocks.len()); if A::Direction::IS_FORWARD { diff --git a/compiler/rustc_mir_dataflow/src/framework/mod.rs b/compiler/rustc_mir_dataflow/src/framework/mod.rs index 90f9f88352537..81d0609f22b20 100644 --- a/compiler/rustc_mir_dataflow/src/framework/mod.rs +++ b/compiler/rustc_mir_dataflow/src/framework/mod.rs @@ -7,18 +7,17 @@ //! //! The `impls` module contains several examples of dataflow analyses. //! -//! Create an `Engine` for your analysis using the `into_engine` method on the `Analysis` trait, -//! then call `iterate_to_fixpoint`. From there, you can use a `ResultsCursor` to inspect the -//! fixpoint solution to your dataflow problem, or implement the `ResultsVisitor` interface and use -//! `visit_results`. The following example uses the `ResultsCursor` approach. +//! Then call `iterate_to_fixpoint` on your type that impls `Analysis` to get a `Results`. From +//! there, you can use a `ResultsCursor` to inspect the fixpoint solution to your dataflow problem, +//! or implement the `ResultsVisitor` interface and use `visit_results`. The following example uses +//! the `ResultsCursor` approach. //! //! ```ignore (cross-crate-imports) -//! use rustc_const_eval::dataflow::Analysis; // Makes `into_engine` available. +//! use rustc_const_eval::dataflow::Analysis; // Makes `iterate_to_fixpoint` available. //! //! fn do_my_analysis(tcx: TyCtxt<'tcx>, body: &mir::Body<'tcx>) { //! let analysis = MyAnalysis::new() -//! .into_engine(tcx, body) -//! .iterate_to_fixpoint() +//! .iterate_to_fixpoint(tcx, body, None) //! .into_results_cursor(body); //! //! // Print the dataflow state *after* each statement in the start block. @@ -39,6 +38,8 @@ use rustc_index::bit_set::{BitSet, ChunkedBitSet, HybridBitSet}; use rustc_middle::mir::{self, BasicBlock, CallReturnPlaces, Location, TerminatorEdges}; use rustc_middle::ty::TyCtxt; +use super::fmt::DebugWithContext; + mod cursor; mod direction; mod engine; @@ -223,26 +224,30 @@ pub trait Analysis<'tcx> { /* Extension methods */ - /// Creates an `Engine` to find the fixpoint for this dataflow problem. + /// Finds the fixpoint for this dataflow problem. /// /// You shouldn't need to override this. Its purpose is to enable method chaining like so: /// /// ```ignore (cross-crate-imports) /// let results = MyAnalysis::new(tcx, body) - /// .into_engine(tcx, body, def_id) - /// .iterate_to_fixpoint() + /// .iterate_to_fixpoint(tcx, body, None) /// .into_results_cursor(body); /// ``` - #[inline] - fn into_engine<'mir>( + /// You can optionally add an identifier to the graphviz output for this particular run of a + /// dataflow analysis. Some analyses are run multiple times in the compilation pipeline. + /// Without a `pass_name` to differentiates them, only the results for the latest run will be + /// saved. + fn iterate_to_fixpoint<'mir>( self, tcx: TyCtxt<'tcx>, body: &'mir mir::Body<'tcx>, - ) -> Engine<'mir, 'tcx, Self> + pass_name: Option<&'static str>, + ) -> Results<'tcx, Self> where Self: Sized, + Self::Domain: DebugWithContext, { - Engine::new(tcx, body, self) + Engine::iterate_to_fixpoint(tcx, body, self, pass_name) } } diff --git a/compiler/rustc_mir_dataflow/src/rustc_peek.rs b/compiler/rustc_mir_dataflow/src/rustc_peek.rs index 5727517bd612a..99d0ccde1052c 100644 --- a/compiler/rustc_mir_dataflow/src/rustc_peek.rs +++ b/compiler/rustc_mir_dataflow/src/rustc_peek.rs @@ -43,31 +43,28 @@ pub fn sanity_check<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>) { let move_data = MoveData::gather_moves(body, tcx, |_| true); if has_rustc_mir_with(tcx, def_id, sym::rustc_peek_maybe_init).is_some() { - let flow_inits = MaybeInitializedPlaces::new(tcx, body, &move_data) - .into_engine(tcx, body) - .iterate_to_fixpoint(); + let flow_inits = + MaybeInitializedPlaces::new(tcx, body, &move_data).iterate_to_fixpoint(tcx, body, None); sanity_check_via_rustc_peek(tcx, flow_inits.into_results_cursor(body)); } if has_rustc_mir_with(tcx, def_id, sym::rustc_peek_maybe_uninit).is_some() { let flow_uninits = MaybeUninitializedPlaces::new(tcx, body, &move_data) - .into_engine(tcx, body) - .iterate_to_fixpoint(); + .iterate_to_fixpoint(tcx, body, None); sanity_check_via_rustc_peek(tcx, flow_uninits.into_results_cursor(body)); } if has_rustc_mir_with(tcx, def_id, sym::rustc_peek_definite_init).is_some() { - let flow_def_inits = DefinitelyInitializedPlaces::new(body, &move_data) - .into_engine(tcx, body) - .iterate_to_fixpoint(); + let flow_def_inits = + DefinitelyInitializedPlaces::new(body, &move_data).iterate_to_fixpoint(tcx, body, None); sanity_check_via_rustc_peek(tcx, flow_def_inits.into_results_cursor(body)); } if has_rustc_mir_with(tcx, def_id, sym::rustc_peek_liveness).is_some() { - let flow_liveness = MaybeLiveLocals.into_engine(tcx, body).iterate_to_fixpoint(); + let flow_liveness = MaybeLiveLocals.iterate_to_fixpoint(tcx, body, None); sanity_check_via_rustc_peek(tcx, flow_liveness.into_results_cursor(body)); } diff --git a/compiler/rustc_mir_transform/src/coroutine.rs b/compiler/rustc_mir_transform/src/coroutine.rs index 43ebbf2f4d8dd..7f92b3206b4fb 100644 --- a/compiler/rustc_mir_transform/src/coroutine.rs +++ b/compiler/rustc_mir_transform/src/coroutine.rs @@ -666,30 +666,24 @@ fn locals_live_across_suspend_points<'tcx>( // Calculate when MIR locals have live storage. This gives us an upper bound of their // lifetimes. let mut storage_live = MaybeStorageLive::new(std::borrow::Cow::Borrowed(always_live_locals)) - .into_engine(tcx, body) - .iterate_to_fixpoint() + .iterate_to_fixpoint(tcx, body, None) .into_results_cursor(body); // Calculate the MIR locals which have been previously // borrowed (even if they are still active). let borrowed_locals_results = - MaybeBorrowedLocals.into_engine(tcx, body).pass_name("coroutine").iterate_to_fixpoint(); - + MaybeBorrowedLocals.iterate_to_fixpoint(tcx, body, Some("coroutine")); let mut borrowed_locals_cursor = borrowed_locals_results.clone().into_results_cursor(body); // Calculate the MIR locals that we actually need to keep storage around // for. let mut requires_storage_cursor = MaybeRequiresStorage::new(body, borrowed_locals_results) - .into_engine(tcx, body) - .iterate_to_fixpoint() + .iterate_to_fixpoint(tcx, body, None) .into_results_cursor(body); // Calculate the liveness of MIR locals ignoring borrows. - let mut liveness = MaybeLiveLocals - .into_engine(tcx, body) - .pass_name("coroutine") - .iterate_to_fixpoint() - .into_results_cursor(body); + let mut liveness = + MaybeLiveLocals.iterate_to_fixpoint(tcx, body, Some("coroutine")).into_results_cursor(body); let mut storage_liveness_map = IndexVec::from_elem(None, &body.basic_blocks); let mut live_locals_at_suspension_points = Vec::new(); diff --git a/compiler/rustc_mir_transform/src/dataflow_const_prop.rs b/compiler/rustc_mir_transform/src/dataflow_const_prop.rs index 2636f1c54767c..01828e5dbc1a5 100644 --- a/compiler/rustc_mir_transform/src/dataflow_const_prop.rs +++ b/compiler/rustc_mir_transform/src/dataflow_const_prop.rs @@ -61,7 +61,7 @@ impl<'tcx> crate::MirPass<'tcx> for DataflowConstProp { // Perform the actual dataflow analysis. let analysis = ConstAnalysis::new(tcx, body, map); let mut results = debug_span!("analyze") - .in_scope(|| analysis.wrap().into_engine(tcx, body).iterate_to_fixpoint()); + .in_scope(|| analysis.wrap().iterate_to_fixpoint(tcx, body, None)); // Collect results and patch the body afterwards. let mut visitor = Collector::new(tcx, &body.local_decls); diff --git a/compiler/rustc_mir_transform/src/dead_store_elimination.rs b/compiler/rustc_mir_transform/src/dead_store_elimination.rs index edffe6ce78f67..2898f82e25c38 100644 --- a/compiler/rustc_mir_transform/src/dead_store_elimination.rs +++ b/compiler/rustc_mir_transform/src/dead_store_elimination.rs @@ -37,8 +37,7 @@ fn eliminate<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { always_live.union(&borrowed_locals); let mut live = MaybeTransitiveLiveLocals::new(&always_live) - .into_engine(tcx, body) - .iterate_to_fixpoint() + .iterate_to_fixpoint(tcx, body, None) .into_results_cursor(body); // For blocks with a call terminator, if an argument copy can be turned into a move, diff --git a/compiler/rustc_mir_transform/src/dest_prop.rs b/compiler/rustc_mir_transform/src/dest_prop.rs index ad83c0295baec..beeab0d4a666b 100644 --- a/compiler/rustc_mir_transform/src/dest_prop.rs +++ b/compiler/rustc_mir_transform/src/dest_prop.rs @@ -169,10 +169,7 @@ impl<'tcx> crate::MirPass<'tcx> for DestinationPropagation { let borrowed = rustc_mir_dataflow::impls::borrowed_locals(body); - let live = MaybeLiveLocals - .into_engine(tcx, body) - .pass_name("MaybeLiveLocals-DestinationPropagation") - .iterate_to_fixpoint(); + let live = MaybeLiveLocals.iterate_to_fixpoint(tcx, body, Some("MaybeLiveLocals-DestProp")); let points = DenseLocationMap::new(body); let mut live = save_as_intervals(&points, body, live); diff --git a/compiler/rustc_mir_transform/src/elaborate_drops.rs b/compiler/rustc_mir_transform/src/elaborate_drops.rs index 30e1ac05e039c..58e1db194380f 100644 --- a/compiler/rustc_mir_transform/src/elaborate_drops.rs +++ b/compiler/rustc_mir_transform/src/elaborate_drops.rs @@ -64,18 +64,14 @@ impl<'tcx> crate::MirPass<'tcx> for ElaborateDrops { let mut inits = MaybeInitializedPlaces::new(tcx, body, &env.move_data) .skipping_unreachable_unwind() - .into_engine(tcx, body) - .pass_name("elaborate_drops") - .iterate_to_fixpoint() + .iterate_to_fixpoint(tcx, body, Some("elaborate_drops")) .into_results_cursor(body); let dead_unwinds = compute_dead_unwinds(body, &mut inits); let uninits = MaybeUninitializedPlaces::new(tcx, body, &env.move_data) .mark_inactive_variants_as_uninit() .skipping_unreachable_unwind(dead_unwinds) - .into_engine(tcx, body) - .pass_name("elaborate_drops") - .iterate_to_fixpoint() + .iterate_to_fixpoint(tcx, body, Some("elaborate_drops")) .into_results_cursor(body); let drop_flags = IndexVec::from_elem(None, &env.move_data.move_paths); diff --git a/compiler/rustc_mir_transform/src/lint.rs b/compiler/rustc_mir_transform/src/lint.rs index 23733994a8b4a..d8ff1cfc90b58 100644 --- a/compiler/rustc_mir_transform/src/lint.rs +++ b/compiler/rustc_mir_transform/src/lint.rs @@ -17,13 +17,11 @@ pub(super) fn lint_body<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, when: String let always_live_locals = &always_storage_live_locals(body); let maybe_storage_live = MaybeStorageLive::new(Cow::Borrowed(always_live_locals)) - .into_engine(tcx, body) - .iterate_to_fixpoint() + .iterate_to_fixpoint(tcx, body, None) .into_results_cursor(body); let maybe_storage_dead = MaybeStorageDead::new(Cow::Borrowed(always_live_locals)) - .into_engine(tcx, body) - .iterate_to_fixpoint() + .iterate_to_fixpoint(tcx, body, None) .into_results_cursor(body); let mut lint = Lint { diff --git a/compiler/rustc_mir_transform/src/ref_prop.rs b/compiler/rustc_mir_transform/src/ref_prop.rs index 53e53d9d5ba8b..b11b503e8d430 100644 --- a/compiler/rustc_mir_transform/src/ref_prop.rs +++ b/compiler/rustc_mir_transform/src/ref_prop.rs @@ -126,8 +126,7 @@ fn compute_replacement<'tcx>( // Compute `MaybeStorageDead` dataflow to check that we only replace when the pointee is // definitely live. let mut maybe_dead = MaybeStorageDead::new(Cow::Owned(always_live_locals)) - .into_engine(tcx, body) - .iterate_to_fixpoint() + .iterate_to_fixpoint(tcx, body, None) .into_results_cursor(body); // Map for each local to the pointee. diff --git a/compiler/rustc_mir_transform/src/remove_uninit_drops.rs b/compiler/rustc_mir_transform/src/remove_uninit_drops.rs index 09969a4c7cc7f..55dd96100b0a6 100644 --- a/compiler/rustc_mir_transform/src/remove_uninit_drops.rs +++ b/compiler/rustc_mir_transform/src/remove_uninit_drops.rs @@ -22,9 +22,7 @@ impl<'tcx> crate::MirPass<'tcx> for RemoveUninitDrops { let move_data = MoveData::gather_moves(body, tcx, |ty| ty.needs_drop(tcx, param_env)); let mut maybe_inits = MaybeInitializedPlaces::new(tcx, body, &move_data) - .into_engine(tcx, body) - .pass_name("remove_uninit_drops") - .iterate_to_fixpoint() + .iterate_to_fixpoint(tcx, body, Some("remove_uninit_drops")) .into_results_cursor(body); let mut to_remove = vec![]; diff --git a/src/tools/clippy/clippy_utils/src/mir/possible_borrower.rs b/src/tools/clippy/clippy_utils/src/mir/possible_borrower.rs index a00196c4b511d..6b3078f52aff5 100644 --- a/src/tools/clippy/clippy_utils/src/mir/possible_borrower.rs +++ b/src/tools/clippy/clippy_utils/src/mir/possible_borrower.rs @@ -185,9 +185,7 @@ impl<'b, 'tcx> PossibleBorrowerMap<'b, 'tcx> { vis.into_map(cx) }; let maybe_storage_live_result = MaybeStorageLive::new(Cow::Owned(BitSet::new_empty(mir.local_decls.len()))) - .into_engine(cx.tcx, mir) - .pass_name("redundant_clone") - .iterate_to_fixpoint() + .iterate_to_fixpoint(cx.tcx, mir, Some("redundant_clone")) .into_results_cursor(mir); let mut vis = PossibleBorrowerVisitor::new(cx, mir, possible_origin); vis.visit_body(mir); From 41b75a32c204c63b9039e0bab77ab9b3cf61b3f6 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 23 Oct 2024 20:36:48 +1100 Subject: [PATCH 09/14] Remove `Engine`. It's no longer needed. `Engine::iterate_to_fixpoint` can be inlined into `Analysis::iterate_to_fixpoint` and removed. The commit also renames `engine.rs` as `results.rs`. --- .../rustc_mir_dataflow/src/framework/mod.rs | 78 +++++++++++++-- .../src/framework/{engine.rs => results.rs} | 98 +------------------ compiler/rustc_mir_dataflow/src/lib.rs | 6 +- 3 files changed, 80 insertions(+), 102 deletions(-) rename compiler/rustc_mir_dataflow/src/framework/{engine.rs => results.rs} (66%) diff --git a/compiler/rustc_mir_dataflow/src/framework/mod.rs b/compiler/rustc_mir_dataflow/src/framework/mod.rs index 81d0609f22b20..33527e2cb6f1f 100644 --- a/compiler/rustc_mir_dataflow/src/framework/mod.rs +++ b/compiler/rustc_mir_dataflow/src/framework/mod.rs @@ -33,25 +33,29 @@ use std::cmp::Ordering; -use rustc_index::Idx; +use rustc_data_structures::work_queue::WorkQueue; use rustc_index::bit_set::{BitSet, ChunkedBitSet, HybridBitSet}; -use rustc_middle::mir::{self, BasicBlock, CallReturnPlaces, Location, TerminatorEdges}; +use rustc_index::{Idx, IndexVec}; +use rustc_middle::bug; +use rustc_middle::mir::{self, BasicBlock, CallReturnPlaces, Location, TerminatorEdges, traversal}; use rustc_middle::ty::TyCtxt; +use tracing::error; +use self::results::write_graphviz_results; use super::fmt::DebugWithContext; mod cursor; mod direction; -mod engine; pub mod fmt; pub mod graphviz; pub mod lattice; +mod results; mod visitor; pub use self::cursor::ResultsCursor; pub use self::direction::{Backward, Direction, Forward}; -pub use self::engine::{Engine, Results}; pub use self::lattice::{JoinSemiLattice, MaybeReachable}; +pub use self::results::Results; pub use self::visitor::{ResultsVisitable, ResultsVisitor, visit_results}; /// Analysis domains are all bitsets of various kinds. This trait holds @@ -238,7 +242,7 @@ pub trait Analysis<'tcx> { /// Without a `pass_name` to differentiates them, only the results for the latest run will be /// saved. fn iterate_to_fixpoint<'mir>( - self, + mut self, tcx: TyCtxt<'tcx>, body: &'mir mir::Body<'tcx>, pass_name: Option<&'static str>, @@ -247,7 +251,69 @@ pub trait Analysis<'tcx> { Self: Sized, Self::Domain: DebugWithContext, { - Engine::iterate_to_fixpoint(tcx, body, self, pass_name) + let mut entry_sets = + IndexVec::from_fn_n(|_| self.bottom_value(body), body.basic_blocks.len()); + self.initialize_start_block(body, &mut entry_sets[mir::START_BLOCK]); + + if Self::Direction::IS_BACKWARD && entry_sets[mir::START_BLOCK] != self.bottom_value(body) { + bug!("`initialize_start_block` is not yet supported for backward dataflow analyses"); + } + + let mut dirty_queue: WorkQueue = WorkQueue::with_none(body.basic_blocks.len()); + + if Self::Direction::IS_FORWARD { + for (bb, _) in traversal::reverse_postorder(body) { + dirty_queue.insert(bb); + } + } else { + // Reverse post-order on the reverse CFG may generate a better iteration order for + // backward dataflow analyses, but probably not enough to matter. + for (bb, _) in traversal::postorder(body) { + dirty_queue.insert(bb); + } + } + + // `state` is not actually used between iterations; + // this is just an optimization to avoid reallocating + // every iteration. + let mut state = self.bottom_value(body); + while let Some(bb) = dirty_queue.pop() { + let bb_data = &body[bb]; + + // Set the state to the entry state of the block. + // This is equivalent to `state = entry_sets[bb].clone()`, + // but it saves an allocation, thus improving compile times. + state.clone_from(&entry_sets[bb]); + + // Apply the block transfer function, using the cached one if it exists. + let edges = Self::Direction::apply_effects_in_block(&mut self, &mut state, bb, bb_data); + + Self::Direction::join_state_into_successors_of( + &mut self, + body, + &mut state, + bb, + edges, + |target: BasicBlock, state: &Self::Domain| { + let set_changed = entry_sets[target].join(state); + if set_changed { + dirty_queue.insert(target); + } + }, + ); + } + + let results = Results { analysis: self, entry_sets }; + + if tcx.sess.opts.unstable_opts.dump_mir_dataflow { + let (res, results) = write_graphviz_results(tcx, body, results, pass_name); + if let Err(e) = res { + error!("Failed to write graphviz dataflow results: {}", e); + } + results + } else { + results + } } } diff --git a/compiler/rustc_mir_dataflow/src/framework/engine.rs b/compiler/rustc_mir_dataflow/src/framework/results.rs similarity index 66% rename from compiler/rustc_mir_dataflow/src/framework/engine.rs rename to compiler/rustc_mir_dataflow/src/framework/results.rs index 4b4a5b366cbda..edfaba1e5a031 100644 --- a/compiler/rustc_mir_dataflow/src/framework/engine.rs +++ b/compiler/rustc_mir_dataflow/src/framework/results.rs @@ -1,23 +1,19 @@ -//! A solver for dataflow problems. +//! Results of dataflow problems. use std::ffi::OsString; use std::path::PathBuf; -use rustc_data_structures::work_queue::WorkQueue; use rustc_hir::def_id::DefId; use rustc_index::IndexVec; -use rustc_middle::bug; use rustc_middle::mir::{self, BasicBlock, create_dump_file, dump_enabled, traversal}; use rustc_middle::ty::TyCtxt; use rustc_middle::ty::print::with_no_trimmed_paths; use rustc_span::symbol::{Symbol, sym}; -use tracing::{debug, error}; +use tracing::debug; use {rustc_ast as ast, rustc_graphviz as dot}; use super::fmt::DebugWithContext; -use super::{ - Analysis, Direction, JoinSemiLattice, ResultsCursor, ResultsVisitor, graphviz, visit_results, -}; +use super::{Analysis, ResultsCursor, ResultsVisitor, graphviz, visit_results}; use crate::errors::{ DuplicateValuesFor, PathMustEndInFilename, RequiresAnArgument, UnknownFormatter, }; @@ -65,101 +61,17 @@ where body: &'mir mir::Body<'tcx>, vis: &mut impl ResultsVisitor<'mir, 'tcx, Self, Domain = A::Domain>, ) { - let blocks = mir::traversal::reachable(body); + let blocks = traversal::reachable(body); visit_results(body, blocks.map(|(bb, _)| bb), self, vis) } } -/// A solver for dataflow problems. -pub struct Engine; - -impl Engine { - /// Creates a new `Engine` to solve a dataflow problem with an arbitrary transfer - /// function. - pub(crate) fn iterate_to_fixpoint<'mir, 'tcx, A>( - tcx: TyCtxt<'tcx>, - body: &'mir mir::Body<'tcx>, - analysis: A, - pass_name: Option<&'static str>, - ) -> Results<'tcx, A> - where - A: Analysis<'tcx>, - A::Domain: DebugWithContext + Clone + JoinSemiLattice, - { - let mut entry_sets = - IndexVec::from_fn_n(|_| analysis.bottom_value(body), body.basic_blocks.len()); - analysis.initialize_start_block(body, &mut entry_sets[mir::START_BLOCK]); - - if A::Direction::IS_BACKWARD && entry_sets[mir::START_BLOCK] != analysis.bottom_value(body) - { - bug!("`initialize_start_block` is not yet supported for backward dataflow analyses"); - } - - let mut dirty_queue: WorkQueue = WorkQueue::with_none(body.basic_blocks.len()); - - if A::Direction::IS_FORWARD { - for (bb, _) in traversal::reverse_postorder(body) { - dirty_queue.insert(bb); - } - } else { - // Reverse post-order on the reverse CFG may generate a better iteration order for - // backward dataflow analyses, but probably not enough to matter. - for (bb, _) in traversal::postorder(body) { - dirty_queue.insert(bb); - } - } - - // `state` is not actually used between iterations; - // this is just an optimization to avoid reallocating - // every iteration. - let mut state = analysis.bottom_value(body); - while let Some(bb) = dirty_queue.pop() { - let bb_data = &body[bb]; - - // Set the state to the entry state of the block. - // This is equivalent to `state = entry_sets[bb].clone()`, - // but it saves an allocation, thus improving compile times. - state.clone_from(&entry_sets[bb]); - - // Apply the block transfer function, using the cached one if it exists. - let edges = - A::Direction::apply_effects_in_block(&analysis, &mut state, bb, bb_data); - - A::Direction::join_state_into_successors_of( - &analysis, - body, - &mut state, - bb, - edges, - |target: BasicBlock, state: &A::Domain| { - let set_changed = entry_sets[target].join(state); - if set_changed { - dirty_queue.insert(target); - } - }, - ); - } - - let results = Results { analysis, entry_sets }; - - if tcx.sess.opts.unstable_opts.dump_mir_dataflow { - let (res, results) = write_graphviz_results(tcx, body, results, pass_name); - if let Err(e) = res { - error!("Failed to write graphviz dataflow results: {}", e); - } - results - } else { - results - } - } -} - // Graphviz /// Writes a DOT file containing the results of a dataflow analysis if the user requested it via /// `rustc_mir` attributes and `-Z dump-mir-dataflow`. The `Result` in and the `Results` out are /// the same. -fn write_graphviz_results<'tcx, A>( +pub(super) fn write_graphviz_results<'tcx, A>( tcx: TyCtxt<'tcx>, body: &mir::Body<'tcx>, results: Results<'tcx, A>, diff --git a/compiler/rustc_mir_dataflow/src/lib.rs b/compiler/rustc_mir_dataflow/src/lib.rs index b284f0308f979..b404e3bfb72c3 100644 --- a/compiler/rustc_mir_dataflow/src/lib.rs +++ b/compiler/rustc_mir_dataflow/src/lib.rs @@ -18,9 +18,9 @@ pub use self::drop_flag_effects::{ move_path_children_matching, on_all_children_bits, on_lookup_result_bits, }; pub use self::framework::{ - Analysis, Backward, Direction, Engine, Forward, GenKill, JoinSemiLattice, MaybeReachable, - Results, ResultsCursor, ResultsVisitable, ResultsVisitor, SwitchIntEdgeEffects, fmt, graphviz, - lattice, visit_results, + Analysis, Backward, Direction, Forward, GenKill, JoinSemiLattice, MaybeReachable, Results, + ResultsCursor, ResultsVisitable, ResultsVisitor, SwitchIntEdgeEffects, fmt, graphviz, lattice, + visit_results, }; use self::move_paths::MoveData; From e8c947e8c0e467bbe02b4b12827e6b365df2ed09 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 24 Oct 2024 09:08:30 +1100 Subject: [PATCH 10/14] Make it possible for `ResultsCursor` to borrow a `Results`. `ResultsCursor` currently owns its `Results`. But sometimes the `Results` is needed again afterwards. So there is `ResultsCursor::into_results` for extracting the `Results`, which leads to some awkwardness. This commit adds `ResultsHandle`, a `Cow`-like type that can either borrow or own a a `Results`. `ResultsCursor` now uses it. This is good because some `ResultsCursor`s really want to own their `Results`, while others just want to borrow it. We end with with a few more lines of code, but get some nice cleanups. - `ResultsCursor::into_results` is removed. - `Formatter::into_results` is removed. - `write_graphviz_results` now just borrows a `Results`, instead of the awkward "take ownership of a `Results` and then return it unchanged" pattern. - `MaybeRequiresStorage` can borrow the `MaybeBorrowedLocals` results, avoiding two `clone` calls: one in `check_for_move` and one in `locals_live_across_suspend_points`. - `Results` no longer needs to derive `Clone`. This reinstates the cursor flexibility that was lost in #118230 -- which removed the old `ResultsRefCursor` and `ResultsCloneCursor` types -- but in a *much* simpler way. Hooray! --- .../src/framework/cursor.rs | 35 +++++++++++++++---- .../src/framework/graphviz.rs | 8 ++--- .../rustc_mir_dataflow/src/framework/mod.rs | 7 ++-- .../src/framework/results.rs | 30 ++++++++++------ .../src/impls/storage_liveness.rs | 6 ++-- compiler/rustc_mir_transform/src/coroutine.rs | 13 ++++--- 6 files changed, 62 insertions(+), 37 deletions(-) diff --git a/compiler/rustc_mir_dataflow/src/framework/cursor.rs b/compiler/rustc_mir_dataflow/src/framework/cursor.rs index 2d091885dead9..311fee0be6d5b 100644 --- a/compiler/rustc_mir_dataflow/src/framework/cursor.rs +++ b/compiler/rustc_mir_dataflow/src/framework/cursor.rs @@ -1,6 +1,7 @@ //! Random access inspection of the results of a dataflow analysis. use std::cmp::Ordering; +use std::ops::Deref; #[cfg(debug_assertions)] use rustc_index::bit_set::BitSet; @@ -8,6 +9,31 @@ use rustc_middle::mir::{self, BasicBlock, Location}; use super::{Analysis, Direction, Effect, EffectIndex, Results}; +/// Some `ResultsCursor`s want to own a `Results`, and some want to borrow a `Results`. This type +/// allows either. We could use `Cow` but that would require `Results` and `A` to impl `Clone`. So +/// this is a greatly cut-down alternative to `Cow`. +pub enum ResultsHandle<'a, 'tcx, A> +where + A: Analysis<'tcx>, +{ + Borrowed(&'a Results<'tcx, A>), + Owned(Results<'tcx, A>), +} + +impl<'tcx, A> Deref for ResultsHandle<'_, 'tcx, A> +where + A: Analysis<'tcx>, +{ + type Target = Results<'tcx, A>; + + fn deref(&self) -> &Results<'tcx, A> { + match self { + ResultsHandle::Borrowed(borrowed) => borrowed, + ResultsHandle::Owned(owned) => owned, + } + } +} + /// Allows random access inspection of the results of a dataflow analysis. /// /// This cursor only has linear performance within a basic block when its statements are visited in @@ -19,7 +45,7 @@ where A: Analysis<'tcx>, { body: &'mir mir::Body<'tcx>, - results: Results<'tcx, A>, + results: ResultsHandle<'mir, 'tcx, A>, state: A::Domain, pos: CursorPosition, @@ -47,13 +73,8 @@ where self.body } - /// Unwraps this cursor, returning the underlying `Results`. - pub fn into_results(self) -> Results<'tcx, A> { - self.results - } - /// Returns a new cursor that can inspect `results`. - pub fn new(body: &'mir mir::Body<'tcx>, results: Results<'tcx, A>) -> Self { + pub fn new(body: &'mir mir::Body<'tcx>, results: ResultsHandle<'mir, 'tcx, A>) -> Self { let bottom_value = results.analysis.bottom_value(body); ResultsCursor { body, diff --git a/compiler/rustc_mir_dataflow/src/framework/graphviz.rs b/compiler/rustc_mir_dataflow/src/framework/graphviz.rs index 56b0f2fb785b3..a4de9dab27214 100644 --- a/compiler/rustc_mir_dataflow/src/framework/graphviz.rs +++ b/compiler/rustc_mir_dataflow/src/framework/graphviz.rs @@ -47,20 +47,16 @@ where { pub(crate) fn new( body: &'mir Body<'tcx>, - results: Results<'tcx, A>, + results: &'mir Results<'tcx, A>, style: OutputStyle, ) -> Self { let reachable = mir::traversal::reachable_as_bitset(body); - Formatter { cursor: results.into_results_cursor(body).into(), style, reachable } + Formatter { cursor: results.as_results_cursor(body).into(), style, reachable } } fn body(&self) -> &'mir Body<'tcx> { self.cursor.borrow().body() } - - pub(crate) fn into_results(self) -> Results<'tcx, A> { - self.cursor.into_inner().into_results() - } } /// A pair of a basic block and an index into that basic blocks `successors`. diff --git a/compiler/rustc_mir_dataflow/src/framework/mod.rs b/compiler/rustc_mir_dataflow/src/framework/mod.rs index 33527e2cb6f1f..e24eedde73188 100644 --- a/compiler/rustc_mir_dataflow/src/framework/mod.rs +++ b/compiler/rustc_mir_dataflow/src/framework/mod.rs @@ -306,14 +306,13 @@ pub trait Analysis<'tcx> { let results = Results { analysis: self, entry_sets }; if tcx.sess.opts.unstable_opts.dump_mir_dataflow { - let (res, results) = write_graphviz_results(tcx, body, results, pass_name); + let res = write_graphviz_results(tcx, body, &results, pass_name); if let Err(e) = res { error!("Failed to write graphviz dataflow results: {}", e); } - results - } else { - results } + + results } } diff --git a/compiler/rustc_mir_dataflow/src/framework/results.rs b/compiler/rustc_mir_dataflow/src/framework/results.rs index edfaba1e5a031..82695daceeb01 100644 --- a/compiler/rustc_mir_dataflow/src/framework/results.rs +++ b/compiler/rustc_mir_dataflow/src/framework/results.rs @@ -17,11 +17,11 @@ use super::{Analysis, ResultsCursor, ResultsVisitor, graphviz, visit_results}; use crate::errors::{ DuplicateValuesFor, PathMustEndInFilename, RequiresAnArgument, UnknownFormatter, }; +use crate::framework::cursor::ResultsHandle; type EntrySets<'tcx, A> = IndexVec>::Domain>; /// A dataflow analysis that has converged to fixpoint. -#[derive(Clone)] pub struct Results<'tcx, A> where A: Analysis<'tcx>, @@ -34,12 +34,22 @@ impl<'tcx, A> Results<'tcx, A> where A: Analysis<'tcx>, { - /// Creates a `ResultsCursor` that can inspect these `Results`. + /// Creates a `ResultsCursor` that can inspect these `Results`. Only borrows the `Results`, + /// which is appropriate when the `Results` is used outside the cursor. + pub fn as_results_cursor<'mir>( + &'mir self, + body: &'mir mir::Body<'tcx>, + ) -> ResultsCursor<'mir, 'tcx, A> { + ResultsCursor::new(body, ResultsHandle::Borrowed(self)) + } + + /// Creates a `ResultsCursor` that can inspect these `Results`. Takes ownership of the + /// `Results`, which is good when the `Results` is only used by the cursor. pub fn into_results_cursor<'mir>( self, body: &'mir mir::Body<'tcx>, ) -> ResultsCursor<'mir, 'tcx, A> { - ResultsCursor::new(body, self) + ResultsCursor::new(body, ResultsHandle::Owned(self)) } /// Gets the dataflow state for the given block. @@ -74,9 +84,9 @@ where pub(super) fn write_graphviz_results<'tcx, A>( tcx: TyCtxt<'tcx>, body: &mir::Body<'tcx>, - results: Results<'tcx, A>, + results: &Results<'tcx, A>, pass_name: Option<&'static str>, -) -> (std::io::Result<()>, Results<'tcx, A>) +) -> std::io::Result<()> where A: Analysis<'tcx>, A::Domain: DebugWithContext, @@ -87,7 +97,7 @@ where let def_id = body.source.def_id(); let Ok(attrs) = RustcMirAttrs::parse(tcx, def_id) else { // Invalid `rustc_mir` attrs are reported in `RustcMirAttrs::parse` - return (Ok(()), results); + return Ok(()); }; let file = try { @@ -104,12 +114,12 @@ where create_dump_file(tcx, "dot", false, A::NAME, &pass_name.unwrap_or("-----"), body)? } - _ => return (Ok(()), results), + _ => return Ok(()), } }; let mut file = match file { Ok(f) => f, - Err(e) => return (Err(e), results), + Err(e) => return Err(e), }; let style = match attrs.formatter { @@ -119,7 +129,7 @@ where let mut buf = Vec::new(); - let graphviz = graphviz::Formatter::new(body, results, style); + let graphviz = graphviz::Formatter::new(body, &results, style); let mut render_opts = vec![dot::RenderOption::Fontname(tcx.sess.opts.unstable_opts.graphviz_font.clone())]; if tcx.sess.opts.unstable_opts.graphviz_dark_mode { @@ -132,7 +142,7 @@ where file.write_all(&buf)?; }; - (lhs, graphviz.into_results()) + lhs } #[derive(Default)] diff --git a/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs b/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs index 5afd174060d7f..0a575a78e63db 100644 --- a/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs +++ b/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs @@ -100,13 +100,13 @@ impl<'a, 'tcx> Analysis<'tcx> for MaybeStorageDead<'a> { /// given location; i.e. whether its storage can go away without being observed. pub struct MaybeRequiresStorage<'mir, 'tcx> { body: &'mir Body<'tcx>, - borrowed_locals: Results<'tcx, MaybeBorrowedLocals>, + borrowed_locals: &'mir Results<'tcx, MaybeBorrowedLocals>, } impl<'mir, 'tcx> MaybeRequiresStorage<'mir, 'tcx> { pub fn new( body: &'mir Body<'tcx>, - borrowed_locals: Results<'tcx, MaybeBorrowedLocals>, + borrowed_locals: &'mir Results<'tcx, MaybeBorrowedLocals>, ) -> Self { MaybeRequiresStorage { body, borrowed_locals } } @@ -278,7 +278,7 @@ impl<'tcx> Analysis<'tcx> for MaybeRequiresStorage<'_, 'tcx> { impl<'tcx> MaybeRequiresStorage<'_, 'tcx> { /// Kill locals that are fully moved and have not been borrowed. fn check_for_move(&self, trans: &mut >::Domain, loc: Location) { - let borrowed_locals = self.borrowed_locals.clone().into_results_cursor(self.body); + let borrowed_locals = self.borrowed_locals.as_results_cursor(self.body); let mut visitor = MoveVisitor { trans, borrowed_locals }; visitor.visit_location(self.body, loc); } diff --git a/compiler/rustc_mir_transform/src/coroutine.rs b/compiler/rustc_mir_transform/src/coroutine.rs index 7f92b3206b4fb..0a09cf2774025 100644 --- a/compiler/rustc_mir_transform/src/coroutine.rs +++ b/compiler/rustc_mir_transform/src/coroutine.rs @@ -673,13 +673,12 @@ fn locals_live_across_suspend_points<'tcx>( // borrowed (even if they are still active). let borrowed_locals_results = MaybeBorrowedLocals.iterate_to_fixpoint(tcx, body, Some("coroutine")); - let mut borrowed_locals_cursor = borrowed_locals_results.clone().into_results_cursor(body); + let mut borrowed_locals_cursor = borrowed_locals_results.as_results_cursor(body); - // Calculate the MIR locals that we actually need to keep storage around - // for. - let mut requires_storage_cursor = MaybeRequiresStorage::new(body, borrowed_locals_results) - .iterate_to_fixpoint(tcx, body, None) - .into_results_cursor(body); + // Calculate the MIR locals for which we need to keep storage around. + let requires_storage_results = MaybeRequiresStorage::new(body, &borrowed_locals_results) + .iterate_to_fixpoint(tcx, body, None); + let mut requires_storage_cursor = requires_storage_results.as_results_cursor(body); // Calculate the liveness of MIR locals ignoring borrows. let mut liveness = @@ -752,7 +751,7 @@ fn locals_live_across_suspend_points<'tcx>( body, &saved_locals, always_live_locals.clone(), - requires_storage_cursor.into_results(), + requires_storage_results, ); LivenessInfo { From d33eb4913fd749bd98e758a3f0e2ca972b17039f Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 24 Oct 2024 15:18:49 +1100 Subject: [PATCH 11/14] Remove an unnecessary method. Because the default method is also a no-op. --- compiler/rustc_mir_dataflow/src/value_analysis.rs | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/compiler/rustc_mir_dataflow/src/value_analysis.rs b/compiler/rustc_mir_dataflow/src/value_analysis.rs index b4619b3e22e05..3054d3f5fb883 100644 --- a/compiler/rustc_mir_dataflow/src/value_analysis.rs +++ b/compiler/rustc_mir_dataflow/src/value_analysis.rs @@ -51,7 +51,7 @@ use tracing::debug; use crate::fmt::DebugWithContext; use crate::lattice::{HasBottom, HasTop}; -use crate::{Analysis, JoinSemiLattice, SwitchIntEdgeEffects}; +use crate::{Analysis, JoinSemiLattice}; pub trait ValueAnalysis<'tcx> { /// For each place of interest, the analysis tracks a value of the given type. @@ -386,14 +386,6 @@ impl<'tcx, T: ValueAnalysis<'tcx>> Analysis<'tcx> for ValueAnalysisWrapper { self.0.handle_call_return(return_places, state) } } - - fn apply_switch_int_edge_effects( - &self, - _block: BasicBlock, - _discr: &Operand<'tcx>, - _apply_edge_effects: &mut impl SwitchIntEdgeEffects, - ) { - } } rustc_index::newtype_index!( From 2b44ac24a5e38b062eeeea50c08add09d7cd0a29 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 24 Oct 2024 20:55:59 +1100 Subject: [PATCH 12/14] Remove some more unnecessary `mut` usage. --- compiler/rustc_mir_dataflow/src/framework/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_mir_dataflow/src/framework/mod.rs b/compiler/rustc_mir_dataflow/src/framework/mod.rs index e24eedde73188..6953107bbebde 100644 --- a/compiler/rustc_mir_dataflow/src/framework/mod.rs +++ b/compiler/rustc_mir_dataflow/src/framework/mod.rs @@ -242,7 +242,7 @@ pub trait Analysis<'tcx> { /// Without a `pass_name` to differentiates them, only the results for the latest run will be /// saved. fn iterate_to_fixpoint<'mir>( - mut self, + self, tcx: TyCtxt<'tcx>, body: &'mir mir::Body<'tcx>, pass_name: Option<&'static str>, @@ -286,10 +286,10 @@ pub trait Analysis<'tcx> { state.clone_from(&entry_sets[bb]); // Apply the block transfer function, using the cached one if it exists. - let edges = Self::Direction::apply_effects_in_block(&mut self, &mut state, bb, bb_data); + let edges = Self::Direction::apply_effects_in_block(&self, &mut state, bb, bb_data); Self::Direction::join_state_into_successors_of( - &mut self, + &self, body, &mut state, bb, From 514f30f4024d27d057b6f7314b86b192f3c5b4ac Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 28 Oct 2024 10:02:42 +1100 Subject: [PATCH 13/14] Reinstate `ResultsCursor` in `MaybeRequiresStorage`. Because creating a new cursor in every `check_for_move` call might results in quadratic behaviour. The cursor must be wrapped in a `RefCell`, now that analyses are passed everywhere by `&` instead of `&mut`. --- .../src/impls/storage_liveness.rs | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs b/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs index 0a575a78e63db..2f246a1db61f2 100644 --- a/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs +++ b/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs @@ -1,4 +1,5 @@ use std::borrow::Cow; +use std::cell::RefCell; use rustc_index::bit_set::BitSet; use rustc_middle::mir::visit::{NonMutatingUseContext, PlaceContext, Visitor}; @@ -99,8 +100,7 @@ impl<'a, 'tcx> Analysis<'tcx> for MaybeStorageDead<'a> { /// Dataflow analysis that determines whether each local requires storage at a /// given location; i.e. whether its storage can go away without being observed. pub struct MaybeRequiresStorage<'mir, 'tcx> { - body: &'mir Body<'tcx>, - borrowed_locals: &'mir Results<'tcx, MaybeBorrowedLocals>, + borrowed_locals: RefCell>, } impl<'mir, 'tcx> MaybeRequiresStorage<'mir, 'tcx> { @@ -108,7 +108,7 @@ impl<'mir, 'tcx> MaybeRequiresStorage<'mir, 'tcx> { body: &'mir Body<'tcx>, borrowed_locals: &'mir Results<'tcx, MaybeBorrowedLocals>, ) -> Self { - MaybeRequiresStorage { body, borrowed_locals } + MaybeRequiresStorage { borrowed_locals: borrowed_locals.as_results_cursor(body).into() } } } @@ -137,7 +137,7 @@ impl<'tcx> Analysis<'tcx> for MaybeRequiresStorage<'_, 'tcx> { loc: Location, ) { // If a place is borrowed in a statement, it needs storage for that statement. - self.borrowed_locals.analysis.apply_statement_effect(trans, stmt, loc); + self.borrowed_locals.borrow().analysis().apply_statement_effect(trans, stmt, loc); match &stmt.kind { StatementKind::StorageDead(l) => trans.kill(*l), @@ -176,7 +176,11 @@ impl<'tcx> Analysis<'tcx> for MaybeRequiresStorage<'_, 'tcx> { loc: Location, ) { // If a place is borrowed in a terminator, it needs storage for that terminator. - self.borrowed_locals.analysis.transfer_function(trans).visit_terminator(terminator, loc); + self.borrowed_locals + .borrow() + .analysis() + .transfer_function(trans) + .visit_terminator(terminator, loc); match &terminator.kind { TerminatorKind::Call { destination, .. } => { @@ -278,14 +282,15 @@ impl<'tcx> Analysis<'tcx> for MaybeRequiresStorage<'_, 'tcx> { impl<'tcx> MaybeRequiresStorage<'_, 'tcx> { /// Kill locals that are fully moved and have not been borrowed. fn check_for_move(&self, trans: &mut >::Domain, loc: Location) { - let borrowed_locals = self.borrowed_locals.as_results_cursor(self.body); - let mut visitor = MoveVisitor { trans, borrowed_locals }; - visitor.visit_location(self.body, loc); + let mut borrowed_locals = self.borrowed_locals.borrow_mut(); + let body = borrowed_locals.body(); + let mut visitor = MoveVisitor { trans, borrowed_locals: &mut borrowed_locals }; + visitor.visit_location(body, loc); } } struct MoveVisitor<'a, 'mir, 'tcx> { - borrowed_locals: ResultsCursor<'mir, 'tcx, MaybeBorrowedLocals>, + borrowed_locals: &'a mut ResultsCursor<'mir, 'tcx, MaybeBorrowedLocals>, trans: &'a mut BitSet, } From aa947cd46dd6149f41f0cc041c73094478d21163 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 28 Oct 2024 10:38:47 +1100 Subject: [PATCH 14/14] Tweak `MaybeBorrowedLocals::transfer_function` usage. First, remove the `self` param, which is unnecessary. Also, in `MaybeRequiresStorage::apply_before_statement_effect`, call `transfer_function` directly, as is already done in `MaybeRequiresStorage::apply_before_terminator_effect`. This makes it clear that the operation doesn't rely on the `MaybeBorrowedLocals` results. --- compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs | 6 +++--- compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs | 8 ++------ 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs b/compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs index 80def9e1b4ae5..288e287900a0c 100644 --- a/compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs +++ b/compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs @@ -15,7 +15,7 @@ use crate::{Analysis, GenKill}; pub struct MaybeBorrowedLocals; impl MaybeBorrowedLocals { - pub(super) fn transfer_function<'a, T>(&'a self, trans: &'a mut T) -> TransferFunction<'a, T> { + pub(super) fn transfer_function<'a, T>(trans: &'a mut T) -> TransferFunction<'a, T> { TransferFunction { trans } } } @@ -39,7 +39,7 @@ impl<'tcx> Analysis<'tcx> for MaybeBorrowedLocals { statement: &Statement<'tcx>, location: Location, ) { - self.transfer_function(trans).visit_statement(statement, location); + Self::transfer_function(trans).visit_statement(statement, location); } fn apply_terminator_effect<'mir>( @@ -48,7 +48,7 @@ impl<'tcx> Analysis<'tcx> for MaybeBorrowedLocals { terminator: &'mir Terminator<'tcx>, location: Location, ) -> TerminatorEdges<'mir, 'tcx> { - self.transfer_function(trans).visit_terminator(terminator, location); + Self::transfer_function(trans).visit_terminator(terminator, location); terminator.edges() } } diff --git a/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs b/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs index 2f246a1db61f2..7575e9e0902c6 100644 --- a/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs +++ b/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs @@ -137,7 +137,7 @@ impl<'tcx> Analysis<'tcx> for MaybeRequiresStorage<'_, 'tcx> { loc: Location, ) { // If a place is borrowed in a statement, it needs storage for that statement. - self.borrowed_locals.borrow().analysis().apply_statement_effect(trans, stmt, loc); + MaybeBorrowedLocals::transfer_function(trans).visit_statement(stmt, loc); match &stmt.kind { StatementKind::StorageDead(l) => trans.kill(*l), @@ -176,11 +176,7 @@ impl<'tcx> Analysis<'tcx> for MaybeRequiresStorage<'_, 'tcx> { loc: Location, ) { // If a place is borrowed in a terminator, it needs storage for that terminator. - self.borrowed_locals - .borrow() - .analysis() - .transfer_function(trans) - .visit_terminator(terminator, loc); + MaybeBorrowedLocals::transfer_function(trans).visit_terminator(terminator, loc); match &terminator.kind { TerminatorKind::Call { destination, .. } => {