diff --git a/compiler/rustc_borrowck/src/borrow_set.rs b/compiler/rustc_borrowck/src/borrow_set.rs index 303fa469332e0..16c3cab0afb35 100644 --- a/compiler/rustc_borrowck/src/borrow_set.rs +++ b/compiler/rustc_borrowck/src/borrow_set.rs @@ -1,3 +1,4 @@ +use std::cell::OnceCell; use std::fmt; use std::ops::Index; @@ -84,6 +85,7 @@ pub struct BorrowData<'tcx> { pub(crate) borrowed_place: mir::Place<'tcx>, /// Place to which the borrow was stored pub(crate) assigned_place: mir::Place<'tcx>, + pub(crate) dependent_regions: OnceCell>, } // These methods are public to support borrowck consumers. @@ -261,6 +263,7 @@ impl<'a, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'tcx> { activation_location: TwoPhaseActivation::NotTwoPhase, borrowed_place, assigned_place: *assigned_place, + dependent_regions: OnceCell::new(), }; let (idx, _) = self.location_map.insert_full(location, borrow); let idx = BorrowIndex::from(idx); diff --git a/compiler/rustc_borrowck/src/dataflow.rs b/compiler/rustc_borrowck/src/dataflow.rs index 7511a55b03ae5..e61218f792a3c 100644 --- a/compiler/rustc_borrowck/src/dataflow.rs +++ b/compiler/rustc_borrowck/src/dataflow.rs @@ -301,6 +301,7 @@ struct PoloniusOutOfScopePrecomputer<'a, 'tcx> { loans_out_of_scope_at_location: FxIndexMap>, } +#[expect(dead_code)] impl<'tcx> PoloniusOutOfScopePrecomputer<'_, 'tcx> { fn compute( body: &Body<'tcx>, @@ -476,11 +477,18 @@ impl<'a, 'tcx> Borrows<'a, 'tcx> { if !tcx.sess.opts.unstable_opts.polonius.is_next_enabled() { calculate_borrows_out_of_scope_at_location(body, regioncx, borrow_set) } else { - PoloniusOutOfScopePrecomputer::compute(body, regioncx, borrow_set) + unimplemented!() // This should probably be removed. }; Borrows { tcx, body, borrow_set, borrows_out_of_scope_at_location } } + /// A dummy `Borrows` with no useful information. + /// + /// Used for Polonius which doesn't need this. + pub fn dummy(tcx: TyCtxt<'tcx>, body: &'a Body<'tcx>, borrow_set: &'a BorrowSet<'tcx>) -> Self { + Borrows { tcx, body, borrow_set, borrows_out_of_scope_at_location: Default::default() } + } + /// Add all borrows to the kill set, if those borrows are out of scope at `location`. /// That means they went out of a nonlexical scope fn kill_loans_out_of_scope_at_location( @@ -563,6 +571,10 @@ impl<'tcx> rustc_mir_dataflow::Analysis<'tcx> for Borrows<'_, 'tcx> { const NAME: &'static str = "borrows"; fn bottom_value(&self, _: &mir::Body<'tcx>) -> Self::Domain { + if !self.tcx.sess.opts.unstable_opts.polonius.is_legacy_enabled() { + return DenseBitSet::new_empty(0); + } + // bottom = nothing is reserved or activated yet; DenseBitSet::new_empty(self.borrow_set.len()) } @@ -578,6 +590,10 @@ impl<'tcx> rustc_mir_dataflow::Analysis<'tcx> for Borrows<'_, 'tcx> { _statement: &mir::Statement<'tcx>, location: Location, ) { + if !self.tcx.sess.opts.unstable_opts.polonius.is_legacy_enabled() { + return; + } + self.kill_loans_out_of_scope_at_location(state, location); } @@ -587,6 +603,10 @@ impl<'tcx> rustc_mir_dataflow::Analysis<'tcx> for Borrows<'_, 'tcx> { stmt: &mir::Statement<'tcx>, location: Location, ) { + if !self.tcx.sess.opts.unstable_opts.polonius.is_legacy_enabled() { + return; + } + match &stmt.kind { mir::StatementKind::Assign(box (lhs, rhs)) => { if let mir::Rvalue::Ref(_, _, place) = rhs { @@ -636,6 +656,10 @@ impl<'tcx> rustc_mir_dataflow::Analysis<'tcx> for Borrows<'_, 'tcx> { _terminator: &mir::Terminator<'tcx>, location: Location, ) { + if !self.tcx.sess.opts.unstable_opts.polonius.is_legacy_enabled() { + return; + } + self.kill_loans_out_of_scope_at_location(state, location); } @@ -645,6 +669,10 @@ impl<'tcx> rustc_mir_dataflow::Analysis<'tcx> for Borrows<'_, 'tcx> { terminator: &'mir mir::Terminator<'tcx>, _location: Location, ) -> TerminatorEdges<'mir, 'tcx> { + if !self.tcx.sess.opts.unstable_opts.polonius.is_legacy_enabled() { + return terminator.edges(); + } + if let mir::TerminatorKind::InlineAsm { operands, .. } = &terminator.kind { for op in operands { if let mir::InlineAsmOperand::Out { place: Some(place), .. } diff --git a/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs b/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs index a845431facac1..eabf8f661bde6 100644 --- a/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs +++ b/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs @@ -632,8 +632,8 @@ impl<'tcx> MirBorrowckCtxt<'_, '_, 'tcx> { // We want to focus on relevant live locals in diagnostics, so when polonius is enabled, we // ensure that we don't emit live boring locals as explanations. let is_local_boring = |local| { - if let Some(polonius_diagnostics) = self.polonius_diagnostics { - polonius_diagnostics.boring_nll_locals.contains(&local) + if let Some(polonius) = &self.polonius { + polonius.pcx.is_boring_local(local) } else { assert!(!tcx.sess.opts.unstable_opts.polonius.is_next_enabled()); diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index 676cb618b725b..08a6bfed85b6e 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -21,6 +21,7 @@ use std::marker::PhantomData; use std::ops::{ControlFlow, Deref}; use borrow_set::LocalsStateAtExit; +use polonius::horatio::Polonius; use root_cx::BorrowCheckRootCtxt; use rustc_abi::FieldIdx; use rustc_data_structures::fx::{FxIndexMap, FxIndexSet}; @@ -46,7 +47,7 @@ use rustc_mir_dataflow::impls::{ use rustc_mir_dataflow::move_paths::{ InitIndex, InitLocation, LookupResult, MoveData, MovePathIndex, }; -use rustc_mir_dataflow::{Analysis, Results, ResultsVisitor, visit_results}; +use rustc_mir_dataflow::{Analysis, AnalysisAndResults, Results, ResultsVisitor, visit_results}; use rustc_session::lint::builtin::{TAIL_EXPR_DROP_ORDER, UNUSED_MUT}; use rustc_span::{ErrorGuaranteed, Span, Symbol}; use smallvec::SmallVec; @@ -61,7 +62,6 @@ use crate::diagnostics::{ use crate::path_utils::*; use crate::place_ext::PlaceExt; use crate::places_conflict::{PlaceConflictBias, places_conflict}; -use crate::polonius::PoloniusDiagnosticsContext; use crate::polonius::legacy::{PoloniusLocationTable, PoloniusOutput}; use crate::prefixes::PrefixSet; use crate::region_infer::RegionInferenceContext; @@ -332,37 +332,23 @@ fn do_mir_borrowck<'tcx>( let borrow_set = BorrowSet::build(tcx, body, locals_are_invalidated_at_exit, &move_data); // Compute non-lexical lifetimes. - let nll::NllOutput { - regioncx, - polonius_input, - polonius_output, - opt_closure_req, - nll_errors, - polonius_diagnostics, - } = nll::compute_regions( - root_cx, - &infcx, - universal_regions, - body, - &promoted, - &location_table, - flow_inits, - &move_data, - &borrow_set, - consumer_options, - ); + let nll::NllOutput { regioncx, polonius_input, polonius_output, opt_closure_req, nll_errors } = + nll::compute_regions( + root_cx, + &infcx, + universal_regions, + body, + &promoted, + &location_table, + flow_inits, + &move_data, + &borrow_set, + consumer_options, + ); // Dump MIR results into a file, if that is enabled. This lets us // write unit-tests, as well as helping with debugging. nll::dump_nll_mir(&infcx, body, ®ioncx, &opt_closure_req, &borrow_set); - polonius::dump_polonius_mir( - &infcx, - body, - ®ioncx, - &opt_closure_req, - &borrow_set, - polonius_diagnostics.as_ref(), - ); // We also have a `#[rustc_regions]` annotation that causes us to dump // information. @@ -403,7 +389,7 @@ fn do_mir_borrowck<'tcx>( polonius_output: None, move_errors: Vec::new(), diags_buffer, - polonius_diagnostics: polonius_diagnostics.as_ref(), + polonius: None, // FIXME: Not needed }; struct MoveVisitor<'a, 'b, 'infcx, 'tcx> { ctxt: &'a mut MirBorrowckCtxt<'b, 'infcx, 'tcx>, @@ -462,7 +448,11 @@ fn do_mir_borrowck<'tcx>( move_errors: Vec::new(), diags_buffer, polonius_output: polonius_output.as_deref(), - polonius_diagnostics: polonius_diagnostics.as_ref(), + polonius: if !tcx.sess.opts.unstable_opts.polonius.is_legacy_enabled() { + Some(Polonius::new(tcx, ®ioncx, body, ®ioncx.location_map, &borrow_set)) + } else { + None + }, }; // Compute and report region errors, if any. @@ -537,11 +527,8 @@ fn get_flow_results<'a, 'tcx>( ) -> (Borrowck<'a, 'tcx>, Results) { // We compute these three analyses individually, but them combine them into // a single results so that `mbcx` can visit them all together. - let borrows = Borrows::new(tcx, body, regioncx, borrow_set).iterate_to_fixpoint( - tcx, - body, - Some("borrowck"), - ); + let borrows = get_borrow_flow_results(tcx, body, borrow_set, regioncx); + let uninits = MaybeUninitializedPlaces::new(tcx, body, move_data).iterate_to_fixpoint( tcx, body, @@ -569,6 +556,31 @@ fn get_flow_results<'a, 'tcx>( (analysis, results) } +// FIXME: This was only factored out from `get_flow_results` to be able to count instructions +// separately. +#[inline(never)] +fn get_borrow_flow_results<'a, 'tcx>( + tcx: TyCtxt<'tcx>, + body: &'a Body<'tcx>, + borrow_set: &'a BorrowSet<'tcx>, + regioncx: &RegionInferenceContext<'tcx>, +) -> AnalysisAndResults<'tcx, Borrows<'a, 'tcx>> { + if tcx.sess.opts.unstable_opts.polonius.is_legacy_enabled() { + Borrows::new(tcx, body, regioncx, borrow_set).iterate_to_fixpoint( + tcx, + body, + Some("borrowck"), + ) + } else { + // Currently, Polonius computes the scopes of borrows with a lazy top-down strategy, so this + // is not needed. + AnalysisAndResults { + analysis: Borrows::dummy(tcx, body, borrow_set), + results: IndexVec::from_elem_n(DenseBitSet::new_empty(0), body.basic_blocks.len()), + } + } +} + pub(crate) struct BorrowckInferCtxt<'tcx> { pub(crate) infcx: InferCtxt<'tcx>, pub(crate) reg_var_to_origin: RefCell>, @@ -702,8 +714,7 @@ struct MirBorrowckCtxt<'a, 'infcx, 'tcx> { /// Results of Polonius analysis. polonius_output: Option<&'a PoloniusOutput>, - /// When using `-Zpolonius=next`: the data used to compute errors and diagnostics. - polonius_diagnostics: Option<&'a PoloniusDiagnosticsContext>, + polonius: Option>, } // Check that: @@ -921,9 +932,8 @@ impl<'a, 'tcx> ResultsVisitor<'tcx, Borrowck<'a, 'tcx>> for MirBorrowckCtxt<'a, TerminatorKind::Yield { value: _, resume: _, resume_arg: _, drop: _ } => { if self.movable_coroutine { // Look for any active borrows to locals - for i in state.borrows.iter() { - let borrow = &self.borrow_set[i]; - self.check_for_local_borrow(borrow, span); + for (i, b) in self.borrow_set.iter_enumerated() { + self.check_for_local_borrow(state, i, b, loc, span); } } } @@ -938,9 +948,8 @@ impl<'a, 'tcx> ResultsVisitor<'tcx, Borrowck<'a, 'tcx>> for MirBorrowckCtxt<'a, // Often, the storage will already have been killed by an explicit // StorageDead, but we don't always emit those (notably on unwind paths), // so this "extra check" serves as a kind of backup. - for i in state.borrows.iter() { - let borrow = &self.borrow_set[i]; - self.check_for_invalidation_at_exit(loc, borrow, span); + for (i, b) in self.borrow_set.iter_enumerated() { + self.check_for_invalidation_at_exit(state, i, b, loc, span); } } // If we do not implicitly invalidate all locals on exit, @@ -1180,77 +1189,109 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { rw: ReadOrWrite, state: &BorrowckDomain, ) -> bool { - let mut error_reported = false; - - let borrows_in_scope = self.borrows_in_scope(location, state); - - each_borrow_involving_path( - self, - self.infcx.tcx, - self.body, - (sd, place_span.0), - self.borrow_set, - |borrow_index| borrows_in_scope.contains(borrow_index), - |this, borrow_index, borrow| match (rw, borrow.kind) { + let (place, _) = place_span; + + // The number of candidates can be large, but borrows for different locals cannot conflict with + // each other, so we restrict the working set a priori. + let Some(borrows_for_place_base) = self.borrow_set.local_map.get(&place.local) else { + return false; + }; + + for &borrow_idx in borrows_for_place_base { + let borrow = &self.borrow_set[borrow_idx]; + + if !self.borrow_maybe_active_at(borrow_idx, borrow, location) { + continue; + } + + match (rw, borrow.kind) { // Obviously an activation is compatible with its own // reservation (or even prior activating uses of same // borrow); so don't check if they interfere. // // NOTE: *reservations* do conflict with themselves; - // thus aren't injecting unsoundness w/ this check.) - (Activation(_, activating), _) if activating == borrow_index => { + // thus aren't injecting unsoundness w/ self check.) + (Activation(_, activating), _) if activating == borrow_idx => { debug!( "check_access_for_conflict place_span: {:?} sd: {:?} rw: {:?} \ - skipping {:?} b/c activation of same borrow_index", + skipping {:?} b/c activation of same borrow_idx", place_span, sd, rw, - (borrow_index, borrow), + (borrow_idx, borrow), ); - ControlFlow::Continue(()) } (Read(_), BorrowKind::Shared | BorrowKind::Fake(_)) | ( Read(ReadKind::Borrow(BorrowKind::Fake(FakeBorrowKind::Shallow))), BorrowKind::Mut { .. }, - ) => ControlFlow::Continue(()), + ) => (), (Reservation(_), BorrowKind::Fake(_) | BorrowKind::Shared) => { // This used to be a future compatibility warning (to be // disallowed on NLL). See rust-lang/rust#56254 - ControlFlow::Continue(()) } (Write(WriteKind::Move), BorrowKind::Fake(FakeBorrowKind::Shallow)) => { // Handled by initialization checks. - ControlFlow::Continue(()) } (Read(kind), BorrowKind::Mut { .. }) => { + if !places_conflict::borrow_conflicts_with_place( + self.infcx.infcx.tcx, + self.body, + borrow.borrowed_place, + borrow.kind, + place.as_ref(), + sd, + places_conflict::PlaceConflictBias::Overlap, + ) { + continue; + } + // Reading from mere reservations of mutable-borrows is OK. - if !is_active(this.dominators(), borrow, location) { + if !is_active(self.dominators(), borrow, location) { assert!(borrow.kind.allows_two_phase_borrow()); - return ControlFlow::Continue(()); + continue; + } + + if !self.borrow_is_active_at(state, borrow_idx, borrow, location) { + continue; } - error_reported = true; match kind { ReadKind::Copy => { - let err = this + let err = self .report_use_while_mutably_borrowed(location, place_span, borrow); - this.buffer_error(err); + self.buffer_error(err); } ReadKind::Borrow(bk) => { let err = - this.report_conflicting_borrow(location, place_span, bk, borrow); - this.buffer_error(err); + self.report_conflicting_borrow(location, place_span, bk, borrow); + self.buffer_error(err); } } - ControlFlow::Break(()) + return true; } (Reservation(kind) | Activation(kind, _) | Write(kind), _) => { + if !places_conflict::borrow_conflicts_with_place( + self.infcx.infcx.tcx, + self.body, + borrow.borrowed_place, + borrow.kind, + place.as_ref(), + sd, + places_conflict::PlaceConflictBias::Overlap, + ) { + continue; + } + + if !self.borrow_is_active_at(state, borrow_idx, borrow, location) { + continue; + } + match rw { Reservation(..) => { debug!( @@ -1258,26 +1299,25 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { place: {:?}", place_span.0 ); - this.reservation_error_reported.insert(place_span.0); + self.reservation_error_reported.insert(place_span.0); } Activation(_, activating) => { debug!( "observing check_place for activation of \ - borrow_index: {:?}", + borrow_idx: {:?}", activating ); } Read(..) | Write(..) => {} } - error_reported = true; match kind { WriteKind::MutableBorrow(bk) => { let err = - this.report_conflicting_borrow(location, place_span, bk, borrow); - this.buffer_error(err); + self.report_conflicting_borrow(location, place_span, bk, borrow); + self.buffer_error(err); } - WriteKind::StorageDeadOrDrop => this + WriteKind::StorageDeadOrDrop => self .report_borrowed_value_does_not_live_long_enough( location, borrow, @@ -1285,21 +1325,105 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { Some(WriteKind::StorageDeadOrDrop), ), WriteKind::Mutate => { - this.report_illegal_mutation_of_borrowed(location, place_span, borrow) + self.report_illegal_mutation_of_borrowed(location, place_span, borrow) } WriteKind::Move => { - this.report_move_out_while_borrowed(location, place_span, borrow) + self.report_move_out_while_borrowed(location, place_span, borrow) } WriteKind::Replace => { - this.report_illegal_mutation_of_borrowed(location, place_span, borrow) + self.report_illegal_mutation_of_borrowed(location, place_span, borrow) } } - ControlFlow::Break(()) + + return true; } - }, - ); + } + } + + false + } + + #[inline] + fn borrow_maybe_active_at( + &mut self, + borrow_idx: BorrowIndex, + borrow: &BorrowData<'tcx>, + location: Location, + ) -> bool { + if let Some(ref mut scopes_computer) = self.polonius { + scopes_computer.loan_maybe_active_at(borrow_idx, borrow, location) + } else { + true + } + } - error_reported + fn borrow_is_active_at( + &mut self, + state: &BorrowckDomain, + borrow_idx: BorrowIndex, + borrow: &BorrowData<'tcx>, + location: Location, + ) -> bool { + if let Some(ref mut polonius) = self.polonius { + polonius.loan_is_active_at(borrow_idx, borrow, location) + } else { + let borrows_in_scope = self.borrows_in_scope(location, state); + borrows_in_scope.contains(borrow_idx) + } + } + + /// Encapsulates the idea of iterating over every borrow that involves a particular path + #[expect(dead_code)] // FIXME: Remove this method. + fn each_borrow_involving_path( + &mut self, + state: &BorrowckDomain, + location: Location, + access_place: (AccessDepth, Place<'tcx>), + mut op: impl FnMut(&mut Self, BorrowIndex, &BorrowData<'tcx>) -> ControlFlow<()>, + ) { + let (access, place) = access_place; + + // The number of candidates can be large, but borrows for different locals cannot conflict with + // each other, so we restrict the working set a priori. + let Some(borrows_for_place_base) = self.borrow_set.local_map.get(&place.local) else { + return; + }; + + // check for loan restricting path P being used. Accounts for + // borrows of P, P.a.b, etc. + for &borrow_idx in borrows_for_place_base { + let borrow = &self.borrow_set[borrow_idx]; + + if !self.borrow_maybe_active_at(borrow_idx, borrow, location) { + continue; + } + + if !places_conflict::borrow_conflicts_with_place( + self.infcx.infcx.tcx, + self.body, + borrow.borrowed_place, + borrow.kind, + place.as_ref(), + access, + places_conflict::PlaceConflictBias::Overlap, + ) { + continue; + } + + // Check if the loan is in scope. + if !self.borrow_is_active_at(state, borrow_idx, borrow, location) { + continue; + } + + debug!( + "each_borrow_involving_path: {:?} @ {:?} vs. {:?}/{:?}", + borrow_idx, borrow, place, access + ); + let ctrl = op(self, borrow_idx, borrow); + if matches!(ctrl, ControlFlow::Break(_)) { + return; + } + } } /// Through #123739, `BackwardIncompatibleDropHint`s (BIDs) are introduced. @@ -1321,40 +1445,55 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { AccessDepth::Shallow(None) }; - let borrows_in_scope = self.borrows_in_scope(location, state); - // This is a very simplified version of `Self::check_access_for_conflict`. // We are here checking on BIDs and specifically still-live borrows of data involving the BIDs. - each_borrow_involving_path( - self, - self.infcx.tcx, - self.body, - (sd, place), - self.borrow_set, - |borrow_index| borrows_in_scope.contains(borrow_index), - |this, _borrow_index, borrow| { - if matches!(borrow.kind, BorrowKind::Fake(_)) { - return ControlFlow::Continue(()); - } - let borrowed = this.retrieve_borrow_spans(borrow).var_or_use_path_span(); - let explain = this.explain_why_borrow_contains_point( - location, - borrow, - Some((WriteKind::StorageDeadOrDrop, place)), - ); - this.infcx.tcx.node_span_lint( - TAIL_EXPR_DROP_ORDER, - CRATE_HIR_ID, - borrowed, - |diag| { - session_diagnostics::TailExprDropOrder { borrowed }.decorate_lint(diag); - explain.add_explanation_to_diagnostic(&this, diag, "", None, None); - }, - ); - // We may stop at the first case - ControlFlow::Break(()) - }, - ); + + // The number of candidates can be large, but borrows for different locals cannot conflict with + // each other, so we restrict the working set a priori. + let Some(borrows_for_place_base) = self.borrow_set.local_map.get(&place.local) else { + return; + }; + + for &borrow_idx in borrows_for_place_base { + let borrow = &self.borrow_set[borrow_idx]; + + if !self.borrow_maybe_active_at(borrow_idx, borrow, location) { + continue; + } + if !places_conflict::borrow_conflicts_with_place( + self.infcx.infcx.tcx, + self.body, + borrow.borrowed_place, + borrow.kind, + place.as_ref(), + sd, + places_conflict::PlaceConflictBias::Overlap, + ) { + continue; + } + + if matches!(borrow.kind, BorrowKind::Fake(_)) { + continue; + } + + // Check if the borrow is in scope. + if !self.borrow_is_active_at(state, borrow_idx, borrow, location) { + continue; + } + + let borrowed = self.retrieve_borrow_spans(borrow).var_or_use_path_span(); + let explain = self.explain_why_borrow_contains_point( + location, + borrow, + Some((WriteKind::StorageDeadOrDrop, place)), + ); + self.infcx.tcx.node_span_lint(TAIL_EXPR_DROP_ORDER, CRATE_HIR_ID, borrowed, |diag| { + session_diagnostics::TailExprDropOrder { borrowed }.decorate_lint(diag); + explain.add_explanation_to_diagnostic(&self, diag, "", None, None); + }); + // We may stop at the first case + break; + } } fn mutate_place( @@ -1705,8 +1844,10 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { #[instrument(level = "debug", skip(self))] fn check_for_invalidation_at_exit( &mut self, - location: Location, + state: &BorrowckDomain, + borrow_idx: BorrowIndex, borrow: &BorrowData<'tcx>, + location: Location, span: Span, ) { let place = borrow.borrowed_place; @@ -1729,15 +1870,18 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { let sd = if might_be_alive { Deep } else { Shallow(None) }; - if places_conflict::borrow_conflicts_with_place( - self.infcx.tcx, - self.body, - place, - borrow.kind, - root_place, - sd, - places_conflict::PlaceConflictBias::Overlap, - ) { + if self.borrow_maybe_active_at(borrow_idx, borrow, location) + && places_conflict::borrow_conflicts_with_place( + self.infcx.tcx, + self.body, + place, + borrow.kind, + root_place, + sd, + places_conflict::PlaceConflictBias::Overlap, + ) + && self.borrow_is_active_at(state, borrow_idx, borrow, location) + { debug!("check_for_invalidation_at_exit({:?}): INVALID", place); // FIXME: should be talking about the region lifetime instead // of just a span here. @@ -1753,10 +1897,20 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { /// Reports an error if this is a borrow of local data. /// This is called for all Yield expressions on movable coroutines - fn check_for_local_borrow(&mut self, borrow: &BorrowData<'tcx>, yield_span: Span) { + fn check_for_local_borrow( + &mut self, + state: &BorrowckDomain, + borrow_idx: BorrowIndex, + borrow: &BorrowData<'tcx>, + location: Location, + yield_span: Span, + ) { debug!("check_for_local_borrow({:?})", borrow); - if borrow_of_local_data(borrow.borrowed_place) { + if borrow_of_local_data(borrow.borrowed_place) + && self.borrow_maybe_active_at(borrow_idx, borrow, location) + && self.borrow_is_active_at(state, borrow_idx, borrow, location) + { let err = self.cannot_borrow_across_coroutine_yield( self.retrieve_borrow_spans(borrow).var_or_use(), yield_span, diff --git a/compiler/rustc_borrowck/src/nll.rs b/compiler/rustc_borrowck/src/nll.rs index fe899bb054fa9..a6da7e7a3e531 100644 --- a/compiler/rustc_borrowck/src/nll.rs +++ b/compiler/rustc_borrowck/src/nll.rs @@ -22,7 +22,6 @@ use tracing::{debug, instrument}; use crate::borrow_set::BorrowSet; use crate::consumers::ConsumerOptions; use crate::diagnostics::RegionErrors; -use crate::polonius::PoloniusDiagnosticsContext; use crate::polonius::legacy::{ PoloniusFacts, PoloniusFactsExt, PoloniusLocationTable, PoloniusOutput, }; @@ -42,10 +41,6 @@ pub(crate) struct NllOutput<'tcx> { pub polonius_output: Option>, pub opt_closure_req: Option>, pub nll_errors: RegionErrors<'tcx>, - - /// When using `-Zpolonius=next`: the data used to compute errors and diagnostics, e.g. - /// localized typeck and liveness constraints. - pub polonius_diagnostics: Option, } /// Rewrites the regions in the MIR to use NLL variables, also scraping out the set of universal @@ -98,24 +93,20 @@ pub(crate) fn compute_regions<'a, 'tcx>( let location_map = Rc::new(DenseLocationMap::new(body)); // Run the MIR type-checker. - let MirTypeckResults { - constraints, - universal_region_relations, - opaque_type_values, - polonius_context, - } = type_check::type_check( - root_cx, - infcx, - body, - promoted, - universal_regions, - location_table, - borrow_set, - &mut polonius_facts, - flow_inits, - move_data, - Rc::clone(&location_map), - ); + let MirTypeckResults { constraints, universal_region_relations, opaque_type_values } = + type_check::type_check( + root_cx, + infcx, + body, + promoted, + universal_regions, + location_table, + borrow_set, + &mut polonius_facts, + flow_inits, + move_data, + Rc::clone(&location_map), + ); // If requested, emit legacy polonius facts. polonius::legacy::emit_facts( @@ -129,14 +120,12 @@ pub(crate) fn compute_regions<'a, 'tcx>( &constraints, ); - let mut regioncx = - RegionInferenceContext::new(infcx, constraints, universal_region_relations, location_map); - - // If requested for `-Zpolonius=next`, convert NLL constraints to localized outlives constraints - // and use them to compute loan liveness. - let polonius_diagnostics = polonius_context.map(|polonius_context| { - polonius_context.compute_loan_liveness(infcx.tcx, &mut regioncx, body, borrow_set) - }); + let mut regioncx = RegionInferenceContext::new( + infcx, + constraints, + universal_region_relations, + Rc::clone(&location_map), + ); // If requested: dump NLL facts, and run legacy polonius analysis. let polonius_output = polonius_facts.as_ref().and_then(|polonius_facts| { @@ -176,7 +165,6 @@ pub(crate) fn compute_regions<'a, 'tcx>( polonius_output, opt_closure_req: closure_region_requirements, nll_errors, - polonius_diagnostics, } } diff --git a/compiler/rustc_borrowck/src/polonius/horatio/constraints.rs b/compiler/rustc_borrowck/src/polonius/horatio/constraints.rs new file mode 100644 index 0000000000000..9983a6c705237 --- /dev/null +++ b/compiler/rustc_borrowck/src/polonius/horatio/constraints.rs @@ -0,0 +1,481 @@ +use std::mem; +use std::ops::ControlFlow; + +use rustc_index::IndexVec; +use rustc_index::bit_set::{DenseBitSet, SparseBitMatrix}; +use rustc_middle::mir::{ + BasicBlock, Body, Location, Statement, StatementKind, Terminator, TerminatorKind, +}; +use rustc_middle::ty::{TyCtxt, TypeVisitable}; +use rustc_mir_dataflow::points::{DenseLocationMap, PointIndex}; + +use crate::constraints::OutlivesConstraint; +use crate::type_check::Locations; +use crate::{RegionInferenceContext, RegionVid}; + +/// The location-sensitive constraint graph. +/// +/// This struct contains all outlives constraints. It internally distinguishes between global +/// constraints, which are in effect everywhere, and local constraints, which apply only at a +/// specific point. It can retrieve all constraints at a given point in constant time. +pub(crate) struct Constraints<'a, 'tcx> { + /// A mapping from points to local outlives constraints (only active at a single point). + /// + /// At point `p`, we store all local outlives constraints that take effect at `p`. This means + /// that their sup-region (`'a` in `'a: 'b`) is checked at `p`. As a consequence, time-travelling + /// constraints that travel backwards in time are stored at the successor location(s) of the + /// location from `constraint.locations`. + local_constraints: IndexVec>, + + /// A list of all outlives constraints that are active at every point in the CFG. + global_constraints: Vec, + + tcx: TyCtxt<'tcx>, + regioncx: &'a RegionInferenceContext<'tcx>, + body: &'a Body<'tcx>, + location_map: &'a DenseLocationMap, +} + +/// A global outlives constraint that is active at every point in the CFG. +#[derive(Clone, Copy)] +struct GlobalConstraint { + /// If we have the constraint `'a: 'b`, then `'a` is the sup and `'b` the sub. + sup: RegionVid, + /// If we have the constraint `'a: 'b`, then `'a` is the sup and `'b` the sub. + sub: RegionVid, +} + +/// A local outlives constraint that is only active at a single point in the CFG. +#[derive(Clone, Copy)] +struct LocalConstraint { + /// If we have the constraint `'a: 'b`, then `'a` is the sup and `'b` the sub. + sup: RegionVid, + /// If we have the constraint `'a: 'b`, then `'a` is the sup and `'b` the sub. + sub: RegionVid, + + /// If and how the constraint travels in time. + time_travel: Option<(TimeTravelDirection, TimeTravelKind)>, +} + +/// The direction of a time-travelling constraint. +/// +/// Most local constraints apply at a single location in the CFG, but some flow either backwards or +/// forwards in time—to the previous or next location. For instance, given the constraint `'a: 'b` +/// at location `l`, if the constraint flows forwards, then `'b` is active at the successor of `l` +/// if `'a` is active at `l`. Conversely, if it flows backwards, `'b` is active at the predecessor +/// of `l` if `'a` is active at `l`. +#[derive(Debug, Copy, Clone)] +enum TimeTravelDirection { + /// The constraint flows backwards in time. + /// + /// `'a: 'b` at location `l` means that `'b` is active at the predecessor of `l` if `'a` is + /// active at `l`. + Backwards, + + /// The constraint flows forwards in time. + /// + /// `'a: 'b` at location `l` means that `'b` is active at the successor of `l` if `'a` is + /// active at `l`. + Forwards, +} + +/// Whether a time-travelling constraint stays within the same block or crosses block boundaries. +/// +/// The constraint's "location" (or source location) is the point in the CFG where the sup-region is +/// checked. For `'a: 'b`, this is where `'a` is checked. The "target location" is where `'b` becomes +/// active. The source and target locations are either the same, or—if the constraint is +/// time-travelling—adjacent. +#[derive(Debug, Copy, Clone)] +enum TimeTravelKind { + /// The constraint travels within the same block. + /// + /// Suppose we have the constraint `'a: 'b`. If it travels backwards, then `'a` cannot be checked + /// at the first location in a block, or `'b` would be active in a preceding block. Similarly, + /// if it travels forwards, `'a` cannot be checked at the terminator. + IntraBlock, + + /// The constraint travels across block boundaries. + /// + /// The source and target locations are in different blocks. Since they must be adjacent, a + /// forward-travelling constraint implies the source location is a terminator and the target is + /// the first location of a block. Conversely, a backward-travelling constraint implies the + /// source is the first location of a block and the target is a terminator. + InterBlock { + /// The block containing the target location. + /// + /// The statement index of the target location is `0` if the constraint travels forwards, + /// or the index of the terminator if it travels backwards. + target_block: BasicBlock, + }, +} + +#[derive(Default)] +pub(crate) struct TimeTravellingRegions { + /// Regions travelling to the proceeding statement within the same block. + pub to_prev_stmt: Option>, + + /// Regions travelling to proceeding blocks. Only applicable at the first statement of a block. + pub to_predecessor_blocks: Option>, + + /// Regions travelling to the next statement within the same block. Not applicable for + /// terminators. + pub to_next_loc: Option>, + + /// Regions travelling to succeeding blocks. Only applicable at the first statement of a block. + pub to_successor_blocks: Option>, +} + +impl TimeTravellingRegions { + fn add_within_block( + &mut self, + regioncx: &RegionInferenceContext<'_>, + region: RegionVid, + direction: TimeTravelDirection, + ) { + match direction { + TimeTravelDirection::Forwards => { + self.to_next_loc + .get_or_insert_with(|| DenseBitSet::new_empty(regioncx.num_regions())) + .insert(region); + } + TimeTravelDirection::Backwards => { + self.to_prev_stmt + .get_or_insert_with(|| DenseBitSet::new_empty(regioncx.num_regions())) + .insert(region); + } + } + } + + fn add_to_predecessor_block( + &mut self, + regioncx: &RegionInferenceContext<'_>, + region: RegionVid, + preceeding_block: BasicBlock, + ) { + self.to_predecessor_blocks + .get_or_insert_with(|| SparseBitMatrix::new(regioncx.num_regions())) + .insert(preceeding_block, region); + } + + fn add_to_successor_block( + &mut self, + regioncx: &RegionInferenceContext<'_>, + region: RegionVid, + succeeding_block: BasicBlock, + ) { + self.to_successor_blocks + .get_or_insert_with(|| SparseBitMatrix::new(regioncx.num_regions())) + .insert(succeeding_block, region); + } +} + +impl<'a, 'tcx> Constraints<'a, 'tcx> { + pub(crate) fn new( + tcx: TyCtxt<'tcx>, + regioncx: &'a RegionInferenceContext<'tcx>, + body: &'a Body<'tcx>, + location_map: &'a DenseLocationMap, + ) -> Self { + Self { + local_constraints: IndexVec::from_elem_n(vec![], location_map.num_points()), + global_constraints: vec![], + tcx, + regioncx, + body, + location_map, + } + } + + pub(crate) fn add_constraint(&mut self, constraint: &OutlivesConstraint<'tcx>) { + match constraint.locations { + Locations::Single(location) => { + let (source_location, time_travel) = if let Some(stmt) = + self.body[location.block].statements.get(location.statement_index) + { + match self.time_travel_at_statement(&constraint, stmt) { + Some(t @ TimeTravelDirection::Forwards) => { + (location, Some((t, TimeTravelKind::IntraBlock))) + } + Some(t @ TimeTravelDirection::Backwards) => ( + location.successor_within_block(), + Some((t, TimeTravelKind::IntraBlock)), + ), + None => (location, None), + } + } else { + debug_assert_eq!( + location.statement_index, + self.body[location.block].statements.len() + ); + let terminator = self.body[location.block].terminator(); + match self.time_travel_at_terminator(&constraint, terminator) { + Some((t @ TimeTravelDirection::Forwards, target_block)) => { + (location, Some((t, TimeTravelKind::InterBlock { target_block }))) + } + Some((t @ TimeTravelDirection::Backwards, source_block)) => ( + Location { block: source_block, statement_index: 0 }, + Some((t, TimeTravelKind::InterBlock { target_block: location.block })), + ), + None => (location, None), + } + }; + + let point = self.location_map.point_from_location(source_location); + self.local_constraints[point].push(LocalConstraint { + sup: constraint.sup, + sub: constraint.sub, + time_travel, + }); + } + Locations::All(_) => { + self.global_constraints + .push(GlobalConstraint { sup: constraint.sup, sub: constraint.sub }); + } + } + } + + /// Checks if and in which direction a constraint at a statement travels in time. + fn time_travel_at_statement( + &self, + constraint: &OutlivesConstraint<'tcx>, + statement: &Statement<'tcx>, + ) -> Option { + match &statement.kind { + StatementKind::Assign(box (lhs, _)) => { + let lhs_ty = self.body.local_decls[lhs.local].ty; + self.compute_constraint_direction(constraint, &lhs_ty) + } + _ => None, + } + } + + /// Check if/how an outlives constraint travels in time at a terminator. + /// + /// Returns an `Option` of the pair `(direction, block)`. Where `direction` is a + /// `TimeTravelDirection` and `block` is the target or source block of a forwards or backwards + /// travelling constraint respectively. + fn time_travel_at_terminator( + &self, + constraint: &OutlivesConstraint<'tcx>, + terminator: &Terminator<'tcx>, + ) -> Option<(TimeTravelDirection, BasicBlock)> { + // FIXME: check if other terminators need the same handling as `Call`s, in particular + // Assert/Yield/Drop. A handful of tests are failing with Drop related issues, as well as some + // coroutine tests, and that may be why. + match &terminator.kind { + // FIXME: also handle diverging calls. + TerminatorKind::Call { destination, target: Some(target_block), .. } => { + // Calls are similar to assignments, and thus follow the same pattern. If there is a + // target for the call we also relate what flows into the destination here to entry to + // that successor. + let destination_ty = destination.ty(&self.body.local_decls, self.tcx); + self.compute_constraint_direction(constraint, &destination_ty) + .map(|t| (t, *target_block)) + } + _ => None, + } + } + + /// For a given outlives constraint and CFG edge, returns the localized constraint with the + /// appropriate `from`-`to` direction. This is computed according to whether the constraint flows to + /// or from a free region in the given `value`, some kind of result for an effectful operation, like + /// the LHS of an assignment. + fn compute_constraint_direction( + &self, + constraint: &OutlivesConstraint<'tcx>, + value: &impl TypeVisitable>, + ) -> Option { + // FIXME: There seem to be cases where both sub and sup appear in the free regions. + + self.tcx.for_each_free_region_until(value, |region| { + let region = self.regioncx.universal_regions().to_region_vid(region); + if region == constraint.sub { + // This constraint flows into the result, its effects start becoming visible on exit. + ControlFlow::Break(TimeTravelDirection::Forwards) + } else if region == constraint.sup { + // This constraint flows from the result, its effects start becoming visible on exit. + ControlFlow::Break(TimeTravelDirection::Backwards) + } else { + ControlFlow::Continue(()) + } + }) + } + + /// Given a set of regions at a certain point in the CFG, add all regions induced by outlives + /// constraints at that point to the set. Additionally, all regions arising from time + /// travelling constraints will be collected and returned. + /// + /// If we have the set `{'a, 'b}`, and we have the following constraints: + /// - `'a: 'c` + /// - `'b: 'd` + /// - `'d: 'e` + /// Then `'c`, `'d` and `'e` will be added to the set. + /// + /// Also, any time travelling constraints implied by any of these five regions would be + /// collected and returned in the `TimeTravellingRegions` struct. + pub(crate) fn add_dependent_regions_at_point( + &self, + point: PointIndex, + regions: &mut DenseBitSet, + ) -> TimeTravellingRegions { + // This function will loop until there are no more regions to add. It will keep a set of + // regions that has not been considered yet (the `to_check` variable). At each iteration of + // the main loop, It'll walk through all constraints at this point and all global + // constraints. Any regions implied from the `to_check` set will be put in the + // `to_check_next_round` set. When all constraints has been considered, the `to_check` set + // will be cleared. It will be swaped with the `to_check_next_round` set, and then the main + // loop runs again. It'll stop when there are no more regions to check. + // + // The time travelling constraints will be treated differently. Regions implied by time + // travelling constraints will be collected in an instance of the `TimeTravellingRegions` + // struct. + + let mut to_check = regions.clone(); + let mut to_check_next_round = DenseBitSet::new_empty(self.regioncx.num_regions()); + let mut time_travelling_regions = TimeTravellingRegions::default(); + + // Loop till the fixpoint: when there are no more regions to add. + while !to_check.is_empty() { + // Loop through all global constraints. + for constraint in &self.global_constraints { + if !to_check.contains(constraint.sup) { + continue; + } + if regions.insert(constraint.sub) { + to_check_next_round.insert(constraint.sub); + } + } + + // Loop through all local constraints. + for constraint in &self.local_constraints[point] { + if !to_check.contains(constraint.sup) { + continue; + } + + // Check if the constraint is travelling in time. + if let Some((travel_direction, travel_kind)) = constraint.time_travel { + match (travel_direction, travel_kind) { + (direction, TimeTravelKind::IntraBlock) => time_travelling_regions + .add_within_block(self.regioncx, constraint.sub, direction), + ( + TimeTravelDirection::Forwards, + TimeTravelKind::InterBlock { target_block }, + ) => time_travelling_regions.add_to_successor_block( + self.regioncx, + constraint.sub, + target_block, + ), + ( + TimeTravelDirection::Backwards, + TimeTravelKind::InterBlock { target_block }, + ) => time_travelling_regions.add_to_predecessor_block( + self.regioncx, + constraint.sub, + target_block, + ), + } + + // If the region is time travelling we should not add it to + // `regions`. + continue; + } + + if regions.insert(constraint.sub) { + to_check_next_round.insert(constraint.sub); + } + } + + mem::swap(&mut to_check, &mut to_check_next_round); + to_check_next_round.clear(); + } + + time_travelling_regions + } + + /// Given a set of regions, add all regions induced by outlives constraints at any point in the + /// CFG to the set. + /// + /// If we have the set `{'a, 'b}`, and we have the following constraints: + /// - `'a: 'c` + /// - `'b: 'd` + /// - `'d: 'e` + /// Then `'c`, `'d` and `'e` will be added to the set. + #[inline(never)] // FIXME: Remove this. + pub(crate) fn add_dependent_regions(&self, regions: &mut DenseBitSet) { + // This function will loop until there are no more regions to add. It will keep a set of + // regions that has not been considered yet (the `to_check` variable). At each iteration of + // the main loop, It'll walk through all constraints, both global and local. Any regions + // implied from the `to_check` set will be put in the `to_check_next_round` set. When all + // constraints has been considered, the `to_check` set will be cleared. It will be swaped + // with the `to_check_next_round` set, and then the main loop runs again. It'll stop when + // there are no more regions to check. + // + // The time travelling constraints will not be treated differently in this function. + + let mut to_check = regions.clone(); + let mut to_check_next_round = DenseBitSet::new_empty(self.regioncx.num_regions()); + + // Loop till the fixpoint: when there are no more regions to add. + while !to_check.is_empty() { + // Loop through all global constraints. + for constraint in &self.global_constraints { + if !to_check.contains(constraint.sup) { + continue; + } + if regions.insert(constraint.sub) { + to_check_next_round.insert(constraint.sub); + } + } + + // Loop through all local constraints. + for constraint in self.local_constraints.iter().flatten() { + if !to_check.contains(constraint.sup) { + continue; + } + if regions.insert(constraint.sub) { + to_check_next_round.insert(constraint.sub); + } + } + + mem::swap(&mut to_check, &mut to_check_next_round); + to_check_next_round.clear(); + } + } + + /// Like `add_dependent_regions()` but with constraints reversed. + // FIXME: Could these functions be merged to avoid code duplication. + #[inline(never)] // FIXME: Remove this. + pub(crate) fn add_dependent_regions_reversed(&self, regions: &mut DenseBitSet) { + // See the `add_dependent_regions()` function for an explonation of the code. The functions + // are identical except that we swapped sub and sup. + + let mut to_check = regions.clone(); + let mut to_check_next_round = DenseBitSet::new_empty(self.regioncx.num_regions()); + + // Loop till the fixpoint: when there are no more regions to add. + while !to_check.is_empty() { + // Loop through all global constraints. + for constraint in &self.global_constraints { + if !to_check.contains(constraint.sub) { + continue; + } + if regions.insert(constraint.sup) { + to_check_next_round.insert(constraint.sup); + } + } + + // Loop through all local constraints. + for constraint in self.local_constraints.iter().flatten() { + if !to_check.contains(constraint.sub) { + continue; + } + if regions.insert(constraint.sup) { + to_check_next_round.insert(constraint.sup); + } + } + + mem::swap(&mut to_check, &mut to_check_next_round); + to_check_next_round.clear(); + } + } +} diff --git a/compiler/rustc_borrowck/src/polonius/horatio/live_region_variance.rs b/compiler/rustc_borrowck/src/polonius/horatio/live_region_variance.rs new file mode 100644 index 0000000000000..fc059fc9f8a6c --- /dev/null +++ b/compiler/rustc_borrowck/src/polonius/horatio/live_region_variance.rs @@ -0,0 +1,207 @@ +use std::collections::BTreeMap; + +use rustc_middle::mir::visit::{TyContext, Visitor}; +use rustc_middle::mir::{Body, Location, SourceInfo}; +use rustc_middle::ty::relate::{self, Relate, RelateResult, TypeRelation}; +use rustc_middle::ty::{GenericArgsRef, Region, RegionVid, Ty, TyCtxt, TypeVisitable}; +use rustc_middle::{span_bug, ty}; + +use super::ConstraintDirection; +use crate::RegionInferenceContext; +use crate::universal_regions::UniversalRegions; + +/// Some variables are "regular live" at `location` -- i.e., they may be used later. This means that +/// all regions appearing in their type must be live at `location`. +pub(super) fn compute_live_region_variances<'tcx>( + tcx: TyCtxt<'tcx>, + regioncx: &RegionInferenceContext<'tcx>, + body: &Body<'tcx>, +) -> BTreeMap { + let mut directions = BTreeMap::new(); + + let variance_extractor = VarianceExtractor { + tcx, + ambient_variance: ty::Variance::Covariant, + directions: &mut directions, + universal_regions: regioncx.universal_regions(), + }; + + let mut visitor = LiveVariablesVisitor { tcx, regioncx, variance_extractor }; + + for (bb, data) in body.basic_blocks.iter_enumerated() { + visitor.visit_basic_block_data(bb, data); + } + + directions +} + +struct LiveVariablesVisitor<'a, 'tcx> { + tcx: TyCtxt<'tcx>, + regioncx: &'a RegionInferenceContext<'tcx>, + variance_extractor: VarianceExtractor<'a, 'tcx>, +} + +impl<'a, 'tcx> Visitor<'tcx> for LiveVariablesVisitor<'a, 'tcx> { + /// We sometimes have `args` within an rvalue, or within a + /// call. Make them live at the location where they appear. + fn visit_args(&mut self, args: &GenericArgsRef<'tcx>, _: Location) { + self.record_regions_live_at(*args); + self.super_args(args); + } + + /// We sometimes have `region`s within an rvalue, or within a + /// call. Make them live at the location where they appear. + fn visit_region(&mut self, region: Region<'tcx>, _: Location) { + self.record_regions_live_at(region); + self.super_region(region); + } + + /// We sometimes have `ty`s within an rvalue, or within a + /// call. Make them live at the location where they appear. + fn visit_ty(&mut self, ty: Ty<'tcx>, ty_context: TyContext) { + match ty_context { + TyContext::ReturnTy(SourceInfo { span, .. }) + | TyContext::YieldTy(SourceInfo { span, .. }) + | TyContext::ResumeTy(SourceInfo { span, .. }) + | TyContext::UserTy(span) + | TyContext::LocalDecl { source_info: SourceInfo { span, .. }, .. } => { + span_bug!(span, "should not be visiting outside of the CFG: {:?}", ty_context); + } + TyContext::Location(_) => { + self.record_regions_live_at(ty); + } + } + + self.super_ty(ty); + } +} + +impl<'a, 'tcx> LiveVariablesVisitor<'a, 'tcx> { + /// Some variable is "regular live" at `location` -- i.e., it may be used later. This means that + /// all regions appearing in the type of `value` must be live at `location`. + fn record_regions_live_at(&mut self, value: T) + where + T: TypeVisitable> + Relate>, + { + self.variance_extractor + .relate(value, value) + .expect("Can't have a type error relating to itself"); + } +} + +/// Extracts variances for regions contained within types. Follows the same structure as +/// `rustc_infer`'s `Generalizer`: we try to relate a type with itself to track and extract the +/// variances of regions. +pub(super) struct VarianceExtractor<'a, 'tcx> { + pub tcx: TyCtxt<'tcx>, + pub ambient_variance: ty::Variance, + pub directions: &'a mut BTreeMap, + pub universal_regions: &'a UniversalRegions<'tcx>, +} + +impl<'tcx> VarianceExtractor<'_, 'tcx> { + fn record_variance(&mut self, region: ty::Region<'tcx>, variance: ty::Variance) { + // We're only interested in the variance of vars and free regions. + // + // Note: even if we currently bail for two cases of unexpected region kinds here, missing + // variance data is not a soundness problem: the regions with missing variance will still be + // present in the constraint graph as they are live, and liveness edges construction has a + // fallback for this case. + // + // FIXME: that being said, we need to investigate these cases better to not ignore regions + // in general. + if region.is_bound() { + // We ignore these because they cannot be turned into the vids we need. + return; + } + + if region.is_erased() { + // These cannot be turned into a vid either, and we also ignore them: the fact that they + // show up here looks like either an issue upstream or a combination with unexpectedly + // continuing compilation too far when we're in a tainted by errors situation. + // + // FIXME: investigate the `generic_const_exprs` test that triggers this issue, + // `ui/const-generics/generic_const_exprs/issue-97047-ice-2.rs` + return; + } + + let direction = match variance { + ty::Covariant => ConstraintDirection::Forward, + ty::Contravariant => ConstraintDirection::Backward, + ty::Invariant => ConstraintDirection::Bidirectional, + ty::Bivariant => { + // We don't add edges for bivariant cases. + return; + } + }; + + let region = self.universal_regions.to_region_vid(region); + self.directions + .entry(region) + .and_modify(|entry| { + // If there's already a recorded direction for this region, we combine the two: + // - combining the same direction is idempotent + // - combining different directions is trivially bidirectional + if entry != &direction { + *entry = ConstraintDirection::Bidirectional; + } + }) + .or_insert(direction); + } +} + +impl<'tcx> TypeRelation> for VarianceExtractor<'_, 'tcx> { + fn cx(&self) -> TyCtxt<'tcx> { + self.tcx + } + + fn relate_with_variance>>( + &mut self, + variance: ty::Variance, + _info: ty::VarianceDiagInfo>, + a: T, + b: T, + ) -> RelateResult<'tcx, T> { + let old_ambient_variance = self.ambient_variance; + self.ambient_variance = self.ambient_variance.xform(variance); + let r = self.relate(a, b)?; + self.ambient_variance = old_ambient_variance; + Ok(r) + } + + fn tys(&mut self, a: Ty<'tcx>, b: Ty<'tcx>) -> RelateResult<'tcx, Ty<'tcx>> { + assert_eq!(a, b); // we are misusing TypeRelation here; both LHS and RHS ought to be == + relate::structurally_relate_tys(self, a, b) + } + + fn regions( + &mut self, + a: ty::Region<'tcx>, + b: ty::Region<'tcx>, + ) -> RelateResult<'tcx, ty::Region<'tcx>> { + assert_eq!(a, b); // we are misusing TypeRelation here; both LHS and RHS ought to be == + self.record_variance(a, self.ambient_variance); + Ok(a) + } + + fn consts( + &mut self, + a: ty::Const<'tcx>, + b: ty::Const<'tcx>, + ) -> RelateResult<'tcx, ty::Const<'tcx>> { + assert_eq!(a, b); // we are misusing TypeRelation here; both LHS and RHS ought to be == + relate::structurally_relate_consts(self, a, b) + } + + fn binders( + &mut self, + a: ty::Binder<'tcx, T>, + _: ty::Binder<'tcx, T>, + ) -> RelateResult<'tcx, ty::Binder<'tcx, T>> + where + T: Relate>, + { + self.relate(a.skip_binder(), a.skip_binder())?; + Ok(a) + } +} diff --git a/compiler/rustc_borrowck/src/polonius/horatio/loan_invalidations.rs b/compiler/rustc_borrowck/src/polonius/horatio/loan_invalidations.rs new file mode 100644 index 0000000000000..24ebdfa045cd1 --- /dev/null +++ b/compiler/rustc_borrowck/src/polonius/horatio/loan_invalidations.rs @@ -0,0 +1,436 @@ +use std::ops::ControlFlow; + +use rustc_data_structures::graph::dominators::Dominators; +use rustc_index::IndexVec; +use rustc_middle::mir::visit::Visitor; +use rustc_middle::mir::*; +use rustc_middle::ty::TyCtxt; + +use crate::borrow_set::BorrowSet; +use crate::path_utils::*; +use crate::{ + AccessDepth, Activation, ArtificialField, BorrowIndex, Deep, LocalMutationIsAllowed, Read, + ReadKind, ReadOrWrite, Reservation, Shallow, Write, WriteKind, +}; + +/// Compute if/when loans are invalidated. +pub(super) fn compute_loan_invalidations<'tcx>( + tcx: TyCtxt<'tcx>, + body: &Body<'tcx>, + borrow_set: &BorrowSet<'tcx>, +) -> IndexVec> { + let dominators = body.basic_blocks.dominators(); + let mut visitor = LoanInvalidationsGenerator { + borrow_set, + tcx, + body, + dominators, + loan_invalidated_at: IndexVec::from_fn_n(|_| Default::default(), borrow_set.len()), + }; + visitor.visit_body(body); + visitor.loan_invalidated_at +} + +struct LoanInvalidationsGenerator<'a, 'tcx> { + tcx: TyCtxt<'tcx>, + body: &'a Body<'tcx>, + dominators: &'a Dominators, + borrow_set: &'a BorrowSet<'tcx>, + loan_invalidated_at: IndexVec>, +} + +/// Visits the whole MIR and generates `invalidates()` facts. +/// Most of the code implementing this was stolen from `borrow_check/mod.rs`. +impl<'a, 'tcx> Visitor<'tcx> for LoanInvalidationsGenerator<'a, 'tcx> { + fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) { + self.check_activations(location); + + match &statement.kind { + StatementKind::Assign(box (lhs, rhs)) => { + self.consume_rvalue(location, rhs); + + self.mutate_place(location, *lhs, Shallow(None)); + } + StatementKind::FakeRead(box (_, _)) => { + // Only relevant for initialized/liveness/safety checks. + } + StatementKind::Intrinsic(box NonDivergingIntrinsic::Assume(op)) => { + self.consume_operand(location, op); + } + StatementKind::Intrinsic(box NonDivergingIntrinsic::CopyNonOverlapping(CopyNonOverlapping { + src, + dst, + count, + })) => { + self.consume_operand(location, src); + self.consume_operand(location, dst); + self.consume_operand(location, count); + } + // Only relevant for mir typeck + StatementKind::AscribeUserType(..) + // Only relevant for liveness and unsafeck + | StatementKind::PlaceMention(..) + // Doesn't have any language semantics + | StatementKind::Coverage(..) + // Does not actually affect borrowck + | StatementKind::StorageLive(..) => {} + StatementKind::StorageDead(local) => { + self.access_place( + location, + Place::from(*local), + (Shallow(None), Write(WriteKind::StorageDeadOrDrop)), + LocalMutationIsAllowed::Yes, + ); + } + StatementKind::ConstEvalCounter + | StatementKind::Nop + | StatementKind::Retag { .. } + | StatementKind::Deinit(..) + | StatementKind::BackwardIncompatibleDropHint { .. } + | StatementKind::SetDiscriminant { .. } => {} + } + + self.super_statement(statement, location); + } + + fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location) { + self.check_activations(location); + + match &terminator.kind { + TerminatorKind::SwitchInt { discr, targets: _ } => { + self.consume_operand(location, discr); + } + TerminatorKind::Drop { place: drop_place, replace, .. } => { + let write_kind = + if *replace { WriteKind::Replace } else { WriteKind::StorageDeadOrDrop }; + self.access_place( + location, + *drop_place, + (AccessDepth::Drop, Write(write_kind)), + LocalMutationIsAllowed::Yes, + ); + } + TerminatorKind::Call { + func, + args, + destination, + target: _, + unwind: _, + call_source: _, + fn_span: _, + } => { + self.consume_operand(location, func); + for arg in args { + self.consume_operand(location, &arg.node); + } + self.mutate_place(location, *destination, Deep); + } + TerminatorKind::TailCall { func, args, .. } => { + self.consume_operand(location, func); + for arg in args { + self.consume_operand(location, &arg.node); + } + } + TerminatorKind::Assert { cond, expected: _, msg, target: _, unwind: _ } => { + self.consume_operand(location, cond); + use rustc_middle::mir::AssertKind; + if let AssertKind::BoundsCheck { len, index } = &**msg { + self.consume_operand(location, len); + self.consume_operand(location, index); + } + } + TerminatorKind::Yield { value, resume, resume_arg, drop: _ } => { + self.consume_operand(location, value); + + // Invalidate all borrows of local places + let resume_location = resume.start_location(); + for (i, data) in self.borrow_set.iter_enumerated() { + if borrow_of_local_data(data.borrowed_place) { + self.loan_invalidated_at[i].push(resume_location); + } + } + + self.mutate_place(location, *resume_arg, Deep); + } + TerminatorKind::UnwindResume + | TerminatorKind::Return + | TerminatorKind::CoroutineDrop => { + // Invalidate all borrows of local places + let start_location = location; + for (i, data) in self.borrow_set.iter_enumerated() { + if borrow_of_local_data(data.borrowed_place) { + self.loan_invalidated_at[i].push(start_location); + } + } + } + TerminatorKind::InlineAsm { + asm_macro: _, + template: _, + operands, + options: _, + line_spans: _, + targets: _, + unwind: _, + } => { + for op in operands { + match op { + InlineAsmOperand::In { reg: _, value } => { + self.consume_operand(location, value); + } + InlineAsmOperand::Out { reg: _, late: _, place, .. } => { + if let &Some(place) = place { + self.mutate_place(location, place, Shallow(None)); + } + } + InlineAsmOperand::InOut { reg: _, late: _, in_value, out_place } => { + self.consume_operand(location, in_value); + if let &Some(out_place) = out_place { + self.mutate_place(location, out_place, Shallow(None)); + } + } + InlineAsmOperand::Const { value: _ } + | InlineAsmOperand::SymFn { value: _ } + | InlineAsmOperand::SymStatic { def_id: _ } + | InlineAsmOperand::Label { target_index: _ } => {} + } + } + } + TerminatorKind::Goto { target: _ } + | TerminatorKind::UnwindTerminate(_) + | TerminatorKind::Unreachable + | TerminatorKind::FalseEdge { real_target: _, imaginary_target: _ } + | TerminatorKind::FalseUnwind { real_target: _, unwind: _ } => { + // no data used, thus irrelevant to borrowck + } + } + + self.super_terminator(terminator, location); + } +} + +impl<'a, 'tcx> LoanInvalidationsGenerator<'a, 'tcx> { + /// Simulates mutation of a place. + fn mutate_place(&mut self, location: Location, place: Place<'tcx>, kind: AccessDepth) { + self.access_place( + location, + place, + (kind, Write(WriteKind::Mutate)), + LocalMutationIsAllowed::ExceptUpvars, + ); + } + + /// Simulates consumption of an operand. + fn consume_operand(&mut self, location: Location, operand: &Operand<'tcx>) { + match *operand { + Operand::Copy(place) => { + self.access_place( + location, + place, + (Deep, Read(ReadKind::Copy)), + LocalMutationIsAllowed::No, + ); + } + Operand::Move(place) => { + self.access_place( + location, + place, + (Deep, Write(WriteKind::Move)), + LocalMutationIsAllowed::Yes, + ); + } + Operand::Constant(_) => {} + } + } + + // Simulates consumption of an rvalue + fn consume_rvalue(&mut self, location: Location, rvalue: &Rvalue<'tcx>) { + match rvalue { + &Rvalue::Ref(_ /*rgn*/, bk, place) => { + let access_kind = match bk { + BorrowKind::Fake(FakeBorrowKind::Shallow) => { + (Shallow(Some(ArtificialField::FakeBorrow)), Read(ReadKind::Borrow(bk))) + } + BorrowKind::Shared | BorrowKind::Fake(FakeBorrowKind::Deep) => { + (Deep, Read(ReadKind::Borrow(bk))) + } + BorrowKind::Mut { .. } => { + let wk = WriteKind::MutableBorrow(bk); + if bk.allows_two_phase_borrow() { + (Deep, Reservation(wk)) + } else { + (Deep, Write(wk)) + } + } + }; + + self.access_place(location, place, access_kind, LocalMutationIsAllowed::No); + } + + &Rvalue::RawPtr(kind, place) => { + let access_kind = match kind { + RawPtrKind::Mut => ( + Deep, + Write(WriteKind::MutableBorrow(BorrowKind::Mut { + kind: MutBorrowKind::Default, + })), + ), + RawPtrKind::Const => (Deep, Read(ReadKind::Borrow(BorrowKind::Shared))), + RawPtrKind::FakeForPtrMetadata => { + (Shallow(Some(ArtificialField::ArrayLength)), Read(ReadKind::Copy)) + } + }; + + self.access_place(location, place, access_kind, LocalMutationIsAllowed::No); + } + + Rvalue::ThreadLocalRef(_) => {} + + Rvalue::Use(operand) + | Rvalue::Repeat(operand, _) + | Rvalue::UnaryOp(_ /*un_op*/, operand) + | Rvalue::Cast(_ /*cast_kind*/, operand, _ /*ty*/) + | Rvalue::ShallowInitBox(operand, _ /*ty*/) => self.consume_operand(location, operand), + + &Rvalue::CopyForDeref(place) => { + let op = &Operand::Copy(place); + self.consume_operand(location, op); + } + + &(Rvalue::Len(place) | Rvalue::Discriminant(place)) => { + let af = match rvalue { + Rvalue::Len(..) => Some(ArtificialField::ArrayLength), + Rvalue::Discriminant(..) => None, + _ => unreachable!(), + }; + self.access_place( + location, + place, + (Shallow(af), Read(ReadKind::Copy)), + LocalMutationIsAllowed::No, + ); + } + + Rvalue::BinaryOp(_bin_op, box (operand1, operand2)) => { + self.consume_operand(location, operand1); + self.consume_operand(location, operand2); + } + + Rvalue::NullaryOp(_op, _ty) => {} + + Rvalue::Aggregate(_, operands) => { + for operand in operands { + self.consume_operand(location, operand); + } + } + + Rvalue::WrapUnsafeBinder(op, _) => { + self.consume_operand(location, op); + } + } + } + + /// Simulates an access to a place. + fn access_place( + &mut self, + location: Location, + place: Place<'tcx>, + kind: (AccessDepth, ReadOrWrite), + _is_local_mutation_allowed: LocalMutationIsAllowed, + ) { + let (sd, rw) = kind; + // note: not doing check_access_permissions checks because they don't generate invalidates + self.check_access_for_conflict(location, place, sd, rw); + } + + fn check_access_for_conflict( + &mut self, + location: Location, + place: Place<'tcx>, + sd: AccessDepth, + rw: ReadOrWrite, + ) { + each_borrow_involving_path( + self, + self.tcx, + self.body, + (sd, place), + self.borrow_set, + |_| true, + |this, borrow_index, borrow| { + match (rw, borrow.kind) { + // Obviously an activation is compatible with its own + // reservation (or even prior activating uses of same + // borrow); so don't check if they interfere. + // + // NOTE: *reservations* do conflict with themselves; + // thus aren't injecting unsoundness w/ this check.) + (Activation(_, activating), _) if activating == borrow_index => { + // Activating a borrow doesn't generate any invalidations, since we + // have already taken the reservation + } + + (Read(_), BorrowKind::Fake(_) | BorrowKind::Shared) + | ( + Read(ReadKind::Borrow(BorrowKind::Fake(FakeBorrowKind::Shallow))), + BorrowKind::Mut { .. }, + ) => { + // Reads don't invalidate shared or shallow borrows + } + + (Read(_), BorrowKind::Mut { .. }) => { + // Reading from mere reservations of mutable-borrows is OK. + if !is_active(this.dominators, borrow, location) { + // If the borrow isn't active yet, reads don't invalidate it + assert!(borrow.kind.allows_two_phase_borrow()); + return ControlFlow::Continue(()); + } + + // Unique and mutable borrows are invalidated by reads from any + // involved path + this.emit_loan_invalidated_at(borrow_index, location); + } + + (Reservation(_) | Activation(_, _) | Write(_), _) => { + // unique or mutable borrows are invalidated by writes. + // Reservations count as writes since we need to check + // that activating the borrow will be OK + // FIXME(bob_twinkles) is this actually the right thing to do? + this.emit_loan_invalidated_at(borrow_index, location); + } + } + ControlFlow::Continue(()) + }, + ); + } + + /// Generates a new `loan_invalidated_at(L, B)` fact. + fn emit_loan_invalidated_at(&mut self, b: BorrowIndex, l: Location) { + self.loan_invalidated_at[b].push(l); + } + + fn check_activations(&mut self, location: Location) { + // Two-phase borrow support: For each activation that is newly + // generated at this statement, check if it interferes with + // another borrow. + for &borrow_index in self.borrow_set.activations_at_location(location) { + let borrow = &self.borrow_set[borrow_index]; + + // only mutable borrows should be 2-phase + assert!(match borrow.kind { + BorrowKind::Shared | BorrowKind::Fake(_) => false, + BorrowKind::Mut { .. } => true, + }); + + self.access_place( + location, + borrow.borrowed_place, + (Deep, Activation(WriteKind::MutableBorrow(borrow.kind), borrow_index)), + LocalMutationIsAllowed::No, + ); + + // We do not need to call `check_if_path_or_subpath_is_moved` + // again, as we already called it when we made the + // initial reservation. + } + } +} diff --git a/compiler/rustc_borrowck/src/polonius/horatio/location_sensitive.rs b/compiler/rustc_borrowck/src/polonius/horatio/location_sensitive.rs new file mode 100644 index 0000000000000..d813f072d66da --- /dev/null +++ b/compiler/rustc_borrowck/src/polonius/horatio/location_sensitive.rs @@ -0,0 +1,340 @@ +use rustc_data_structures::fx::FxHashMap; +use rustc_index::bit_set::DenseBitSet; +use rustc_middle::mir::{BasicBlockData, Location}; + +use super::constraints::TimeTravellingRegions; +use super::{ + BorrowContext, KillsCache, PoloniusBlock, PoloniusContext, is_killed, my_println, + remove_dead_regions, +}; +use crate::RegionVid; + +/// The main struct for the location-sensitive analysis. +/// +/// Whenever the location-insensitive analysis (NLL) fails to prove that a loan is not active at a +/// given point, and we are about to emit an error, this location-sensitive analysis is triggered to +/// compute a definitive answer to the question: "Is this loan truly active at this location?" +/// +/// The computation traverses the relevant parts of the CFG until a conclusive answer can be +/// determined, then pauses. The same instance of this struct can be reused to check whether the +/// loan is active at another point, for the same loan. +pub(super) struct LocationSensitiveAnalysis { + /// All the nodes in the location sensitive graph. This graph is a subgraph of the CFG. + pub nodes: FxHashMap, + + /// Whether the computation is finished. + /// + /// If this is `true`, the loan's activeness has already been computed for all relevant + /// locations, and it is sufficient to query the [Self::nodes] map to determine whether the loan + /// is active at a specific location. + pub is_finished: bool, + + /// A stack of nodes that should be checked next. + primary_stack: Vec, + + /// A secondary stack. This will only be popped when the primary stack is empty. + secondary_stack: Vec, +} + +/// A node in the location-sensitive analysis. +pub(super) struct Node { + /// The set of regions associated with the loan at this location. + /// + /// This set may grow on subsequent visits to this node, but it will never shrink. If it is + /// empty, the traversal should not proceed to this node's neighbours. + associated_regions: DenseBitSet, + + /// The regions that were added to [Self::associated_regions] last time this node was added to + /// the stack. + /// + /// This is only for optimization purposes, we don't want to check the regions that have already + /// been checked before. + added_regions: Option>, + + /// Whether this location is reachable by forward edges from the loan's introduction point in + /// the localized constraint graph. + // FIXME: This comment seems strange. + reachable_by_loan: bool, + + /// Whether the loan is active at this point. + pub is_active: bool, + + /// Whether this node has been added to the stack for processing. + added_to_stack: bool, +} + +impl LocationSensitiveAnalysis { + pub(super) fn new(bcx: BorrowContext<'_, '_, '_>) -> Self { + // Put the loan's initial region in a set. + let mut initial_region_set = DenseBitSet::new_empty(bcx.pcx.regioncx.num_regions()); + initial_region_set.insert(bcx.borrow.region); + + let mut nodes = FxHashMap::default(); + // Add the node at the loan's reserve location. + nodes.insert( + bcx.borrow.reserve_location, + Node { + associated_regions: DenseBitSet::new_empty(bcx.pcx.regioncx.num_regions()), + added_regions: Some(initial_region_set), + reachable_by_loan: false, + is_active: false, + added_to_stack: true, + }, + ); + + Self { + primary_stack: vec![bcx.borrow.reserve_location], + secondary_stack: vec![], + nodes, + is_finished: false, + } + } + + /// Compute the necessary nodes to conclude if a loan is active at `target_location`. + #[inline(never)] // FIXME: Remove this. + pub(super) fn compute( + &mut self, + bcx: BorrowContext<'_, '_, '_>, + kills_cache: &mut KillsCache, + target_location: Location, + live_paths: DenseBitSet, + ) -> bool { + my_println!("Checking {:?} at {:?}", bcx.borrow_idx, target_location); + debug_assert!( + !self.is_finished, + "If the location sensitive analysis is finished you should just query `LocationInsensitiveAnalysis::nodes` instead." + ); + + // Pop a node from the stack until it is empty. + while let Some(location) = self.primary_stack.pop().or_else(|| self.secondary_stack.pop()) { + let point = bcx.pcx.location_map.point_from_location(location); + let block_data = &bcx.pcx.body[location.block]; + + // Debugging: Print the current location and statement/expression. + if let Some(stmt) = block_data.statements.get(location.statement_index) { + my_println!(" {:?}: {:?}", location, stmt); + } else { + my_println!(" {:?}: {:?}", location, block_data.terminator().kind); + } + + // Fetch the current node. + let Node { + associated_regions, + added_regions, + reachable_by_loan, + is_active, + added_to_stack, + } = self.nodes.get_mut(&location).unwrap(); + let reachable_by_loan = *reachable_by_loan; // Make copy. + + debug_assert!(*added_to_stack); + *added_to_stack = false; + + let time_travelling_regions = if let Some(mut added_regions) = added_regions.take() { + debug_assert!( + !added_regions.is_empty(), + "added_regions should never be empty, in that case it should be `None`." + ); + debug_assert!( + added_regions.iter().all(|r| !associated_regions.contains(r)), + "`added_regions` and `associated_regions` should be disjunct." + ); + + // Traverse the location-sensitive constraint graph at this point, adding any + // regions reachable from the ones in `added_regions`. All time-travelling regions + // encountered will be returned and stored in this variable. + let time_travelling_regions = bcx + .pcx + .cache() + .constraints + .add_dependent_regions_at_point(point, &mut added_regions); + + // FIXME: Just debugging of the time-travelling regions. + if let Some(tf) = &time_travelling_regions.to_next_loc { + my_println!(" Forward time travellers: {:?}", tf); + } + if let Some(tf) = &time_travelling_regions.to_prev_stmt { + my_println!(" Backward time travellers: {:?}", tf); + } + if let Some(x) = &time_travelling_regions.to_predecessor_blocks { + my_println!(" To preceeding blocks: {:?}", x); + } + if let Some(x) = &time_travelling_regions.to_successor_blocks { + my_println!(" To succeeding blocks: {:?}", x); + } + + // Incorporate the added regions into `associated_regions`. + associated_regions.union(&added_regions); + my_println!(" Regions: {:?}", associated_regions); + + Some(time_travelling_regions) + } else { + my_println!("Nothing new here."); + // FIXME: This should be unnecessary if we don't track kills. + if reachable_by_loan { + // FIXME: This is just a hack. + let mut associated_regions = associated_regions.clone(); + remove_dead_regions(bcx.pcx, location, &mut associated_regions); + if associated_regions.is_empty() { + my_println!(" Loan killed."); + continue; + } + } else { + continue; + } + + None + }; + + // Remove the dead regions from `associated_regions`. + let mut associated_regions = associated_regions.clone(); + remove_dead_regions(bcx.pcx, location, &mut associated_regions); + + // If `associated_regions` is not empty and the node is reachable from the loan's + // introduction point in the location-sensitive graph, then the loan is active. + if reachable_by_loan && !associated_regions.is_empty() { + *is_active = true; + my_println!(" In scope at {location:?}"); + } + + // Check if the loan is killed. + let is_killed = is_killed(bcx, kills_cache, location); + + // If the loan is killed at this location, and this location is reachable from the + // loan's reserve location, then we should not add any of this node’s neighbours to the + // stack. + if is_killed && bcx.pcx.is_predecessor(bcx.borrow.reserve_location, location) { + continue; + } + + let successor_reachable_by_loan = + !is_killed && reachable_by_loan || location == bcx.borrow.reserve_location; + + // Necessary to make the borrow checker happy. + let is_active = *is_active; + + // Visit all the neighbours of this node—that is, both predecessors and successors—and + // potentially add more associated regions to them. + visit_adjacent_locations( + bcx.pcx, + block_data, + location, + time_travelling_regions, + |new_location, time_travellers, is_forward| { + // Get or add a new node at this location. + let new_node = self.nodes.entry(new_location).or_insert_with(|| Node { + associated_regions: DenseBitSet::new_empty(bcx.pcx.regioncx.num_regions()), + added_regions: None, + reachable_by_loan: false, + is_active: false, + added_to_stack: false, + }); + + // Keep track of whether `new_node` has changed. + let mut new_node_changed = false; + + // If we are going forwards, we need to propagate reachability for the loan. + if is_forward && successor_reachable_by_loan && !new_node.reachable_by_loan { + new_node.reachable_by_loan = true; + // `reachable_by_loan` was `false` before on `new_node` but has now been + // changed to `true`. + new_node_changed = true; + } + + // Check if any regions should be added to `new_node`. + let mut added_regions = associated_regions.clone(); + if !is_forward { + // FIXME: Now we only ignore the universal regions when going backwards. Actually we might need to record the variance of all regions, but the tests seem to pass in this way. + added_regions.subtract(&bcx.pcx.cache().universal_regions); + } + + remove_dead_regions(bcx.pcx, new_location, &mut added_regions); + + if let Some(time_travellers) = time_travellers { + added_regions.union(time_travellers); + } + + // Subtract the already associated regions from `added_regions` so they become + // disjunct. + added_regions.subtract(&new_node.associated_regions); + + if !added_regions.is_empty() { + if let Some(already_added_regions) = new_node.added_regions.as_mut() { + already_added_regions.union(&added_regions); + } else { + new_node.added_regions = Some(added_regions); + } + new_node_changed = true; + } + + if new_node_changed && !new_node.added_to_stack { + if !is_forward + || live_paths.contains(PoloniusBlock::from_location(bcx, new_location)) + { + self.primary_stack.push(new_location); + } else { + self.secondary_stack.push(new_location); + } + new_node.added_to_stack = true; + } + }, + ); + + if is_active && location == target_location { + return true; + } + } + + // The stack is empty so the location sensitive analysis is complete. + self.is_finished = true; + + // Fetch the result. + self.nodes.get(&target_location).is_some_and(|x| x.is_active) + } +} + +/// This is a very specific function used in [`LocationSensitiveAnalysis::compute()`] to visit all +/// +/// predecessors and successors of a node. One could argue that it shouldn’t be a separate function +/// and should just be hardcoded, but that led to a ton of repetitive code. +#[inline] +fn visit_adjacent_locations( + pcx: &PoloniusContext<'_, '_>, + block_data: &BasicBlockData<'_>, + location: Location, + maybe_time_travellers: Option, + mut op: impl FnMut(Location, Option<&DenseBitSet>, bool), +) { + // Forwards: + if location.statement_index < block_data.statements.len() { + let successor_location = location.successor_within_block(); + let time_travellers = maybe_time_travellers.as_ref().and_then(|t| t.to_next_loc.as_ref()); + op(successor_location, time_travellers, true); + } else { + for successor_block in block_data.terminator().successors() { + let successor_location = Location { block: successor_block, statement_index: 0 }; + let time_travellers = maybe_time_travellers + .as_ref() + .and_then(|t| t.to_successor_blocks.as_ref().and_then(|x| x.row(successor_block))); + op(successor_location, time_travellers, true); + } + } + + // Backwards: + if location.statement_index > 0 { + let predecessor_location = location.predecessor_within_block(); + let time_travellers = maybe_time_travellers.as_ref().and_then(|t| t.to_prev_stmt.as_ref()); + op(predecessor_location, time_travellers, false); + } else { + for &predecessor_block in &pcx.body.basic_blocks.predecessors()[location.block] { + let predecessor_location = Location { + block: predecessor_block, + statement_index: pcx.body[predecessor_block].statements.len(), + }; + let time_travellers = maybe_time_travellers.as_ref().and_then(|t| { + t.to_predecessor_blocks.as_ref().and_then(|x| x.row(predecessor_block)) + }); + op(predecessor_location, time_travellers, false); + } + } +} diff --git a/compiler/rustc_borrowck/src/polonius/horatio/mod.rs b/compiler/rustc_borrowck/src/polonius/horatio/mod.rs new file mode 100644 index 0000000000000..d2478c1b7854a --- /dev/null +++ b/compiler/rustc_borrowck/src/polonius/horatio/mod.rs @@ -0,0 +1,662 @@ +#![allow(dead_code)] +#![deny(unused_imports)] +mod constraints; +mod live_region_variance; +mod loan_invalidations; +mod location_sensitive; +mod polonius_block; +use std::cell::OnceCell; +use std::sync::LazyLock; + +use constraints::Constraints; +use location_sensitive::LocationSensitiveAnalysis; +use polonius_block::PoloniusBlock; +use rustc_index::IndexVec; +use rustc_index::bit_set::DenseBitSet; +use rustc_middle::mir::{self, BasicBlock, Body, Local, Location, Place, Statement, Terminator}; +use rustc_middle::ty::TyCtxt; +use rustc_mir_dataflow::points::DenseLocationMap; +use smallvec::{SmallVec, smallvec}; + +use super::ConstraintDirection; +use crate::{ + BorrowData, BorrowIndex, BorrowSet, PlaceConflictBias, PlaceExt, RegionInferenceContext, + RegionVid, places_conflict, +}; + +/// This toggles the `my_println!` and `my_print!` macros. Those macros are used here and there to +/// print tracing information about Polonius. +pub(crate) const MY_DEBUG_PRINTS: LazyLock = LazyLock::new(|| { + matches!(std::env::var("POLONIUS_TRACING").as_ref().map(String::as_str), Ok("1")) +}); + +macro_rules! my_println { + ($($x:expr),*) => { + if *crate::polonius::horatio::MY_DEBUG_PRINTS { + println!($($x,)*); + } + }; +} +pub(crate) use my_println; + +macro_rules! my_print { + ($($x:expr),*) => { + if *crate::polonius::horatio::MY_DEBUG_PRINTS { + print!($($x,)*); + } + }; +} +pub(crate) use my_print; + +/// A cache remembering whether a loan is killed at a block. +type KillsCache = IndexVec>; + +/// The main struct of Polonius which computes if loans are active at certain locations. +pub(crate) struct Polonius<'a, 'tcx> { + pub pcx: PoloniusContext<'a, 'tcx>, + borrows: IndexVec>, +} + +pub(crate) struct PoloniusContext<'a, 'tcx> { + /// A cache that is only computed if we need the location sensitive analysis. + cache: OnceCell>, + + /// For every block, we store a set of all proceeding blocks. + /// + /// ```text + /// a + /// / \ + /// b c + /// \ / + /// d + /// ``` + /// In this case we have: + /// ```text + /// a: {} + /// b: {a} + /// c: {a} + /// d: {a, b, c} + /// ``` + transitive_predecessors: IndexVec>, + + /// For every block we store the immediate predecessors. + /// + /// ```text + /// a + /// / \ + /// b c + /// \ / + /// d + /// ``` + /// In this case we have: + /// ```text + /// a: {} + /// b: {a} + /// c: {a} + /// d: {b, c} + /// ``` + // FIXME: This is equivalent to `BasicBlocks.predecessors` but uses bit sets instead of + // `SmallVec`. Maybe that should be replaced by this. + adjacent_predecessors: IndexVec>, + + /// Only computed for diagnostics: The regions that outlive free regions are used to distinguish + /// relevant live locals from boring locals. A boring local is one whose type contains only such + /// regions. Polonius currently has more boring locals than NLLs so we record the latter to use + /// in errors and diagnostics, to focus on the locals we consider relevant and match NLL + /// diagnostics. + boring_nll_locals: OnceCell>, + + tcx: TyCtxt<'tcx>, + regioncx: &'a RegionInferenceContext<'tcx>, + body: &'a Body<'tcx>, + location_map: &'a DenseLocationMap, + borrow_set: &'a BorrowSet<'tcx>, +} + +struct Cache<'a, 'tcx> { + /// All universal regions. + universal_regions: DenseBitSet, + + /// All outlives constraints. + constraints: Constraints<'a, 'tcx>, +} + +/// A borrow with some context. +/// +/// It turns out that the index operation on [`BorrowSet`] takes a fair bit of time if executed many +/// many times. So we want to keep a reference to the [`BorrowData`] as well. The problem is that +/// sometimes a [`BorrowIndex`] is required, sometimes a `&[BorrowData]`, and sometimes we need a +/// reference to the [`Body`] or something else. So we bundles all this information in one struct. +#[derive(Copy, Clone)] +struct BorrowContext<'a, 'b, 'tcx> { + pcx: &'a PoloniusContext<'b, 'tcx>, + borrow_idx: BorrowIndex, + borrow: &'a BorrowData<'tcx>, +} + +/// Data used when computing when a loan is active. +enum PoloniusBorrowData { + /// This borrow should be ignored. + Ignored, + + Data { + /// A cache of kills for this loan. + kills_cache: KillsCache, + scope_computation: Option, + }, +} + +/// Information of when/if a loan is killed at a block. +#[derive(Debug, Copy, Clone)] +enum KillAtBlock { + /// The loan is not killed at this block. + NotKilled, + + /// The loan is killed. + Killed { statement_index: usize }, +} +use KillAtBlock::*; + +impl<'a, 'tcx> PoloniusContext<'a, 'tcx> { + pub(crate) fn new( + tcx: TyCtxt<'tcx>, + regioncx: &'a RegionInferenceContext<'tcx>, + body: &'a Body<'tcx>, + location_map: &'a DenseLocationMap, + borrow_set: &'a BorrowSet<'tcx>, + ) -> Self { + // Compute `transitive_predecessors` and `adjacent_predecessors`. + let mut transitive_predecessors = IndexVec::from_elem_n( + DenseBitSet::new_empty(body.basic_blocks.len()), + body.basic_blocks.len(), + ); + let mut adjacent_predecessors = transitive_predecessors.clone(); + // The stack is initially a reversed postorder traversal of the CFG. However, we might add + // add blocks again to the stack if we have loops. + let mut stack = + body.basic_blocks.reverse_postorder().iter().rev().copied().collect::>(); + // We keep track of all blocks that are currently not in the stack. + let mut not_in_stack = DenseBitSet::new_empty(body.basic_blocks.len()); + while let Some(block) = stack.pop() { + not_in_stack.insert(block); + + // Loop over all successors to the block and add `block` to their predecessors. + for succ_block in body.basic_blocks[block].terminator().successors() { + // Keep track of whether the transitive predecessors of `succ_block` has changed. + let mut changed = false; + + // Insert `block` in `succ_block`s predecessors. + if adjacent_predecessors[succ_block].insert(block) { + // Remember that `adjacent_predecessors` is a subset of + // `transitive_predecessors`. + changed |= transitive_predecessors[succ_block].insert(block); + } + + // Add all transitive predecessors of `block` to the transitive predecessors of + // `succ_block`. + if block != succ_block { + let (blocks_predecessors, succ_blocks_predecessors) = + transitive_predecessors.pick2_mut(block, succ_block); + changed |= succ_blocks_predecessors.union(blocks_predecessors); + + // Check if the `succ_block`s transitive predecessors changed. If so, we may + // need to add it to the stack again. + if changed && not_in_stack.remove(succ_block) { + stack.push(succ_block); + } + } + } + + debug_assert!(transitive_predecessors[block].superset(&adjacent_predecessors[block])); + } + + Self { + cache: OnceCell::new(), + transitive_predecessors, + adjacent_predecessors, + boring_nll_locals: OnceCell::new(), + tcx, + regioncx, + body, + location_map, + borrow_set, + } + } + + fn cache(&self) -> &Cache<'a, 'tcx> { + self.cache.get_or_init(|| { + let mut universal_regions = DenseBitSet::new_empty(self.regioncx.num_regions()); + universal_regions + .insert_range(self.regioncx.universal_regions().universal_regions_range()); + + let mut constraints = + Constraints::new(self.tcx, self.regioncx, self.body, self.location_map); + for constraint in self.regioncx.outlives_constraints() { + constraints.add_constraint(&constraint); + } + + Cache { universal_regions, constraints } + }) + } + + fn boring_nll_locals(&self) -> &DenseBitSet { + self.boring_nll_locals.get_or_init(|| { + let mut free_regions = DenseBitSet::new_empty(self.regioncx.num_regions()); + for region in self.regioncx.universal_regions().universal_regions_iter() { + free_regions.insert(region); + } + self.cache().constraints.add_dependent_regions_reversed(&mut free_regions); + + let mut boring_locals = DenseBitSet::new_empty(self.body.local_decls.len()); + for (local, local_decl) in self.body.local_decls.iter_enumerated() { + if self + .tcx + .all_free_regions_meet(&local_decl.ty, |r| free_regions.contains(r.as_var())) + { + boring_locals.insert(local); + } + } + + boring_locals + }) + } + + pub(crate) fn is_boring_local(&self, local: Local) -> bool { + self.boring_nll_locals().contains(local) + } + + /// Returns `true` iff `a` is earlier in the control flow graph than `b`. + #[inline] + fn is_predecessor(&self, a: Location, b: Location) -> bool { + a.block == b.block && a.statement_index < b.statement_index + || self.transitive_predecessors[b.block].contains(a.block) + } +} + +impl<'a, 'b, 'tcx> BorrowContext<'a, 'b, 'tcx> { + /// Construct a new empty set with capacity for [`PoloniusBlock`]s. + fn new_polonius_block_set(self) -> DenseBitSet { + DenseBitSet::new_empty(PoloniusBlock::num_blocks(self)) + } + + fn dependent_regions(&self) -> &DenseBitSet { + self.borrow.dependent_regions.get_or_init(|| { + let mut dependent_regions = DenseBitSet::new_empty(self.pcx.regioncx.num_regions()); + dependent_regions.insert(self.borrow.region); + self.pcx.cache().constraints.add_dependent_regions(&mut dependent_regions); + dependent_regions + }) + } + + fn has_live_region_at(&self, location: Location) -> bool { + self.pcx.regioncx.region_contains(self.borrow.region, location) + } +} + +impl<'a, 'tcx> Polonius<'a, 'tcx> { + pub(crate) fn new( + tcx: TyCtxt<'tcx>, + regioncx: &'a RegionInferenceContext<'tcx>, + body: &'a Body<'tcx>, + location_map: &'a DenseLocationMap, + borrow_set: &'a BorrowSet<'tcx>, + ) -> Self { + Self { + pcx: PoloniusContext::new(tcx, regioncx, body, location_map, borrow_set), + borrows: IndexVec::new(), + } + } + + /// Quick check to check if a loan is active at a certain point in the CFG. + /// + /// If this function returns `false`, we know for sure that the loan is not active at + /// `location`, otherwise it may or may not be active. + /// + /// The purpose of this function is to be really quick. In most cases it will return `false` and + /// no conflict is therefore possible. In the rare situations it returns `true`, the caller + /// should proceed with other more time consuming methods of checking for a conflict and + /// eventually call the [`Polonius::loan_is_active_at`] function which will give a definite answer. + #[inline] + pub(crate) fn loan_maybe_active_at( + &mut self, + borrow_idx: BorrowIndex, + borrow: &BorrowData<'tcx>, + location: Location, + ) -> bool { + // Check if this location can never be reached by the borrow. + if !self.pcx.is_predecessor(borrow.reserve_location(), location) { + return false; + } + + let bcx = BorrowContext { pcx: &self.pcx, borrow_idx, borrow }; + + if !bcx.has_live_region_at(location) { + return false; + } + + true + } + + /// Check if a loan is is active at a point in the CFG. + pub(crate) fn loan_is_active_at( + &mut self, + borrow_idx: BorrowIndex, + borrow: &BorrowData<'tcx>, + location: Location, + ) -> bool { + let maybe_borrow_data = self.borrows.ensure_contains_elem(borrow_idx, || None); + let (kills_cache, scope_computation) = match maybe_borrow_data { + Some(PoloniusBorrowData::Ignored) => return false, + Some(PoloniusBorrowData::Data { scope_computation, kills_cache }) => { + if let Some(scope_computation) = &scope_computation { + // Check if we have already computed an "in scope-value" for location. + if scope_computation.is_finished { + // If the scope computation is finished, it's appropriate to return `false` if no + // node for the location exists. + return scope_computation.nodes.get(&location).is_some_and(|x| x.is_active); + + // If the computation is not finished, we can only be sure if the `in_scope`-field + // has been set to `true` for the relevant node. + } else if scope_computation.nodes.get(&location).is_some_and(|x| x.is_active) { + return true; + } + } + + (kills_cache, scope_computation) + } + None => { + // Check if this borrow is ignored. + if borrow.borrowed_place().ignore_borrow( + self.pcx.tcx, + self.pcx.body, + &self.pcx.borrow_set.locals_state_at_exit, + ) { + *maybe_borrow_data = Some(PoloniusBorrowData::Ignored); + return false; + } + + *maybe_borrow_data = Some(PoloniusBorrowData::Data { + kills_cache: IndexVec::new(), + scope_computation: None, + }); + + let Some(PoloniusBorrowData::Data { kills_cache, scope_computation }) = + maybe_borrow_data + else { + unreachable!() + }; + (kills_cache, scope_computation) + } + }; + + let bcx = BorrowContext { pcx: &self.pcx, borrow_idx, borrow }; + + // Check if the loan is killed anywhere between its reserve location and `location`. + let Some(live_paths) = live_paths(bcx, kills_cache, location) else { + return false; + }; + + if self.pcx.tcx.sess.opts.unstable_opts.polonius.is_next_enabled() { + scope_computation.get_or_insert_with(|| LocationSensitiveAnalysis::new(bcx)).compute( + bcx, + kills_cache, + location, + live_paths, + ) + } else { + true + } + } +} + +/// Returns `true` if the loan is killed at `location`. Note that the kill takes effect at the next +/// statement. +fn is_killed( + bcx: BorrowContext<'_, '_, '_>, + kills_cache: &mut KillsCache, + location: Location, +) -> bool { + let polonius_block = PoloniusBlock::from_location(bcx, location); + + // Check if we already know the answer. + match kills_cache.get(polonius_block) { + Some(Some(Killed { statement_index })) => { + return *statement_index == location.statement_index; + } + Some(Some(NotKilled)) => return false, + Some(None) | None => (), + } + // The answer was not known so we have to compute it ourselfs. + + let is_kill = !bcx.has_live_region_at(location) + || if let Some(stmt) = bcx.pcx.body[location.block].statements.get(location.statement_index) + { + is_killed_at_stmt(bcx, stmt) + } else { + is_killed_at_terminator(bcx, &bcx.pcx.body[location.block].terminator()) + }; + + // If we had a kill at this location, we should add it to the cache. + if is_kill { + *kills_cache.ensure_contains_elem(polonius_block, || None) = + Some(Killed { statement_index: location.statement_index }); + } + + is_kill +} + +/// Calculate when/if a loan goes out of scope for a set of statements in a block. +fn is_killed_at_block( + bcx: BorrowContext<'_, '_, '_>, + kills_cache: &mut KillsCache, + block: PoloniusBlock, +) -> bool { + let res = kills_cache.get_or_insert_with(block, || { + let block_data = &bcx.pcx.body[block.basic_block(bcx)]; + for statement_index in block.first_index(bcx)..=block.last_index(bcx) { + let location = Location { statement_index, block: block.basic_block(bcx) }; + + let is_kill = !bcx.has_live_region_at(location) + || if let Some(stmt) = block_data.statements.get(statement_index) { + is_killed_at_stmt(bcx, stmt) + } else { + is_killed_at_terminator(bcx, &block_data.terminator()) + }; + + if is_kill { + return Killed { statement_index }; + } + } + + NotKilled + }); + + matches!(res, Killed { .. }) +} + +/// Given that the borrow was in scope on entry to this statement, check if it goes out of scope +/// till the next location. +#[inline] +fn is_killed_at_stmt<'tcx>(bcx: BorrowContext<'_, '_, 'tcx>, stmt: &Statement<'tcx>) -> bool { + match &stmt.kind { + mir::StatementKind::Assign(box (lhs, _rhs)) => kill_on_place(bcx, *lhs), + mir::StatementKind::StorageDead(local) => { + bcx.pcx.borrow_set.local_map.get(local).is_some_and(|bs| bs.contains(&bcx.borrow_idx)) + } + _ => false, + } +} + +/// Given that the borrow was in scope on entry to this terminator, check if it goes out of scope +/// till the succeeding blocks. +#[inline] +fn is_killed_at_terminator<'tcx>( + bcx: BorrowContext<'_, '_, 'tcx>, + terminator: &Terminator<'tcx>, +) -> bool { + match &terminator.kind { + // A `Call` terminator's return value can be a local which has borrows, so we need to record + // those as killed as well. + mir::TerminatorKind::Call { destination, .. } => kill_on_place(bcx, *destination), + mir::TerminatorKind::InlineAsm { operands, .. } => operands.iter().any(|op| { + if let mir::InlineAsmOperand::Out { place: Some(place), .. } + | mir::InlineAsmOperand::InOut { out_place: Some(place), .. } = op + { + kill_on_place(bcx, *place) + } else { + false + } + }), + _ => false, + } +} + +#[inline] +fn kill_on_place<'tcx>(bcx: BorrowContext<'_, '_, 'tcx>, place: Place<'tcx>) -> bool { + bcx.pcx.borrow_set.local_map.get(&place.local).is_some_and(|bs| bs.contains(&bcx.borrow_idx)) + && if place.projection.is_empty() { + !bcx.pcx.body.local_decls[place.local].is_ref_to_static() + } else { + places_conflict( + bcx.pcx.tcx, + bcx.pcx.body, + bcx.borrow.borrowed_place, + place, + PlaceConflictBias::NoOverlap, + ) + } +} + +#[inline(never)] // FIXME: Remove this. +fn live_paths( + bcx: BorrowContext<'_, '_, '_>, + kills_cache: &mut KillsCache, + destination: Location, +) -> Option> { + // `destination_block` is the `PoloniusBlock` for `destination`. + let destination_block = PoloniusBlock::from_location(bcx, destination); + + // We begin by checking the relevant statements in `destination_block`. + // FIXME: Is this the most efficient solution? + for statement_index in destination_block.first_index(bcx)..destination.statement_index { + let location = Location { block: destination.block, statement_index }; + if is_killed(bcx, kills_cache, location) { + return None; + } + } + + if destination_block.is_introduction_block(bcx) { + // We are finished. + return Some(bcx.new_polonius_block_set()); + } + + // Traverse all blocks between `reserve_location` and `destination` in the CFG and check for + // kills. If there is no live path from `reserve_location` to `destination`, we no for sure + // that the loan is dead at `destination`. + + // Keep track of all visited `PoloniusBlock`s. + let mut visited = bcx.new_polonius_block_set(); + + // The stack contains `(block, path)` pairs, where `block` is a `PoloniusBlock` and `path` is + // a set of `PoloniusBlock`s making a path from `reserve_location` to `destination_block`. + // In this way we can record the live paths. + let introduction_block = PoloniusBlock::introduction_block(bcx); + let mut stack: SmallVec<[(PoloniusBlock, DenseBitSet); 4]> = + smallvec![(introduction_block, bcx.new_polonius_block_set())]; + visited.insert(introduction_block); + + let mut valid_paths = None; + + while let Some((block, path)) = stack.pop() { + // Check if the loan is killed in this block. + if is_killed_at_block(bcx, kills_cache, block) { + continue; + } + + // Loop through all successors to `block` and follow those that are predecessors to + // `destination.block`. + for successor in block.successors(bcx) { + let successor_bb = successor.basic_block(bcx); + + if successor == destination_block { + // We have reached the destination so let's save this path. + valid_paths.get_or_insert_with(|| bcx.new_polonius_block_set()).union(&path); + + // We continue traversal to record all live paths. + continue; + } + + if !visited.insert(successor) { + continue; + } + + // Check that `successor` is a predecessor of `destination_block`. + // + // Given two `PoloniusBlock`s a and b, then a is a predecessor of b iff + // `a.basic_block()` is a predecessor of `b.basic_block()`, or a is the "before + // introduction block" and b is the "introduction block". + if !bcx.pcx.transitive_predecessors[destination.block].contains(successor_bb) + || destination_block.is_introduction_block(bcx) + && successor.is_before_introduction_block(bcx) + { + // `successor` is not a predecessor of `destination_block`. + continue; + } + + // Push `successor` to `path`. + let mut path = path.clone(); + path.insert(successor); + stack.push((successor, path)); + } + } + + valid_paths +} + +/// Remove dead regions from the set of associated regions. +fn remove_dead_regions( + pcx: &PoloniusContext<'_, '_>, + location: Location, + region_set: &mut DenseBitSet, +) { + for region in region_set.clone().iter() { + if !pcx.regioncx.liveness_constraints().is_live_at(region, location) { + region_set.remove(region); + } + } +} + +/// FIXME: Just for debugging. +pub(crate) fn format_body_with_borrows<'tcx>( + body: &Body<'tcx>, + borrow_set: &BorrowSet<'tcx>, +) -> String { + let mut res = String::default(); + for (block, block_data) in body.basic_blocks.iter_enumerated() { + res += &format!("{:?}:\n", block); + for statement_index in 0..=block_data.statements.len() { + let location = Location { block, statement_index }; + res += &format!(" {}: ", statement_index); + if let Some(stmt) = body[location.block].statements.get(location.statement_index) { + res += &format!("{:?}\n", stmt); + } else { + debug_assert_eq!(location.statement_index, body[location.block].statements.len()); + let terminator = body[location.block].terminator(); + res += &format!("{:?}\n", terminator.kind); + } + + let introduced_borrows = borrow_set + .iter_enumerated() + .filter(|(_, b)| b.reserve_location == location) + .collect::>(); + if !introduced_borrows.is_empty() { + res += " reserved borrows: "; + for (borrow_idx, _) in introduced_borrows { + res += &format!("{:?}, ", borrow_idx); + } + res += "\n" + } + } + } + res +} diff --git a/compiler/rustc_borrowck/src/polonius/horatio/polonius_block.rs b/compiler/rustc_borrowck/src/polonius/horatio/polonius_block.rs new file mode 100644 index 0000000000000..34d843b4bee59 --- /dev/null +++ b/compiler/rustc_borrowck/src/polonius/horatio/polonius_block.rs @@ -0,0 +1,141 @@ +use itertools::Either; +use rustc_middle::mir::{BasicBlock, Location}; + +use super::BorrowContext; + +rustc_index::newtype_index! { + /// A `PoloniusBlock` is a `BasicBlock` which splits the block where a loan is introduced into + /// two blocks. + /// + /// The problem is that we want to record at most one location per block where a loan is killed. + /// But a loan might be killed twice in the block where it is introduced, both before and after + /// the reserve location. So we use an additional index to denote the introduction block up to + /// and including the statement where the loan is introduced. This has the consequence that a + /// `PoloniusBlock` is specific for a given loan. + /// + /// We call the block containing all statements after the reserve location for the + /// "introduction block", and the block containing statements up to and including the reserve + /// location "before introduction block". These names might be bad, but my (Tage's) fantacy + /// struggles to come up with anything better. + /// + /// So if the loan is introduced at `bb2[2]`, `bb2[0..=2]` is the "before introduction block" + /// and `bb2[3..]` is the "introduction block". + /// + /// For a given loan `l` introduced at a basic block `b`, a `PoloniusBlock` is equivalent to a + /// `BasicBlock` with the following exceptions: + /// - `PoloniusBlock::from_u32(b.as_u32())` is `l`'s introduction block. + /// - `PoloniusBlock::from_usize(basic_blocks.len())` is `l`'s "before introduction block". + #[debug_format = "pbb{}"] + pub(super) struct PoloniusBlock {} +} + +impl PoloniusBlock { + /// Converts a [`BasicBlock`] to a [`PoloniusBlock`] assuming this is not the "before + /// introduction block". + #[inline] + pub(super) fn from_basic_block(basic_block: BasicBlock) -> Self { + Self::from_u32(basic_block.as_u32()) + } + + /// Get the "introduction block". I.E the first block where the loan is introduced. + #[inline] + pub(super) fn introduction_block(bcx: BorrowContext<'_, '_, '_>) -> Self { + Self::from_basic_block(bcx.borrow.reserve_location.block) + } + + /// Get the "before introduction block". I.E the block consisting of statements up to and + /// including the loan's reserve location. + #[inline] + pub(super) fn before_introduction_block(bcx: BorrowContext<'_, '_, '_>) -> Self { + Self::from_usize(bcx.pcx.body.basic_blocks.len()) + } + + /// Get the correct block from a loan and a location. + #[inline] + pub(super) fn from_location(bcx: BorrowContext<'_, '_, '_>, location: Location) -> Self { + if location.block == bcx.borrow.reserve_location.block + && location.statement_index <= bcx.borrow.reserve_location.statement_index + { + Self::before_introduction_block(bcx) + } else { + Self::from_basic_block(location.block) + } + } + + /// Returns the number of polonius blocks. THat is, the number of blocks + 1. + #[inline] + pub(super) fn num_blocks(bcx: BorrowContext<'_, '_, '_>) -> usize { + bcx.pcx.body.basic_blocks.len() + 1 + } + + /// Get the [`BasicBlock`] containing this [`PoloniusBlock`]. + #[inline] + pub(super) fn basic_block(self, bcx: BorrowContext<'_, '_, '_>) -> BasicBlock { + if self.as_usize() == bcx.pcx.body.basic_blocks.len() { + bcx.borrow.reserve_location.block + } else { + BasicBlock::from_u32(self.as_u32()) + } + } + + /// Check if this is the "introduction block". I.E the block immediately after the loan has been + /// introduced. + #[inline] + pub(super) fn is_introduction_block(self, bcx: BorrowContext<'_, '_, '_>) -> bool { + self.as_u32() == bcx.borrow.reserve_location.block.as_u32() + } + + /// Check if this is the "before introduction block". I.E the block containing statements up to + /// and including the loan's reserve location. + #[inline] + pub(super) fn is_before_introduction_block(self, bcx: BorrowContext<'_, '_, '_>) -> bool { + self.as_usize() == bcx.pcx.body.basic_blocks.len() + } + + /// Get the index of the first statement in this block. This will be 0 except for the + /// introduction block. + #[inline] + pub(super) fn first_index(self, bcx: BorrowContext<'_, '_, '_>) -> usize { + if self.is_introduction_block(bcx) { + bcx.borrow.reserve_location.statement_index + 1 + } else { + 0 + } + } + + /// Get the last statement index for this block. For all blocks except the "before introduction + /// block", this will point to a terminator, not a statement. + #[inline] + pub(super) fn last_index(self, bcx: BorrowContext<'_, '_, '_>) -> usize { + if !self.is_before_introduction_block(bcx) { + bcx.pcx.body.basic_blocks[self.basic_block(bcx)].statements.len() + } else { + bcx.borrow.reserve_location.statement_index + } + } + + /// Iterate over the successor blocks to this block. + /// + /// Note that this is same as + /// [`Terminator::successors`](rustc_middle::mir::Terminator::successors) except for the "before + /// introduction block" where it is the "introduction block". + #[inline] + pub(super) fn successors( + self, + bcx: BorrowContext<'_, '_, '_>, + ) -> impl DoubleEndedIterator { + if !self.is_before_introduction_block(bcx) { + Either::Left(bcx.pcx.body[self.basic_block(bcx)].terminator().successors().map( + move |bb| { + if bb == bcx.borrow.reserve_location.block { + Self::before_introduction_block(bcx) + } else { + Self::from_basic_block(bb) + } + }, + )) + } else { + Either::Right([Self::introduction_block(bcx)].into_iter()) + } + } +} diff --git a/compiler/rustc_borrowck/src/polonius/liveness_constraints.rs b/compiler/rustc_borrowck/src/polonius/liveness_constraints.rs index 6ab09f731c078..fca40169e7df8 100644 --- a/compiler/rustc_borrowck/src/polonius/liveness_constraints.rs +++ b/compiler/rustc_borrowck/src/polonius/liveness_constraints.rs @@ -13,24 +13,6 @@ use super::{ use crate::region_infer::values::LivenessValues; use crate::universal_regions::UniversalRegions; -impl PoloniusLivenessContext { - /// Record the variance of each region contained within the given value. - pub(crate) fn record_live_region_variance<'tcx>( - &mut self, - tcx: TyCtxt<'tcx>, - universal_regions: &UniversalRegions<'tcx>, - value: impl TypeVisitable> + Relate>, - ) { - let mut extractor = VarianceExtractor { - tcx, - ambient_variance: ty::Variance::Covariant, - directions: &mut self.live_region_variances, - universal_regions, - }; - extractor.relate(value, value).expect("Can't have a type error relating to itself"); - } -} - /// Propagate loans throughout the CFG: for each statement in the MIR, create localized outlives /// constraints for loans that are propagated to the next statements. pub(super) fn create_liveness_constraints<'tcx>( @@ -197,120 +179,3 @@ fn add_liveness_constraint( } } } - -/// Extracts variances for regions contained within types. Follows the same structure as -/// `rustc_infer`'s `Generalizer`: we try to relate a type with itself to track and extract the -/// variances of regions. -struct VarianceExtractor<'a, 'tcx> { - tcx: TyCtxt<'tcx>, - ambient_variance: ty::Variance, - directions: &'a mut BTreeMap, - universal_regions: &'a UniversalRegions<'tcx>, -} - -impl<'tcx> VarianceExtractor<'_, 'tcx> { - fn record_variance(&mut self, region: ty::Region<'tcx>, variance: ty::Variance) { - // We're only interested in the variance of vars and free regions. - // - // Note: even if we currently bail for two cases of unexpected region kinds here, missing - // variance data is not a soundness problem: the regions with missing variance will still be - // present in the constraint graph as they are live, and liveness edges construction has a - // fallback for this case. - // - // FIXME: that being said, we need to investigate these cases better to not ignore regions - // in general. - if region.is_bound() { - // We ignore these because they cannot be turned into the vids we need. - return; - } - - if region.is_erased() { - // These cannot be turned into a vid either, and we also ignore them: the fact that they - // show up here looks like either an issue upstream or a combination with unexpectedly - // continuing compilation too far when we're in a tainted by errors situation. - // - // FIXME: investigate the `generic_const_exprs` test that triggers this issue, - // `ui/const-generics/generic_const_exprs/issue-97047-ice-2.rs` - return; - } - - let direction = match variance { - ty::Covariant => ConstraintDirection::Forward, - ty::Contravariant => ConstraintDirection::Backward, - ty::Invariant => ConstraintDirection::Bidirectional, - ty::Bivariant => { - // We don't add edges for bivariant cases. - return; - } - }; - - let region = self.universal_regions.to_region_vid(region); - self.directions - .entry(region) - .and_modify(|entry| { - // If there's already a recorded direction for this region, we combine the two: - // - combining the same direction is idempotent - // - combining different directions is trivially bidirectional - if entry != &direction { - *entry = ConstraintDirection::Bidirectional; - } - }) - .or_insert(direction); - } -} - -impl<'tcx> TypeRelation> for VarianceExtractor<'_, 'tcx> { - fn cx(&self) -> TyCtxt<'tcx> { - self.tcx - } - - fn relate_with_variance>>( - &mut self, - variance: ty::Variance, - _info: ty::VarianceDiagInfo>, - a: T, - b: T, - ) -> RelateResult<'tcx, T> { - let old_ambient_variance = self.ambient_variance; - self.ambient_variance = self.ambient_variance.xform(variance); - let r = self.relate(a, b)?; - self.ambient_variance = old_ambient_variance; - Ok(r) - } - - fn tys(&mut self, a: Ty<'tcx>, b: Ty<'tcx>) -> RelateResult<'tcx, Ty<'tcx>> { - assert_eq!(a, b); // we are misusing TypeRelation here; both LHS and RHS ought to be == - relate::structurally_relate_tys(self, a, b) - } - - fn regions( - &mut self, - a: ty::Region<'tcx>, - b: ty::Region<'tcx>, - ) -> RelateResult<'tcx, ty::Region<'tcx>> { - assert_eq!(a, b); // we are misusing TypeRelation here; both LHS and RHS ought to be == - self.record_variance(a, self.ambient_variance); - Ok(a) - } - - fn consts( - &mut self, - a: ty::Const<'tcx>, - b: ty::Const<'tcx>, - ) -> RelateResult<'tcx, ty::Const<'tcx>> { - assert_eq!(a, b); // we are misusing TypeRelation here; both LHS and RHS ought to be == - relate::structurally_relate_consts(self, a, b) - } - - fn binders( - &mut self, - a: ty::Binder<'tcx, T>, - _: ty::Binder<'tcx, T>, - ) -> RelateResult<'tcx, ty::Binder<'tcx, T>> - where - T: Relate>, - { - self.relate(a.skip_binder(), a.skip_binder())?; - Ok(a) - } -} diff --git a/compiler/rustc_borrowck/src/polonius/loan_liveness.rs b/compiler/rustc_borrowck/src/polonius/loan_liveness.rs index 5cd265e0db92d..19ee8dd0540d8 100644 --- a/compiler/rustc_borrowck/src/polonius/loan_liveness.rs +++ b/compiler/rustc_borrowck/src/polonius/loan_liveness.rs @@ -8,6 +8,7 @@ use rustc_middle::mir::{ use rustc_middle::ty::{RegionVid, TyCtxt}; use rustc_mir_dataflow::points::PointIndex; +use super::horatio::{my_print, my_println}; use super::{LiveLoans, LocalizedOutlivesConstraintSet}; use crate::constraints::OutlivesConstraint; use crate::dataflow::BorrowIndex; @@ -45,6 +46,7 @@ pub(super) fn compute_loan_liveness<'tcx>( // Compute reachability per loan by traversing each loan's subgraph starting from where it is // introduced. for (loan_idx, loan) in borrow_set.iter_enumerated() { + my_println!("* {:?}:", loan_idx); visited.clear(); stack.clear(); @@ -117,6 +119,11 @@ pub(super) fn compute_loan_liveness<'tcx>( let is_loan_killed = kills.get(¤t_location).is_some_and(|kills| kills.contains(&loan_idx)); + if !is_loan_killed { + my_print!(" {:?}, {:?}: ", current_location, node.region); + } else { + my_print!(" {:?}, {:?} (kill): ", current_location, node.region); + } for succ in graph.outgoing_edges(node) { // If the loan is killed at this point, it is killed _on exit_. But only during // forward traversal. @@ -127,7 +134,9 @@ pub(super) fn compute_loan_liveness<'tcx>( } } stack.push(succ); + my_print!(" ({:?}, {:?});", liveness.location_from_point(succ.point), succ.region); } + my_println!(); } } @@ -194,7 +203,7 @@ impl LocalizedConstraintGraph { } /// Traverses the MIR and collects kills. -fn collect_kills<'tcx>( +pub(super) fn collect_kills<'tcx>( body: &Body<'tcx>, tcx: TyCtxt<'tcx>, borrow_set: &BorrowSet<'tcx>, diff --git a/compiler/rustc_borrowck/src/polonius/mod.rs b/compiler/rustc_borrowck/src/polonius/mod.rs index 142ef8ba28eff..e19aa97f74136 100644 --- a/compiler/rustc_borrowck/src/polonius/mod.rs +++ b/compiler/rustc_borrowck/src/polonius/mod.rs @@ -43,8 +43,11 @@ //! 4) transfer this back to the main borrowck procedure: it handles computing errors and //! diagnostics, debugging and MIR dumping concerns. +#![expect(dead_code, unused_imports)] // FIXME: Most things here are currently not used. + mod constraints; -mod dump; +#[deny(dead_code, unused_imports)] +pub(crate) mod horatio; pub(crate) mod legacy; mod liveness_constraints; mod loan_liveness; @@ -53,14 +56,13 @@ mod typeck_constraints; use std::collections::BTreeMap; use rustc_data_structures::fx::FxHashSet; -use rustc_index::bit_set::SparseBitMatrix; +use rustc_index::bit_set::{DenseBitSet, SparseBitMatrix}; use rustc_index::interval::SparseIntervalMatrix; use rustc_middle::mir::{Body, Local}; use rustc_middle::ty::{RegionVid, TyCtxt}; use rustc_mir_dataflow::points::PointIndex; pub(crate) use self::constraints::*; -pub(crate) use self::dump::dump_polonius_mir; use self::liveness_constraints::create_liveness_constraints; use self::loan_liveness::compute_loan_liveness; use self::typeck_constraints::convert_typeck_constraints; @@ -71,17 +73,12 @@ pub(crate) type LiveLoans = SparseBitMatrix; /// This struct holds the liveness data created during MIR typeck, and which will be used later in /// the process, to compute the polonius localized constraints. -#[derive(Default)] pub(crate) struct PoloniusLivenessContext { - /// The expected edge direction per live region: the kind of directed edge we'll create as - /// liveness constraints depends on the variance of types with respect to each contained region. - live_region_variances: BTreeMap, - /// The regions that outlive free regions are used to distinguish relevant live locals from /// boring locals. A boring local is one whose type contains only such regions. Polonius /// currently has more boring locals than NLLs so we record the latter to use in errors and /// diagnostics, to focus on the locals we consider relevant and match NLL diagnostics. - pub(crate) boring_nll_locals: FxHashSet, + pub(crate) boring_nll_locals: DenseBitSet, } /// This struct holds the data needed to create the Polonius localized constraints. Its data is @@ -98,17 +95,14 @@ pub(crate) struct PoloniusContext { /// This struct holds the data needed by the borrowck error computation and diagnostics. Its data is /// computed from the [PoloniusContext] when computing NLL regions. pub(crate) struct PoloniusDiagnosticsContext { - /// The localized outlives constraints that were computed in the main analysis. - localized_outlives_constraints: LocalizedOutlivesConstraintSet, - /// The liveness data computed during MIR typeck: [PoloniusLivenessContext::boring_nll_locals]. - pub(crate) boring_nll_locals: FxHashSet, + pub(crate) boring_nll_locals: DenseBitSet, } /// The direction a constraint can flow into. Used to create liveness constraints according to /// variance. #[derive(Copy, Clone, PartialEq, Eq, Debug)] -enum ConstraintDirection { +pub(crate) enum ConstraintDirection { /// For covariant cases, we add a forward edge `O at P1 -> O at P2`. Forward, @@ -150,6 +144,7 @@ impl PoloniusContext { /// liveness, to be used by the loan scope and active loans computations. /// /// The constraint data will be used to compute errors and diagnostics. + #[expect(unused_variables)] pub(crate) fn compute_loan_liveness<'tcx>( self, tcx: TyCtxt<'tcx>, @@ -157,9 +152,9 @@ impl PoloniusContext { body: &Body<'tcx>, borrow_set: &BorrowSet<'tcx>, ) -> PoloniusDiagnosticsContext { - let PoloniusLivenessContext { live_region_variances, boring_nll_locals } = - self.liveness_context; + let PoloniusLivenessContext { boring_nll_locals } = self.liveness_context; + /* Deactivates old Polonius let mut localized_outlives_constraints = LocalizedOutlivesConstraintSet::default(); convert_typeck_constraints( tcx, @@ -190,7 +185,8 @@ impl PoloniusContext { &localized_outlives_constraints, ); regioncx.record_live_loans(live_loans); + */ - PoloniusDiagnosticsContext { localized_outlives_constraints, boring_nll_locals } + PoloniusDiagnosticsContext { boring_nll_locals } } } diff --git a/compiler/rustc_borrowck/src/region_infer/mod.rs b/compiler/rustc_borrowck/src/region_infer/mod.rs index b4ff3d66f3d5b..59659963835f7 100644 --- a/compiler/rustc_borrowck/src/region_infer/mod.rs +++ b/compiler/rustc_borrowck/src/region_infer/mod.rs @@ -223,6 +223,9 @@ pub struct RegionInferenceContext<'tcx> { /// Information about how the universally quantified regions in /// scope on this function relate to one another. universal_region_relations: Frozen>, + + /// FIXME: Should these really go here? + pub(crate) location_map: Rc, } /// Each time that `apply_member_constraint` is successful, it appends @@ -484,8 +487,11 @@ impl<'tcx> RegionInferenceContext<'tcx> { sccs_info(infcx, &constraint_sccs); } - let mut scc_values = - RegionValues::new(location_map, universal_regions.len(), placeholder_indices); + let mut scc_values = RegionValues::new( + Rc::clone(&location_map), + universal_regions.len(), + placeholder_indices, + ); for region in liveness_constraints.regions() { let scc = constraint_sccs.scc(region); @@ -509,6 +515,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { scc_values, type_tests, universal_region_relations, + location_map, }; result.init_free_and_bound_regions(); @@ -601,6 +608,12 @@ impl<'tcx> RegionInferenceContext<'tcx> { self.definitions.indices() } + /// Returns the total number of regions. + #[inline] + pub fn num_regions(&self) -> usize { + self.definitions.len() + } + /// Given a universal region in scope on the MIR, returns the /// corresponding index. /// @@ -2255,6 +2268,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// When using `-Zpolonius=next`, records the given live loans for the loan scopes and active /// loans dataflow computations. + #[expect(dead_code)] // FIXME: Maybe remove this function? pub(crate) fn record_live_loans(&mut self, live_loans: LiveLoans) { self.liveness_constraints.record_live_loans(live_loans); } diff --git a/compiler/rustc_borrowck/src/region_infer/values.rs b/compiler/rustc_borrowck/src/region_infer/values.rs index f1427218cdb02..df56350b5bca4 100644 --- a/compiler/rustc_borrowck/src/region_infer/values.rs +++ b/compiler/rustc_borrowck/src/region_infer/values.rs @@ -79,14 +79,6 @@ impl LivenessValues { } } - /// Returns the liveness matrix of points where each region is live. Panics if the liveness - /// values have been created without any per-point data (that is, for promoteds). - pub(crate) fn points(&self) -> &SparseIntervalMatrix { - self.points - .as_ref() - .expect("this `LivenessValues` wasn't created using `with_specific_points`") - } - /// Iterate through each region that has a value in this set. pub(crate) fn regions(&self) -> impl Iterator { self.points.as_ref().expect("use with_specific_points").rows() diff --git a/compiler/rustc_borrowck/src/type_check/liveness/local_use_map.rs b/compiler/rustc_borrowck/src/type_check/liveness/local_use_map.rs index 9591da83708b7..8a26b46d1e535 100644 --- a/compiler/rustc_borrowck/src/type_check/liveness/local_use_map.rs +++ b/compiler/rustc_borrowck/src/type_check/liveness/local_use_map.rs @@ -1,4 +1,5 @@ use rustc_index::IndexVec; +use rustc_index::bit_set::DenseBitSet; use rustc_middle::mir::visit::{PlaceContext, Visitor}; use rustc_middle::mir::{Body, Local, Location}; use rustc_mir_dataflow::points::{DenseLocationMap, PointIndex}; @@ -81,7 +82,7 @@ impl<'a> Iterator for AppearancesIter<'a> { impl LocalUseMap { pub(crate) fn build( - live_locals: &[Local], + live_locals: &DenseBitSet, location_map: &DenseLocationMap, body: &Body<'_>, ) -> Self { @@ -99,7 +100,7 @@ impl LocalUseMap { let mut locals_with_use_data: IndexVec = IndexVec::from_elem(false, &body.local_decls); - live_locals.iter().for_each(|&local| locals_with_use_data[local] = true); + live_locals.iter().for_each(|local| locals_with_use_data[local] = true); LocalUseMapBuild { local_use_map: &mut local_use_map, location_map, locals_with_use_data } .visit_body(body); diff --git a/compiler/rustc_borrowck/src/type_check/liveness/mod.rs b/compiler/rustc_borrowck/src/type_check/liveness/mod.rs index b7a21cf48c8f7..c9118a17ca0e0 100644 --- a/compiler/rustc_borrowck/src/type_check/liveness/mod.rs +++ b/compiler/rustc_borrowck/src/type_check/liveness/mod.rs @@ -1,5 +1,5 @@ -use itertools::{Either, Itertools}; use rustc_data_structures::fx::FxHashSet; +use rustc_index::bit_set::DenseBitSet; use rustc_middle::mir::visit::{TyContext, Visitor}; use rustc_middle::mir::{Body, Local, Location, SourceInfo}; use rustc_middle::span_bug; @@ -13,7 +13,6 @@ use tracing::debug; use super::TypeChecker; use crate::constraints::OutlivesConstraintSet; -use crate::polonius::PoloniusLivenessContext; use crate::region_infer::values::LivenessValues; use crate::universal_regions::UniversalRegions; @@ -36,37 +35,26 @@ pub(super) fn generate<'a, 'tcx>( ) { debug!("liveness::generate"); - let mut free_regions = regions_that_outlive_free_regions( - typeck.infcx.num_region_vars(), - &typeck.universal_regions, - &typeck.constraints.outlives_constraints, - ); - - // NLLs can avoid computing some liveness data here because its constraints are - // location-insensitive, but that doesn't work in polonius: locals whose type contains a region - // that outlives a free region are not necessarily live everywhere in a flow-sensitive setting, - // unlike NLLs. - // We do record these regions in the polonius context, since they're used to differentiate - // relevant and boring locals, which is a key distinction used later in diagnostics. - if typeck.tcx().sess.opts.unstable_opts.polonius.is_next_enabled() { - let (_, boring_locals) = - compute_relevant_live_locals(typeck.tcx(), &free_regions, typeck.body); - typeck.polonius_liveness.as_mut().unwrap().boring_nll_locals = - boring_locals.into_iter().collect(); - free_regions = typeck.universal_regions.universal_regions_iter().collect(); - } - let (relevant_live_locals, boring_locals) = - compute_relevant_live_locals(typeck.tcx(), &free_regions, typeck.body); + let relevant_live_locals = if typeck.tcx().sess.opts.unstable_opts.polonius.is_next_enabled() { + compute_relevant_live_locals(typeck.tcx(), typeck.body, |r| { + typeck.universal_regions.is_universal_region(r) + }) + } else { + let free_regions = regions_that_outlive_free_regions( + typeck.infcx.num_region_vars(), + &typeck.universal_regions, + &typeck.constraints.outlives_constraints, + ); + compute_relevant_live_locals(typeck.tcx(), typeck.body, |r| free_regions.contains(&r)) + }; - trace::trace(typeck, location_map, flow_inits, move_data, relevant_live_locals, boring_locals); + trace::trace(typeck, location_map, flow_inits, move_data, relevant_live_locals); // Mark regions that should be live where they appear within rvalues or within a call: like // args, regions, and types. record_regular_live_regions( typeck.tcx(), &mut typeck.constraints.liveness_constraints, - &typeck.universal_regions, - &mut typeck.polonius_liveness, typeck.body, ); } @@ -78,23 +66,17 @@ pub(super) fn generate<'a, 'tcx>( // region (i.e., where `R` may be valid for just a subset of the fn body). fn compute_relevant_live_locals<'tcx>( tcx: TyCtxt<'tcx>, - free_regions: &FxHashSet, body: &Body<'tcx>, -) -> (Vec, Vec) { - let (boring_locals, relevant_live_locals): (Vec<_>, Vec<_>) = - body.local_decls.iter_enumerated().partition_map(|(local, local_decl)| { - if tcx.all_free_regions_meet(&local_decl.ty, |r| free_regions.contains(&r.as_var())) { - Either::Left(local) - } else { - Either::Right(local) - } - }); - - debug!("{} total variables", body.local_decls.len()); - debug!("{} variables need liveness", relevant_live_locals.len()); - debug!("{} regions outlive free regions", free_regions.len()); + mut is_free_region: impl FnMut(RegionVid) -> bool, +) -> DenseBitSet { + let mut relevant_live_locals = DenseBitSet::new_empty(body.local_decls.len()); + for (local, local_decl) in body.local_decls.iter_enumerated() { + if !tcx.all_free_regions_meet(&local_decl.ty, |r| is_free_region(r.as_var())) { + relevant_live_locals.insert(local); + } + } - (relevant_live_locals, boring_locals) + relevant_live_locals } /// Computes all regions that are (currently) known to outlive free @@ -142,12 +124,9 @@ fn regions_that_outlive_free_regions<'tcx>( fn record_regular_live_regions<'tcx>( tcx: TyCtxt<'tcx>, liveness_constraints: &mut LivenessValues, - universal_regions: &UniversalRegions<'tcx>, - polonius_liveness: &mut Option, body: &Body<'tcx>, ) { - let mut visitor = - LiveVariablesVisitor { tcx, liveness_constraints, universal_regions, polonius_liveness }; + let mut visitor = LiveVariablesVisitor { tcx, liveness_constraints }; for (bb, data) in body.basic_blocks.iter_enumerated() { visitor.visit_basic_block_data(bb, data); } @@ -157,8 +136,6 @@ fn record_regular_live_regions<'tcx>( struct LiveVariablesVisitor<'a, 'tcx> { tcx: TyCtxt<'tcx>, liveness_constraints: &'a mut LivenessValues, - universal_regions: &'a UniversalRegions<'tcx>, - polonius_liveness: &'a mut Option, } impl<'a, 'tcx> Visitor<'tcx> for LiveVariablesVisitor<'a, 'tcx> { @@ -208,10 +185,5 @@ impl<'a, 'tcx> LiveVariablesVisitor<'a, 'tcx> { let live_region_vid = live_region.as_var(); self.liveness_constraints.add_location(live_region_vid, location); }); - - // When using `-Zpolonius=next`, we record the variance of each live region. - if let Some(polonius_liveness) = self.polonius_liveness { - polonius_liveness.record_live_region_variance(self.tcx, self.universal_regions, value); - } } } diff --git a/compiler/rustc_borrowck/src/type_check/liveness/trace.rs b/compiler/rustc_borrowck/src/type_check/liveness/trace.rs index 512288a0f7d85..e67f629f5e724 100644 --- a/compiler/rustc_borrowck/src/type_check/liveness/trace.rs +++ b/compiler/rustc_borrowck/src/type_check/liveness/trace.rs @@ -1,4 +1,4 @@ -use rustc_data_structures::fx::{FxIndexMap, FxIndexSet}; +use rustc_data_structures::fx::FxIndexMap; use rustc_index::bit_set::DenseBitSet; use rustc_index::interval::IntervalSet; use rustc_infer::infer::canonical::QueryRegionConstraints; @@ -42,10 +42,10 @@ pub(super) fn trace<'a, 'tcx>( location_map: &DenseLocationMap, flow_inits: ResultsCursor<'a, 'tcx, MaybeInitializedPlaces<'a, 'tcx>>, move_data: &MoveData<'tcx>, - relevant_live_locals: Vec, - boring_locals: Vec, + relevant_live_locals: DenseBitSet, ) { let local_use_map = &LocalUseMap::build(&relevant_live_locals, location_map, typeck.body); + let body = typeck.body; // typeck will be mutably borrowed soon so we store the body separately. let cx = LivenessContext { typeck, flow_inits, @@ -59,8 +59,10 @@ pub(super) fn trace<'a, 'tcx>( results.add_extra_drop_facts(&relevant_live_locals); - results.compute_for_all_locals(relevant_live_locals); + results.compute_for_all_locals(relevant_live_locals.iter()); + // Boring locals are the locals that are not relevant. + let boring_locals = body.local_decls.indices().filter(|&l| !relevant_live_locals.contains(l)); results.dropck_boring_locals(boring_locals); } @@ -129,7 +131,7 @@ impl<'a, 'typeck, 'b, 'tcx> LivenessResults<'a, 'typeck, 'b, 'tcx> { } } - fn compute_for_all_locals(&mut self, relevant_live_locals: Vec) { + fn compute_for_all_locals(&mut self, relevant_live_locals: impl IntoIterator) { for local in relevant_live_locals { self.reset_local_state(); self.add_defs_for(local); @@ -159,7 +161,7 @@ impl<'a, 'typeck, 'b, 'tcx> LivenessResults<'a, 'typeck, 'b, 'tcx> { /// These are all the locals which do not potentially reference a region local /// to this body. Locals which only reference free regions are always drop-live /// and can therefore safely be dropped. - fn dropck_boring_locals(&mut self, boring_locals: Vec) { + fn dropck_boring_locals(&mut self, boring_locals: impl IntoIterator) { for local in boring_locals { let local_ty = self.cx.body().local_decls[local].ty; let local_span = self.cx.body().local_decls[local].source_info.span; @@ -180,7 +182,7 @@ impl<'a, 'typeck, 'b, 'tcx> LivenessResults<'a, 'typeck, 'b, 'tcx> { /// /// Add facts for all locals with free regions, since regions may outlive /// the function body only at certain nodes in the CFG. - fn add_extra_drop_facts(&mut self, relevant_live_locals: &[Local]) { + fn add_extra_drop_facts(&mut self, relevant_live_locals: &DenseBitSet) { // This collect is more necessary than immediately apparent // because these facts go into `add_drop_live_facts_for()`, // which also writes to `polonius_facts`, and so this is genuinely @@ -192,15 +194,12 @@ impl<'a, 'typeck, 'b, 'tcx> LivenessResults<'a, 'typeck, 'b, 'tcx> { // `add_drop_live_facts_for()` that make sense. let Some(facts) = self.cx.typeck.polonius_facts.as_ref() else { return }; let facts_to_add: Vec<_> = { - let relevant_live_locals: FxIndexSet<_> = - relevant_live_locals.iter().copied().collect(); - facts .var_dropped_at .iter() .filter_map(|&(local, location_index)| { let local_ty = self.cx.body().local_decls[local].ty; - if relevant_live_locals.contains(&local) || !local_ty.has_free_regions() { + if relevant_live_locals.contains(local) || !local_ty.has_free_regions() { return None; } @@ -583,15 +582,6 @@ impl<'tcx> LivenessContext<'_, '_, '_, 'tcx> { typeck.constraints.liveness_constraints.add_points(live_region_vid, live_at); }, }); - - // When using `-Zpolonius=next`, we record the variance of each live region. - if let Some(polonius_liveness) = typeck.polonius_liveness.as_mut() { - polonius_liveness.record_live_region_variance( - typeck.infcx.tcx, - typeck.universal_regions, - value, - ); - } } fn compute_drop_data( diff --git a/compiler/rustc_borrowck/src/type_check/mod.rs b/compiler/rustc_borrowck/src/type_check/mod.rs index 8c51225712093..6c1910f84bc05 100644 --- a/compiler/rustc_borrowck/src/type_check/mod.rs +++ b/compiler/rustc_borrowck/src/type_check/mod.rs @@ -46,7 +46,6 @@ use crate::constraints::{OutlivesConstraint, OutlivesConstraintSet}; use crate::diagnostics::UniverseInfo; use crate::member_constraints::MemberConstraintSet; use crate::polonius::legacy::{PoloniusFacts, PoloniusLocationTable}; -use crate::polonius::{PoloniusContext, PoloniusLivenessContext}; use crate::region_infer::TypeTest; use crate::region_infer::values::{LivenessValues, PlaceholderIndex, PlaceholderIndices}; use crate::session_diagnostics::{MoveUnsized, SimdIntrinsicArgConst}; @@ -138,12 +137,6 @@ pub(crate) fn type_check<'a, 'tcx>( debug!(?normalized_inputs_and_output); - let polonius_liveness = if infcx.tcx.sess.opts.unstable_opts.polonius.is_next_enabled() { - Some(PoloniusLivenessContext::default()) - } else { - None - }; - let mut typeck = TypeChecker { root_cx, infcx, @@ -159,7 +152,6 @@ pub(crate) fn type_check<'a, 'tcx>( polonius_facts, borrow_set, constraints: &mut constraints, - polonius_liveness, }; typeck.check_user_type_annotations(); @@ -172,21 +164,7 @@ pub(crate) fn type_check<'a, 'tcx>( let opaque_type_values = opaque_types::take_opaques_and_register_member_constraints(&mut typeck); - // We're done with typeck, we can finalize the polonius liveness context for region inference. - let polonius_context = typeck.polonius_liveness.take().map(|liveness_context| { - PoloniusContext::create_from_liveness( - liveness_context, - infcx.num_region_vars(), - typeck.constraints.liveness_constraints.points(), - ) - }); - - MirTypeckResults { - constraints, - universal_region_relations, - opaque_type_values, - polonius_context, - } + MirTypeckResults { constraints, universal_region_relations, opaque_type_values } } #[track_caller] @@ -224,8 +202,6 @@ struct TypeChecker<'a, 'tcx> { polonius_facts: &'a mut Option, borrow_set: &'a BorrowSet<'tcx>, constraints: &'a mut MirTypeckRegionConstraints<'tcx>, - /// When using `-Zpolonius=next`, the liveness helper data used to create polonius constraints. - polonius_liveness: Option, } /// Holder struct for passing results from MIR typeck to the rest of the non-lexical regions @@ -234,7 +210,6 @@ pub(crate) struct MirTypeckResults<'tcx> { pub(crate) constraints: MirTypeckRegionConstraints<'tcx>, pub(crate) universal_region_relations: Frozen>, pub(crate) opaque_type_values: FxIndexMap, OpaqueHiddenType<'tcx>>, - pub(crate) polonius_context: Option, } /// A collection of region constraints that must be satisfied for the diff --git a/compiler/rustc_borrowck/src/universal_regions.rs b/compiler/rustc_borrowck/src/universal_regions.rs index c11e14d214c42..eb84454906bfc 100644 --- a/compiler/rustc_borrowck/src/universal_regions.rs +++ b/compiler/rustc_borrowck/src/universal_regions.rs @@ -17,6 +17,7 @@ use std::cell::Cell; use std::iter; +use std::ops::Range; use rustc_data_structures::fx::FxIndexMap; use rustc_errors::Diag; @@ -309,6 +310,13 @@ impl<'tcx> UniversalRegions<'tcx> { region_mapping } + ///Returns a range of all universal regions. + /// + ///In some cases we benefit from the fact that all universal regions lie in a contiguous range. + pub(crate) fn universal_regions_range(&self) -> Range { + FIRST_GLOBAL_INDEX.into()..self.num_universals.into() + } + /// Returns `true` if `r` is a member of this set of universal regions. pub(crate) fn is_universal_region(&self, r: RegionVid) -> bool { (FIRST_GLOBAL_INDEX..self.num_universals).contains(&r.index()) diff --git a/compiler/rustc_index/src/bit_set.rs b/compiler/rustc_index/src/bit_set.rs index 07934389158e5..5dc4e8947d458 100644 --- a/compiler/rustc_index/src/bit_set.rs +++ b/compiler/rustc_index/src/bit_set.rs @@ -1595,7 +1595,7 @@ impl SparseBitMatrix { Self { num_columns, rows: IndexVec::new() } } - fn ensure_row(&mut self, row: R) -> &mut DenseBitSet { + pub fn ensure_row(&mut self, row: R) -> &mut DenseBitSet { // Instantiate any missing rows up to and including row `row` with an empty `DenseBitSet`. // Then replace row `row` with a full `DenseBitSet` if necessary. self.rows.get_or_insert_with(row, || DenseBitSet::new_empty(self.num_columns)) diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index adc100941a39a..93d00ef3a8cb9 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -1575,7 +1575,14 @@ impl Location { Location { block: self.block, statement_index: self.statement_index + 1 } } - /// Returns `true` if `other` is earlier in the control flow graph than `self`. + /// Returns the location immediately before this one within the enclosing block. Panics if this + /// is the first location in the block. + #[inline] + pub fn predecessor_within_block(&self) -> Location { + Location { block: self.block, statement_index: self.statement_index - 1 } + } + + /// Returns `true` if `self` is earlier in the control flow graph than `other`. pub fn is_predecessor_of<'tcx>(&self, other: Location, body: &Body<'tcx>) -> bool { // If we are in the same block as the other location and are an earlier statement // then we are a predecessor of `other`. diff --git a/compiler/rustc_middle/src/ty/visit.rs b/compiler/rustc_middle/src/ty/visit.rs index f804217459915..c472346f2004b 100644 --- a/compiler/rustc_middle/src/ty/visit.rs +++ b/compiler/rustc_middle/src/ty/visit.rs @@ -36,8 +36,22 @@ impl<'tcx> TyCtxt<'tcx> { pub fn any_free_region_meets( self, value: &impl TypeVisitable>, - callback: impl FnMut(ty::Region<'tcx>) -> bool, + mut callback: impl FnMut(ty::Region<'tcx>) -> bool, ) -> bool { + self.for_each_free_region_until(value, |r| { + if callback(r) { ControlFlow::Break(()) } else { ControlFlow::Continue(()) } + }) + .is_some() + } + + /// Invoke `callback` on every region appearing free in `value` and exit on + /// [`ControlFlow::Break`]. + #[inline] + pub fn for_each_free_region_until( + self, + value: &impl TypeVisitable>, + callback: impl FnMut(ty::Region<'tcx>) -> ControlFlow, + ) -> Option { struct RegionVisitor { /// The index of a binder *just outside* the things we have /// traversed. If we encounter a bound region bound by this @@ -60,15 +74,15 @@ impl<'tcx> TyCtxt<'tcx> { callback: F, } - impl<'tcx, F> TypeVisitor> for RegionVisitor + impl<'tcx, F, T> TypeVisitor> for RegionVisitor where - F: FnMut(ty::Region<'tcx>) -> bool, + F: FnMut(ty::Region<'tcx>) -> ControlFlow, { - type Result = ControlFlow<()>; + type Result = ControlFlow; - fn visit_binder>>( + fn visit_binder>>( &mut self, - t: &Binder<'tcx, T>, + t: &Binder<'tcx, Ty>, ) -> Self::Result { self.outer_index.shift_in(1); let result = t.super_visit_with(self); @@ -81,13 +95,7 @@ impl<'tcx> TyCtxt<'tcx> { ty::ReBound(debruijn, _) if debruijn < self.outer_index => { ControlFlow::Continue(()) } - _ => { - if (self.callback)(r) { - ControlFlow::Break(()) - } else { - ControlFlow::Continue(()) - } - } + _ => (self.callback)(r), } } @@ -101,7 +109,7 @@ impl<'tcx> TyCtxt<'tcx> { } } - value.visit_with(&mut RegionVisitor { outer_index: ty::INNERMOST, callback }).is_break() + value.visit_with(&mut RegionVisitor { outer_index: ty::INNERMOST, callback }).break_value() } /// Returns a set of all late-bound regions that are constrained diff --git a/compiler/rustc_mir_dataflow/src/framework/mod.rs b/compiler/rustc_mir_dataflow/src/framework/mod.rs index 9cadec100b534..43ea059151cd6 100644 --- a/compiler/rustc_mir_dataflow/src/framework/mod.rs +++ b/compiler/rustc_mir_dataflow/src/framework/mod.rs @@ -58,8 +58,7 @@ mod visitor; pub use self::cursor::ResultsCursor; pub use self::direction::{Backward, Direction, Forward}; pub use self::lattice::{JoinSemiLattice, MaybeReachable}; -pub(crate) use self::results::AnalysisAndResults; -pub use self::results::Results; +pub use self::results::{AnalysisAndResults, Results}; pub use self::visitor::{ResultsVisitor, visit_reachable_results, visit_results}; /// Analysis domains are all bitsets of various kinds. This trait holds diff --git a/compiler/rustc_mir_dataflow/src/lib.rs b/compiler/rustc_mir_dataflow/src/lib.rs index 2e8c916544160..ba21fd4da00be 100644 --- a/compiler/rustc_mir_dataflow/src/lib.rs +++ b/compiler/rustc_mir_dataflow/src/lib.rs @@ -17,8 +17,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, Forward, GenKill, JoinSemiLattice, MaybeReachable, Results, - ResultsCursor, ResultsVisitor, fmt, graphviz, lattice, visit_reachable_results, visit_results, + Analysis, AnalysisAndResults, Backward, Direction, Forward, GenKill, JoinSemiLattice, + MaybeReachable, Results, ResultsCursor, ResultsVisitor, fmt, graphviz, lattice, + visit_reachable_results, visit_results, }; use self::move_paths::MoveData; diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 75f24adb70fa5..aee330fcc4338 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -1645,7 +1645,7 @@ impl<'test> TestCx<'test> { match self.config.compare_mode { Some(CompareMode::Polonius) => { - rustc.args(&["-Zpolonius"]); + rustc.args(&["-Zpolonius=next"]); } Some(CompareMode::NextSolver) => { rustc.args(&["-Znext-solver"]); diff --git a/tests/crashes/135646.rs b/tests/crashes/135646.rs deleted file mode 100644 index 841ea5b81b41f..0000000000000 --- a/tests/crashes/135646.rs +++ /dev/null @@ -1,7 +0,0 @@ -//@ known-bug: #135646 -//@ compile-flags: -Zpolonius=next -//@ edition: 2024 - -fn main() { - &{ [1, 2, 3][4] }; -}