From 1b5893748616a8ce7e13ff104eb1fdbfc4ec7252 Mon Sep 17 00:00:00 2001 From: Tage Johansson Date: Tue, 20 May 2025 14:48:30 +0200 Subject: [PATCH 01/12] Create Horatio: A faster implementation of Polonius. --- compiler/rustc_borrowck/src/borrow_set.rs | 3 + compiler/rustc_borrowck/src/dataflow.rs | 30 +- .../src/diagnostics/explain_borrow.rs | 4 +- compiler/rustc_borrowck/src/lib.rs | 412 +++++++---- compiler/rustc_borrowck/src/nll.rs | 52 +- .../src/polonius/horatio/constraints.rs | 481 +++++++++++++ .../polonius/horatio/live_region_variance.rs | 207 ++++++ .../polonius/horatio/loan_invalidations.rs | 436 ++++++++++++ .../polonius/horatio/location_sensitive.rs | 340 +++++++++ .../src/polonius/horatio/mod.rs | 662 ++++++++++++++++++ .../src/polonius/horatio/polonius_block.rs | 140 ++++ .../src/polonius/liveness_constraints.rs | 135 ---- .../src/polonius/loan_liveness.rs | 11 +- compiler/rustc_borrowck/src/polonius/mod.rs | 30 +- .../rustc_borrowck/src/region_infer/mod.rs | 13 +- .../rustc_borrowck/src/region_infer/values.rs | 8 - .../src/type_check/liveness/local_use_map.rs | 5 +- .../src/type_check/liveness/mod.rs | 76 +- .../src/type_check/liveness/trace.rs | 30 +- compiler/rustc_borrowck/src/type_check/mod.rs | 27 +- .../rustc_borrowck/src/universal_regions.rs | 8 + compiler/rustc_index/src/bit_set.rs | 2 +- compiler/rustc_middle/src/mir/mod.rs | 9 +- compiler/rustc_middle/src/ty/visit.rs | 36 +- .../rustc_mir_dataflow/src/framework/mod.rs | 3 +- compiler/rustc_mir_dataflow/src/lib.rs | 5 +- 26 files changed, 2719 insertions(+), 446 deletions(-) create mode 100644 compiler/rustc_borrowck/src/polonius/horatio/constraints.rs create mode 100644 compiler/rustc_borrowck/src/polonius/horatio/live_region_variance.rs create mode 100644 compiler/rustc_borrowck/src/polonius/horatio/loan_invalidations.rs create mode 100644 compiler/rustc_borrowck/src/polonius/horatio/location_sensitive.rs create mode 100644 compiler/rustc_borrowck/src/polonius/horatio/mod.rs create mode 100644 compiler/rustc_borrowck/src/polonius/horatio/polonius_block.rs 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..7cee784f8a8ce 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, + location_map.clone(), + ); // 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..86d9407d37592 --- /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 [`LocationInsensitiveAnalysis::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..d81c5a8170d7e --- /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. + /// + /// ``` + /// a + /// / \ + /// b c + /// \ / + /// d + /// ``` + /// In this case we have: + /// ``` + /// 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: + /// ``` + /// 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`] 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..b5de073d3c6a2 --- /dev/null +++ b/compiler/rustc_borrowck/src/polonius/horatio/polonius_block.rs @@ -0,0 +1,140 @@ +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`] 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..24b76955f88f9 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 @@ -485,7 +488,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { } let mut scc_values = - RegionValues::new(location_map, universal_regions.len(), placeholder_indices); + RegionValues::new(location_map.clone(), universal_regions.len(), placeholder_indices); for region in liveness_constraints.regions() { let scc = constraint_sccs.scc(region); @@ -509,6 +512,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { scc_values, type_tests, universal_region_relations, + location_map, }; result.init_free_and_bound_regions(); @@ -601,6 +605,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 +2265,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; From 95e1a430ded3cf5ac33ca7c69c3a3e3a6f1aa617 Mon Sep 17 00:00:00 2001 From: Tage Johansson Date: Tue, 4 Mar 2025 16:23:07 +0100 Subject: [PATCH 02/12] compiletest: Change Polonius compare mode to run polonius=next instead of legacy. --- src/tools/compiletest/src/runtest.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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"]); From 1bfbc9f6f55dcd222e395b84a3cd766fbaeecaeb Mon Sep 17 00:00:00 2001 From: Tage Johansson Date: Wed, 21 May 2025 15:03:06 +0200 Subject: [PATCH 03/12] fix documentation errors --- .../src/polonius/horatio/location_sensitive.rs | 2 +- compiler/rustc_borrowck/src/polonius/horatio/mod.rs | 4 ++-- .../rustc_borrowck/src/polonius/horatio/polonius_block.rs | 5 +++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_borrowck/src/polonius/horatio/location_sensitive.rs b/compiler/rustc_borrowck/src/polonius/horatio/location_sensitive.rs index 86d9407d37592..d813f072d66da 100644 --- a/compiler/rustc_borrowck/src/polonius/horatio/location_sensitive.rs +++ b/compiler/rustc_borrowck/src/polonius/horatio/location_sensitive.rs @@ -293,7 +293,7 @@ impl LocationSensitiveAnalysis { } } -/// This is a very specific function used in [`LocationInsensitiveAnalysis::compute()`] to visit all +/// 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. diff --git a/compiler/rustc_borrowck/src/polonius/horatio/mod.rs b/compiler/rustc_borrowck/src/polonius/horatio/mod.rs index d81c5a8170d7e..aa05925be115b 100644 --- a/compiler/rustc_borrowck/src/polonius/horatio/mod.rs +++ b/compiler/rustc_borrowck/src/polonius/horatio/mod.rs @@ -63,7 +63,7 @@ pub(crate) struct PoloniusContext<'a, 'tcx> { /// For every block, we store a set of all proceeding blocks. /// - /// ``` + /// ```text /// a /// / \ /// b c @@ -315,7 +315,7 @@ impl<'a, 'tcx> Polonius<'a, 'tcx> { /// 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`] function which will give a definite answer. + /// 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, diff --git a/compiler/rustc_borrowck/src/polonius/horatio/polonius_block.rs b/compiler/rustc_borrowck/src/polonius/horatio/polonius_block.rs index b5de073d3c6a2..34d843b4bee59 100644 --- a/compiler/rustc_borrowck/src/polonius/horatio/polonius_block.rs +++ b/compiler/rustc_borrowck/src/polonius/horatio/polonius_block.rs @@ -116,8 +116,9 @@ impl PoloniusBlock { /// Iterate over the successor blocks to this block. /// - /// Note that this is same as [`Terminator::successors`] except for the "before introduction - /// block" where it is the "introduction 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, From 37e44c5435955f56b2dbf3004f8f6adc0c86db63 Mon Sep 17 00:00:00 2001 From: Tage Johansson Date: Wed, 21 May 2025 16:04:45 +0200 Subject: [PATCH 04/12] fix Clippy warning about cloning reference counted pointers --- compiler/rustc_borrowck/src/nll.rs | 2 +- compiler/rustc_borrowck/src/region_infer/mod.rs | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_borrowck/src/nll.rs b/compiler/rustc_borrowck/src/nll.rs index 7cee784f8a8ce..a6da7e7a3e531 100644 --- a/compiler/rustc_borrowck/src/nll.rs +++ b/compiler/rustc_borrowck/src/nll.rs @@ -124,7 +124,7 @@ pub(crate) fn compute_regions<'a, 'tcx>( infcx, constraints, universal_region_relations, - location_map.clone(), + Rc::clone(&location_map), ); // If requested: dump NLL facts, and run legacy polonius analysis. diff --git a/compiler/rustc_borrowck/src/region_infer/mod.rs b/compiler/rustc_borrowck/src/region_infer/mod.rs index 24b76955f88f9..59659963835f7 100644 --- a/compiler/rustc_borrowck/src/region_infer/mod.rs +++ b/compiler/rustc_borrowck/src/region_infer/mod.rs @@ -487,8 +487,11 @@ impl<'tcx> RegionInferenceContext<'tcx> { sccs_info(infcx, &constraint_sccs); } - let mut scc_values = - RegionValues::new(location_map.clone(), 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); From 2522f24161b5d544713d00738024f73863817378 Mon Sep 17 00:00:00 2001 From: Tage Johansson Date: Thu, 22 May 2025 10:25:30 +0200 Subject: [PATCH 05/12] remove tests/crashes/135646.rs as it doesn't crash anymore --- tests/crashes/135646.rs | 7 ------- 1 file changed, 7 deletions(-) delete mode 100644 tests/crashes/135646.rs 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] }; -} From baa12bc727a6c41850622b26316558978f46428c Mon Sep 17 00:00:00 2001 From: Tage Johansson Date: Thu, 22 May 2025 14:32:10 +0200 Subject: [PATCH 06/12] make some code blocks in comments explicitly ```text ``` so they are not interpretted as Rust doctests --- compiler/rustc_borrowck/src/polonius/horatio/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_borrowck/src/polonius/horatio/mod.rs b/compiler/rustc_borrowck/src/polonius/horatio/mod.rs index aa05925be115b..d2478c1b7854a 100644 --- a/compiler/rustc_borrowck/src/polonius/horatio/mod.rs +++ b/compiler/rustc_borrowck/src/polonius/horatio/mod.rs @@ -71,7 +71,7 @@ pub(crate) struct PoloniusContext<'a, 'tcx> { /// d /// ``` /// In this case we have: - /// ``` + /// ```text /// a: {} /// b: {a} /// c: {a} @@ -89,7 +89,7 @@ pub(crate) struct PoloniusContext<'a, 'tcx> { /// d /// ``` /// In this case we have: - /// ``` + /// ```text /// a: {} /// b: {a} /// c: {a} From c7076d9f2db1e724cc1e901ad0c2f3ecaaf7c37b Mon Sep 17 00:00:00 2001 From: Tage Johansson Date: Tue, 20 May 2025 15:01:30 +0200 Subject: [PATCH 07/12] Rewrite the DenseBitSet structure to only use one word on the stack. This commit modifies DenseBitSet so that it only uses one word on the stack instead of 4 words as before, allowing for faster clones. The downside is that it may at most store 63 elements on the stack as aposed to 128 for the previous implementation. --- Cargo.lock | 1 + compiler/rustc_abi/src/layout/coroutine.rs | 2 +- compiler/rustc_index/Cargo.toml | 1 + compiler/rustc_index/src/bit_set.rs | 577 +------- .../rustc_index/src/bit_set/dense_bit_set.rs | 1275 +++++++++++++++++ compiler/rustc_index/src/bit_set/tests.rs | 658 ++++++++- compiler/rustc_middle/src/values.rs | 8 +- .../src/framework/cursor.rs | 2 +- .../rustc_mir_dataflow/src/framework/fmt.rs | 19 +- .../src/impls/initialized.rs | 2 +- .../src/impls/storage_liveness.rs | 1 - compiler/rustc_mir_transform/src/copy_prop.rs | 2 +- compiler/rustc_mir_transform/src/coroutine.rs | 16 +- .../src/coverage/counters.rs | 3 +- .../src/coverage/counters/balanced_flow.rs | 2 +- .../rustc_mir_transform/src/coverage/query.rs | 7 +- .../src/deduce_param_attrs.rs | 7 +- .../src/lint_tail_expr_drop_order.rs | 2 +- .../src/single_use_consts.rs | 4 +- compiler/rustc_mir_transform/src/sroa.rs | 2 +- 20 files changed, 1975 insertions(+), 616 deletions(-) create mode 100644 compiler/rustc_index/src/bit_set/dense_bit_set.rs diff --git a/Cargo.lock b/Cargo.lock index 99cb71cd0ac87..d398c1d5637b2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3855,6 +3855,7 @@ dependencies = [ name = "rustc_index" version = "0.0.0" dependencies = [ + "itertools", "rustc_index_macros", "rustc_macros", "rustc_serialize", diff --git a/compiler/rustc_abi/src/layout/coroutine.rs b/compiler/rustc_abi/src/layout/coroutine.rs index 27e704d538c83..73564705686f0 100644 --- a/compiler/rustc_abi/src/layout/coroutine.rs +++ b/compiler/rustc_abi/src/layout/coroutine.rs @@ -120,7 +120,7 @@ fn coroutine_saved_local_eligibility { fn intersect(&mut self, other: &Rhs) -> bool; } -#[inline] -fn inclusive_start_end( - range: impl RangeBounds, - domain: usize, -) -> Option<(usize, usize)> { - // Both start and end are inclusive. - let start = match range.start_bound().cloned() { - Bound::Included(start) => start.index(), - Bound::Excluded(start) => start.index() + 1, - Bound::Unbounded => 0, - }; - let end = match range.end_bound().cloned() { - Bound::Included(end) => end.index(), - Bound::Excluded(end) => end.index().checked_sub(1)?, - Bound::Unbounded => domain - 1, - }; - assert!(end < domain); - if start > end { - return None; - } - Some((start, end)) -} - macro_rules! bit_relations_inherent_impls { () => { /// Sets `self = self | other` and returns `true` if `self` changed @@ -96,345 +75,7 @@ macro_rules! bit_relations_inherent_impls { } }; } - -/// A fixed-size bitset type with a dense representation. -/// -/// Note 1: Since this bitset is dense, if your domain is big, and/or relatively -/// homogeneous (for example, with long runs of bits set or unset), then it may -/// be preferable to instead use a [MixedBitSet], or an -/// [IntervalSet](crate::interval::IntervalSet). They should be more suited to -/// sparse, or highly-compressible, domains. -/// -/// Note 2: Use [`GrowableBitSet`] if you need support for resizing after creation. -/// -/// `T` is an index type, typically a newtyped `usize` wrapper, but it can also -/// just be `usize`. -/// -/// All operations that involve an element will panic if the element is equal -/// to or greater than the domain size. All operations that involve two bitsets -/// will panic if the bitsets have differing domain sizes. -/// -#[cfg_attr(feature = "nightly", derive(Decodable_NoContext, Encodable_NoContext))] -#[derive(Eq, PartialEq, Hash)] -pub struct DenseBitSet { - domain_size: usize, - words: SmallVec<[Word; 2]>, - marker: PhantomData, -} - -impl DenseBitSet { - /// Gets the domain size. - pub fn domain_size(&self) -> usize { - self.domain_size - } -} - -impl DenseBitSet { - /// Creates a new, empty bitset with a given `domain_size`. - #[inline] - pub fn new_empty(domain_size: usize) -> DenseBitSet { - let num_words = num_words(domain_size); - DenseBitSet { domain_size, words: smallvec![0; num_words], marker: PhantomData } - } - - /// Creates a new, filled bitset with a given `domain_size`. - #[inline] - pub fn new_filled(domain_size: usize) -> DenseBitSet { - let num_words = num_words(domain_size); - let mut result = - DenseBitSet { domain_size, words: smallvec![!0; num_words], marker: PhantomData }; - result.clear_excess_bits(); - result - } - - /// Clear all elements. - #[inline] - pub fn clear(&mut self) { - self.words.fill(0); - } - - /// Clear excess bits in the final word. - fn clear_excess_bits(&mut self) { - clear_excess_bits_in_final_word(self.domain_size, &mut self.words); - } - - /// Count the number of set bits in the set. - pub fn count(&self) -> usize { - self.words.iter().map(|e| e.count_ones() as usize).sum() - } - - /// Returns `true` if `self` contains `elem`. - #[inline] - pub fn contains(&self, elem: T) -> bool { - assert!(elem.index() < self.domain_size); - let (word_index, mask) = word_index_and_mask(elem); - (self.words[word_index] & mask) != 0 - } - - /// Is `self` is a (non-strict) superset of `other`? - #[inline] - pub fn superset(&self, other: &DenseBitSet) -> bool { - assert_eq!(self.domain_size, other.domain_size); - self.words.iter().zip(&other.words).all(|(a, b)| (a & b) == *b) - } - - /// Is the set empty? - #[inline] - pub fn is_empty(&self) -> bool { - self.words.iter().all(|a| *a == 0) - } - - /// Insert `elem`. Returns whether the set has changed. - #[inline] - pub fn insert(&mut self, elem: T) -> bool { - assert!( - elem.index() < self.domain_size, - "inserting element at index {} but domain size is {}", - elem.index(), - self.domain_size, - ); - let (word_index, mask) = word_index_and_mask(elem); - let word_ref = &mut self.words[word_index]; - let word = *word_ref; - let new_word = word | mask; - *word_ref = new_word; - new_word != word - } - - #[inline] - pub fn insert_range(&mut self, elems: impl RangeBounds) { - let Some((start, end)) = inclusive_start_end(elems, self.domain_size) else { - return; - }; - - let (start_word_index, start_mask) = word_index_and_mask(start); - let (end_word_index, end_mask) = word_index_and_mask(end); - - // Set all words in between start and end (exclusively of both). - for word_index in (start_word_index + 1)..end_word_index { - self.words[word_index] = !0; - } - - if start_word_index != end_word_index { - // Start and end are in different words, so we handle each in turn. - // - // We set all leading bits. This includes the start_mask bit. - self.words[start_word_index] |= !(start_mask - 1); - // And all trailing bits (i.e. from 0..=end) in the end word, - // including the end. - self.words[end_word_index] |= end_mask | (end_mask - 1); - } else { - self.words[start_word_index] |= end_mask | (end_mask - start_mask); - } - } - - /// Sets all bits to true. - pub fn insert_all(&mut self) { - self.words.fill(!0); - self.clear_excess_bits(); - } - - /// Returns `true` if the set has changed. - #[inline] - pub fn remove(&mut self, elem: T) -> bool { - assert!(elem.index() < self.domain_size); - let (word_index, mask) = word_index_and_mask(elem); - let word_ref = &mut self.words[word_index]; - let word = *word_ref; - let new_word = word & !mask; - *word_ref = new_word; - new_word != word - } - - /// Iterates over the indices of set bits in a sorted order. - #[inline] - pub fn iter(&self) -> BitIter<'_, T> { - BitIter::new(&self.words) - } - - pub fn last_set_in(&self, range: impl RangeBounds) -> Option { - let (start, end) = inclusive_start_end(range, self.domain_size)?; - let (start_word_index, _) = word_index_and_mask(start); - let (end_word_index, end_mask) = word_index_and_mask(end); - - let end_word = self.words[end_word_index] & (end_mask | (end_mask - 1)); - if end_word != 0 { - let pos = max_bit(end_word) + WORD_BITS * end_word_index; - if start <= pos { - return Some(T::new(pos)); - } - } - - // We exclude end_word_index from the range here, because we don't want - // to limit ourselves to *just* the last word: the bits set it in may be - // after `end`, so it may not work out. - if let Some(offset) = - self.words[start_word_index..end_word_index].iter().rposition(|&w| w != 0) - { - let word_idx = start_word_index + offset; - let start_word = self.words[word_idx]; - let pos = max_bit(start_word) + WORD_BITS * word_idx; - if start <= pos { - return Some(T::new(pos)); - } - } - - None - } - - bit_relations_inherent_impls! {} - - /// Sets `self = self | !other`. - /// - /// FIXME: Incorporate this into [`BitRelations`] and fill out - /// implementations for other bitset types, if needed. - pub fn union_not(&mut self, other: &DenseBitSet) { - assert_eq!(self.domain_size, other.domain_size); - - // FIXME(Zalathar): If we were to forcibly _set_ all excess bits before - // the bitwise update, and then clear them again afterwards, we could - // quickly and accurately detect whether the update changed anything. - // But that's only worth doing if there's an actual use-case. - - bitwise(&mut self.words, &other.words, |a, b| a | !b); - // The bitwise update `a | !b` can result in the last word containing - // out-of-domain bits, so we need to clear them. - self.clear_excess_bits(); - } -} - -// dense REL dense -impl BitRelations> for DenseBitSet { - fn union(&mut self, other: &DenseBitSet) -> bool { - assert_eq!(self.domain_size, other.domain_size); - bitwise(&mut self.words, &other.words, |a, b| a | b) - } - - fn subtract(&mut self, other: &DenseBitSet) -> bool { - assert_eq!(self.domain_size, other.domain_size); - bitwise(&mut self.words, &other.words, |a, b| a & !b) - } - - fn intersect(&mut self, other: &DenseBitSet) -> bool { - assert_eq!(self.domain_size, other.domain_size); - bitwise(&mut self.words, &other.words, |a, b| a & b) - } -} - -impl From> for DenseBitSet { - fn from(bit_set: GrowableBitSet) -> Self { - bit_set.bit_set - } -} - -impl Clone for DenseBitSet { - fn clone(&self) -> Self { - DenseBitSet { - domain_size: self.domain_size, - words: self.words.clone(), - marker: PhantomData, - } - } - - fn clone_from(&mut self, from: &Self) { - self.domain_size = from.domain_size; - self.words.clone_from(&from.words); - } -} - -impl fmt::Debug for DenseBitSet { - fn fmt(&self, w: &mut fmt::Formatter<'_>) -> fmt::Result { - w.debug_list().entries(self.iter()).finish() - } -} - -impl ToString for DenseBitSet { - fn to_string(&self) -> String { - let mut result = String::new(); - let mut sep = '['; - - // Note: this is a little endian printout of bytes. - - // i tracks how many bits we have printed so far. - let mut i = 0; - for word in &self.words { - let mut word = *word; - for _ in 0..WORD_BYTES { - // for each byte in `word`: - let remain = self.domain_size - i; - // If less than a byte remains, then mask just that many bits. - let mask = if remain <= 8 { (1 << remain) - 1 } else { 0xFF }; - assert!(mask <= 0xFF); - let byte = word & mask; - - result.push_str(&format!("{sep}{byte:02x}")); - - if remain <= 8 { - break; - } - word >>= 8; - i += 8; - sep = '-'; - } - sep = '|'; - } - result.push(']'); - - result - } -} - -pub struct BitIter<'a, T: Idx> { - /// A copy of the current word, but with any already-visited bits cleared. - /// (This lets us use `trailing_zeros()` to find the next set bit.) When it - /// is reduced to 0, we move onto the next word. - word: Word, - - /// The offset (measured in bits) of the current word. - offset: usize, - - /// Underlying iterator over the words. - iter: slice::Iter<'a, Word>, - - marker: PhantomData, -} - -impl<'a, T: Idx> BitIter<'a, T> { - #[inline] - fn new(words: &'a [Word]) -> BitIter<'a, T> { - // We initialize `word` and `offset` to degenerate values. On the first - // call to `next()` we will fall through to getting the first word from - // `iter`, which sets `word` to the first word (if there is one) and - // `offset` to 0. Doing it this way saves us from having to maintain - // additional state about whether we have started. - BitIter { - word: 0, - offset: usize::MAX - (WORD_BITS - 1), - iter: words.iter(), - marker: PhantomData, - } - } -} - -impl<'a, T: Idx> Iterator for BitIter<'a, T> { - type Item = T; - fn next(&mut self) -> Option { - loop { - if self.word != 0 { - // Get the position of the next set bit in the current word, - // then clear the bit. - let bit_pos = self.word.trailing_zeros() as usize; - self.word ^= 1 << bit_pos; - return Some(T::new(bit_pos + self.offset)); - } - - // Move onto the next word. `wrapping_add()` is needed to handle - // the degenerate initial value given to `offset` in `new()`. - self.word = *self.iter.next()?; - self.offset = self.offset.wrapping_add(WORD_BITS); - } - } -} +use bit_relations_inherent_impls; /// A fixed-size bitset type with a partially dense, partially sparse /// representation. The bitset is broken into chunks, and chunks that are all @@ -727,7 +368,7 @@ impl ChunkedBitSet { Some(Ones(chunk_domain_size)) => ChunkIter::Ones(0..*chunk_domain_size as usize), Some(Mixed(chunk_domain_size, _, words)) => { let num_words = num_words(*chunk_domain_size as usize); - ChunkIter::Mixed(BitIter::new(&words[0..num_words])) + ChunkIter::Mixed(BitIter::from_slice(&words[0..num_words])) } None => ChunkIter::Finished, } @@ -771,8 +412,8 @@ impl BitRelations> for ChunkedBitSet { ) { let self_chunk_words = Rc::make_mut(self_chunk_words); let has_changed = bitwise( - &mut self_chunk_words[0..num_words], - &other_chunk_words[0..num_words], + self_chunk_words[0..num_words].iter_mut(), + other_chunk_words[0..num_words].iter().copied(), op, ); debug_assert!(has_changed); @@ -847,8 +488,8 @@ impl BitRelations> for ChunkedBitSet { ) { let self_chunk_words = Rc::make_mut(self_chunk_words); let has_changed = bitwise( - &mut self_chunk_words[0..num_words], - &other_chunk_words[0..num_words], + self_chunk_words[0..num_words].iter_mut(), + other_chunk_words[0..num_words].iter().copied(), op, ); debug_assert!(has_changed); @@ -898,8 +539,8 @@ impl BitRelations> for ChunkedBitSet { ) { let self_chunk_words = Rc::make_mut(self_chunk_words); let has_changed = bitwise( - &mut self_chunk_words[0..num_words], - &other_chunk_words[0..num_words], + self_chunk_words[0..num_words].iter_mut(), + other_chunk_words[0..num_words].iter().copied(), op, ); debug_assert!(has_changed); @@ -920,48 +561,6 @@ impl BitRelations> for ChunkedBitSet { } } -impl BitRelations> for DenseBitSet { - fn union(&mut self, other: &ChunkedBitSet) -> bool { - sequential_update(|elem| self.insert(elem), other.iter()) - } - - fn subtract(&mut self, _other: &ChunkedBitSet) -> bool { - unimplemented!("implement if/when necessary"); - } - - fn intersect(&mut self, other: &ChunkedBitSet) -> bool { - assert_eq!(self.domain_size(), other.domain_size); - let mut changed = false; - for (i, chunk) in other.chunks.iter().enumerate() { - let mut words = &mut self.words[i * CHUNK_WORDS..]; - if words.len() > CHUNK_WORDS { - words = &mut words[..CHUNK_WORDS]; - } - match chunk { - Zeros(..) => { - for word in words { - if *word != 0 { - changed = true; - *word = 0; - } - } - } - Ones(..) => (), - Mixed(_, _, data) => { - for (i, word) in words.iter_mut().enumerate() { - let new_val = *word & data[i]; - if new_val != *word { - changed = true; - *word = new_val; - } - } - } - } - } - changed - } -} - impl Clone for ChunkedBitSet { fn clone(&self) -> Self { ChunkedBitSet { @@ -1080,15 +679,6 @@ enum ChunkIter<'a> { Finished, } -// Applies a function to mutate a bitset, and returns true if any -// of the applications return true -fn sequential_update( - mut self_update: impl FnMut(T) -> bool, - it: impl Iterator, -) -> bool { - it.fold(false, |changed, elem| self_update(elem) | changed) -} - impl fmt::Debug for ChunkedBitSet { fn fmt(&self, w: &mut fmt::Formatter<'_>) -> fmt::Result { w.debug_list().entries(self.iter()).finish() @@ -1108,15 +698,16 @@ impl fmt::Debug for ChunkedBitSet { /// "changed" return value unreliable, because the change might have only /// affected excess bits. #[inline] -fn bitwise(out_vec: &mut [Word], in_vec: &[Word], op: Op) -> bool -where - Op: Fn(Word, Word) -> Word, -{ - assert_eq!(out_vec.len(), in_vec.len()); +fn bitwise<'a>( + out: impl ExactSizeIterator, + in_: impl ExactSizeIterator, + op: impl Fn(Word, Word) -> Word, +) -> bool { + assert_eq!(out.len(), in_.len()); let mut changed = 0; - for (out_elem, in_elem) in iter::zip(out_vec, in_vec) { + for (out_elem, in_elem) in iter::zip(out, in_) { let old_val = *out_elem; - let new_val = op(old_val, *in_elem); + let new_val = op(old_val, in_elem); *out_elem = new_val; // This is essentially equivalent to a != with changed being a bool, but // in practice this code gets auto-vectorized by the compiler for most @@ -1161,15 +752,6 @@ pub enum MixedBitSet { Large(ChunkedBitSet), } -impl MixedBitSet { - pub fn domain_size(&self) -> usize { - match self { - MixedBitSet::Small(set) => set.domain_size(), - MixedBitSet::Large(set) => set.domain_size(), - } - } -} - impl MixedBitSet { #[inline] pub fn new_empty(domain_size: usize) -> MixedBitSet { @@ -1204,10 +786,15 @@ impl MixedBitSet { } } - pub fn insert_all(&mut self) { + /// Insert `0..domain_size` in the set. + /// + /// We would like an insert all function that doesn't require the domain size, but the exact + /// domain size is not stored in the `Small` variant, so that is not possible. + #[inline] + pub fn insert_all(&mut self, domain_size: usize) { match self { - MixedBitSet::Small(set) => set.insert_all(), - MixedBitSet::Large(set) => set.insert_all(), + Self::Small(set) => set.insert_all(domain_size), + Self::Large(set) => set.insert_all(), } } @@ -1304,87 +891,6 @@ impl<'a, T: Idx> Iterator for MixedBitIter<'a, T> { } } -/// A resizable bitset type with a dense representation. -/// -/// `T` is an index type, typically a newtyped `usize` wrapper, but it can also -/// just be `usize`. -/// -/// All operations that involve an element will panic if the element is equal -/// to or greater than the domain size. -#[derive(Clone, Debug, PartialEq)] -pub struct GrowableBitSet { - bit_set: DenseBitSet, -} - -impl Default for GrowableBitSet { - fn default() -> Self { - GrowableBitSet::new_empty() - } -} - -impl GrowableBitSet { - /// Ensure that the set can hold at least `min_domain_size` elements. - pub fn ensure(&mut self, min_domain_size: usize) { - if self.bit_set.domain_size < min_domain_size { - self.bit_set.domain_size = min_domain_size; - } - - let min_num_words = num_words(min_domain_size); - if self.bit_set.words.len() < min_num_words { - self.bit_set.words.resize(min_num_words, 0) - } - } - - pub fn new_empty() -> GrowableBitSet { - GrowableBitSet { bit_set: DenseBitSet::new_empty(0) } - } - - pub fn with_capacity(capacity: usize) -> GrowableBitSet { - GrowableBitSet { bit_set: DenseBitSet::new_empty(capacity) } - } - - /// Returns `true` if the set has changed. - #[inline] - pub fn insert(&mut self, elem: T) -> bool { - self.ensure(elem.index() + 1); - self.bit_set.insert(elem) - } - - /// Returns `true` if the set has changed. - #[inline] - pub fn remove(&mut self, elem: T) -> bool { - self.ensure(elem.index() + 1); - self.bit_set.remove(elem) - } - - #[inline] - pub fn is_empty(&self) -> bool { - self.bit_set.is_empty() - } - - #[inline] - pub fn contains(&self, elem: T) -> bool { - let (word_index, mask) = word_index_and_mask(elem); - self.bit_set.words.get(word_index).is_some_and(|word| (word & mask) != 0) - } - - #[inline] - pub fn iter(&self) -> BitIter<'_, T> { - self.bit_set.iter() - } - - #[inline] - pub fn len(&self) -> usize { - self.bit_set.count() - } -} - -impl From> for GrowableBitSet { - fn from(bit_set: DenseBitSet) -> Self { - Self { bit_set } - } -} - /// A fixed-size 2D bit matrix type with a dense representation. /// /// `R` and `C` are index types used to identify rows and columns respectively; @@ -1416,14 +922,17 @@ impl BitMatrix { } /// Creates a new matrix, with `row` used as the value for every row. - pub fn from_row_n(row: &DenseBitSet, num_rows: usize) -> BitMatrix { - let num_columns = row.domain_size(); + pub fn from_row_n( + row: &DenseBitSet, + num_rows: usize, + num_columns: usize, + ) -> BitMatrix { let words_per_row = num_words(num_columns); - assert_eq!(words_per_row, row.words.len()); + assert_eq!(words_per_row, row.words().len()); BitMatrix { num_rows, num_columns, - words: iter::repeat(&row.words).take(num_rows).flatten().cloned().collect(), + words: iter::repeat_with(|| row.words()).take(num_rows).flatten().collect(), marker: PhantomData, } } @@ -1516,9 +1025,9 @@ impl BitMatrix { /// returns `true` if anything changed. pub fn union_row_with(&mut self, with: &DenseBitSet, write: R) -> bool { assert!(write.index() < self.num_rows); - assert_eq!(with.domain_size(), self.num_columns); + assert!(with.capacity() >= self.num_columns); let (write_start, write_end) = self.range(write); - bitwise(&mut self.words[write_start..write_end], &with.words, |a, b| a | b) + bitwise(self.words[write_start..write_end].iter_mut(), with.words(), |a, b| a | b) } /// Sets every cell in `row` to true. @@ -1542,7 +1051,7 @@ impl BitMatrix { pub fn iter(&self, row: R) -> BitIter<'_, C> { assert!(row.index() < self.num_rows); let (start, end) = self.range(row); - BitIter::new(&self.words[start..end]) + BitIter::from_slice(&self.words[start..end]) } /// Returns the number of elements in `row`. @@ -1657,11 +1166,6 @@ impl SparseBitMatrix { } } - /// Insert all bits in the given row. - pub fn insert_all_into_row(&mut self, row: R) { - self.ensure_row(row).insert_all(); - } - pub fn rows(&self) -> impl Iterator { self.rows.indices() } @@ -1754,11 +1258,6 @@ fn clear_excess_bits_in_final_word(domain_size: usize, words: &mut [Word]) { } } -#[inline] -fn max_bit(word: Word) -> usize { - WORD_BITS - 1 - word.leading_zeros() as usize -} - /// Integral type used to represent the bit set. pub trait FiniteBitSetTy: BitAnd diff --git a/compiler/rustc_index/src/bit_set/dense_bit_set.rs b/compiler/rustc_index/src/bit_set/dense_bit_set.rs new file mode 100644 index 0000000000000..334dfadbf3952 --- /dev/null +++ b/compiler/rustc_index/src/bit_set/dense_bit_set.rs @@ -0,0 +1,1275 @@ +use std::alloc::{Layout, alloc, alloc_zeroed, dealloc, handle_alloc_error, realloc}; +use std::hash::{Hash, Hasher}; +use std::iter::FusedIterator; +use std::marker::PhantomData; +use std::mem::ManuallyDrop; +use std::ops::{Range, RangeInclusive}; +use std::ptr::NonNull; +use std::{fmt, iter, slice}; + +use itertools::Either; +use rustc_serialize::{Decodable, Decoder, Encodable, Encoder}; + +use super::{ + BitRelations, CHUNK_WORDS, Chunk, ChunkedBitSet, WORD_BITS, Word, word_index_and_mask, +}; +use crate::Idx; + +/// A fixed-size bitset type with a dense representation, using only one [`Word`] on the stack. +/// +/// This bit set occupies only a single [`Word`] of stack space. It can represent a domain size +/// of up to `[WORD_BITS] - 1` directly inline. If the domain size exceeds this limit, it instead +/// becomes a pointer to a sequence of [`Word`]s on the heap. This makes it very efficient for +/// domain sizes smaller than `[WORD_BITS]`. +/// +/// Additionally, if the set does not fit in one [`Word`], there is a special inline +/// variant for the empty set. In this case, the domain size is stored inline along with a few +/// bits indicating that the set is empty. Allocation is deferred until needed, such as on +/// the first insert or remove operation. This avoids the need to wrap a lazily initialised bit set +/// in a [`OnceCell`] or an [`Option`]—you can simply create an empty set and populate it if needed. +/// +/// Note 1: Since this bitset is dense, if your domain is large and/or relatively homogeneous (e.g. +/// long runs of set or unset bits), it may be more efficient to use a [MixedBitSet] or an +/// [IntervalSet](crate::interval::IntervalSet), which are better suited for sparse or highly +/// compressible domains. +/// +/// Note 2: Use [`GrowableBitSet`] if you require support for resizing after creation. +/// +/// `T` is an index type—typically a newtyped `usize` wrapper, but it may also simply be `usize`. +/// +/// Any operation involving an element may panic if the element is equal to or greater than the +/// domain size. Operations involving two bitsets may panic if their domain sizes differ. Panicking +/// is not garranteed though as we store the domain size rounded up to the next multiple of +/// [`WORD_BITS`]. +#[repr(C)] +pub union DenseBitSet { + /// The bit set fits in a single [`Word`] stored inline on the stack. + /// + /// The most significant bit is set to 1 to distinguish this from the other variants. You + /// must never change that "tag bit" after the bit set has been created. + /// + /// The remaining bits makes up the bit set. The exact domain size is not stored. + inline: Word, + + /// The bit set doesn't fit in a single word, but is empty and not yet allocated. + /// + /// The first (most significant) two bits are set to `[0, 1]` to distinguish this variant + /// from others. This tag is stored in [`Self::EMPTY_UNALLOCATED_TAG_BITS`]. The remaining bits + /// hold the domain size (capacity) **in words** of the set, which is needed if the set is + /// eventually allocated. + /// + /// Note that because the capacity is stored in words, not in bits, there is plenty of room + /// for the two tag bits. + empty_unallocated: usize, + + /// The bit set is stored on the heap. + /// + /// The two most significant bits are set to zero if this field is active. + on_heap: ManuallyDrop, + + /// This variant will never be created. + marker: PhantomData, +} + +impl DenseBitSet { + /// The maximum domain size that could be stored inlined on the stack. + pub const INLINE_CAPACITY: usize = WORD_BITS - 1; + + /// A [`Word`] with the most significant bit set. That is the tag bit telling that the set is + /// inlined. + const IS_INLINE_TAG_BIT: Word = 0x1 << (WORD_BITS - 1); + + /// The tag for the `empty_unallocated` variant. The two most significant bits are + /// `[0, 1]`. + const EMPTY_UNALLOCATED_TAG_BITS: usize = 0b01 << (WORD_BITS - 2); + + /// Create a new empty bit set with a given domain_size. + /// + /// If `domain_size` is <= [`Self::INLINE_CAPACITY`], then it is stored inline on the stack, + /// otherwise it is stored on the heap. + #[inline] + pub fn new_empty(domain_size: usize) -> Self { + if domain_size <= Self::INLINE_CAPACITY { + // The first bit is set to indicate the union variant. + Self { inline: Self::IS_INLINE_TAG_BIT } + } else { + let num_words = domain_size.div_ceil(WORD_BITS); + debug_assert!(num_words.leading_zeros() >= 2); + Self { empty_unallocated: Self::EMPTY_UNALLOCATED_TAG_BITS | num_words } + } + } + + /// Create a new filled bit set. + #[inline] + pub fn new_filled(domain_size: usize) -> Self { + if domain_size <= Self::INLINE_CAPACITY { + Self { + inline: Word::MAX.unbounded_shr((WORD_BITS - domain_size) as u32) + | Self::IS_INLINE_TAG_BIT, + } + } else { + let num_words = domain_size.div_ceil(WORD_BITS); + let mut on_heap = BitSetOnHeap::new_empty(num_words); + let words = on_heap.as_mut_slice(); + for word in words.iter_mut() { + *word = Word::MAX; + } + // Remove excessive bits on the last word. + *words.last_mut().unwrap() >>= WORD_BITS - domain_size % WORD_BITS; + Self { on_heap: ManuallyDrop::new(on_heap) } + } + } + + /// Check if `self` is inlined. + // If this function returns `true`, it is safe to assume `self.inline`. Else, it is safe to + // assume `self.empty_unallocated`, or `self.on_heap`. + #[inline(always)] + pub fn is_inline(&self) -> bool { + // We check if the first bit is set. If so, it is inlined, otherwise it is on the heap. + (unsafe { self.inline } & Self::IS_INLINE_TAG_BIT) != 0 + } + + /// Check if `self` has a too large domain to be stored inline, is empty, and is not yet + /// allocated. + // If this function returns `true`, it is safe to assume `self.empty_unallocated`. Else, it is + // safe to assume `self.inline`, or `self.on_heap`. + #[inline(always)] + pub const fn is_empty_unallocated(&self) -> bool { + (unsafe { self.empty_unallocated }) >> usize::BITS as u32 - 2 + == Self::EMPTY_UNALLOCATED_TAG_BITS >> usize::BITS as u32 - 2 + } + + /// Check if `self` is `empty_unallocated` and if so return the number of words required to + /// store the expected capacity. + // If this function returns `true`, it is safe to assume `self.empty_unallocated`. Else, it is + // safe to assume `self.inline`, or `self.on_heap`. + #[inline(always)] + pub const fn empty_unallocated_get_num_words(&self) -> Option { + if self.is_empty_unallocated() { + Some(unsafe { self.empty_unallocated } ^ Self::EMPTY_UNALLOCATED_TAG_BITS) + } else { + None + } + } + + /// Check if `self` is allocated on the heap and return a reference to it in that case. + fn on_heap(&self) -> Option<&BitSetOnHeap> { + let self_word = unsafe { self.inline }; + // Check if the two most significant bits are 0. + if self_word & Word::MAX >> 2 == self_word { Some(unsafe { &self.on_heap }) } else { None } + } + + /// Check if `self` is allocated on the heap and return a mutable reference to it in that case. + fn on_heap_mut(&mut self) -> Option<&mut ManuallyDrop> { + let self_word = unsafe { self.inline }; + // Check if the two most significant bits are 0. + if self_word & Word::MAX >> 2 == self_word { + Some(unsafe { &mut self.on_heap }) + } else { + None + } + } + + /// If `self` is `empty_unallocated`, allocate it, otherwise return `self.on_heap_mut()`. + fn on_heap_get_or_alloc(&mut self) -> &mut BitSetOnHeap { + if let Some(num_words) = self.empty_unallocated_get_num_words() { + *self = Self { on_heap: ManuallyDrop::new(BitSetOnHeap::new_empty(num_words)) }; + unsafe { &mut self.on_heap } + } else { + self.on_heap_mut().unwrap() + } + } + + /// Get the capacity of this set. This is >= the initial domain size. + #[inline(always)] + pub(super) fn capacity(&self) -> usize { + if self.is_inline() { + Self::INLINE_CAPACITY + } else if let Some(num_words) = self.empty_unallocated_get_num_words() { + num_words * WORD_BITS + } else { + self.on_heap().unwrap().capacity() + } + } + + /// Checks if the bit set is empty. + #[inline(always)] + pub fn is_empty(&self) -> bool { + if self.is_inline() { + let x = unsafe { self.inline }; + x == Self::IS_INLINE_TAG_BIT + } else if self.is_empty_unallocated() { + true + } else { + self.on_heap().unwrap().is_empty() + } + } + + /// Clear the set. + #[inline(always)] + pub fn clear(&mut self) { + if self.is_inline() { + self.inline = Self::IS_INLINE_TAG_BIT + } else if let Some(on_heap) = self.on_heap_mut() { + for word in on_heap.as_mut_slice() { + *word = 0x0; + } + } + } + + /// Get an iterator of all words making up the set. + pub(super) fn words(&self) -> impl ExactSizeIterator { + if self.is_inline() { + let word = unsafe { self.inline } ^ Self::IS_INLINE_TAG_BIT; + Either::Left(iter::once(word)) + } else if let Some(num_words) = self.empty_unallocated_get_num_words() { + Either::Right(Either::Left(iter::repeat_n(0, num_words))) + } else { + Either::Right(Either::Right(self.on_heap().unwrap().as_slice().iter().copied())) + } + } + + /// Checks if `self` is a (non-strict) superset of `other`. + /// + /// May panic if `self` and other have different sizes. + #[inline(always)] + pub fn superset(&self, other: &Self) -> bool { + // Function to check that a usize is a superset of another. + let word_is_superset = |x: Word, other: Word| (!x & other) == 0; + + if self.is_inline() { + let x = unsafe { self.inline }; + assert!(other.is_inline(), "bit sets has different domain sizes"); + let y = unsafe { other.inline }; + word_is_superset(x, y) + } else if other.is_empty_unallocated() { + true + } else { + let other_on_heap = other.on_heap().unwrap(); + if self.is_empty_unallocated() { + other_on_heap.is_empty() + } else { + let on_heap = self.on_heap().unwrap(); + let self_slice = on_heap.as_slice(); + let other_slice = other_on_heap.as_slice(); + debug_assert_eq!( + self_slice.len(), + other_slice.len(), + "bit sets have different domain sizes" + ); + self_slice.iter().zip(other_slice).all(|(&x, &y)| (!x & y) == 0) + } + } + } + + /// Count the number of set bits in the set. + #[inline(always)] + pub fn count(&self) -> usize { + if self.is_inline() { + let x = unsafe { self.inline }; + x.count_ones() as usize - 1 + } else if self.is_empty_unallocated() { + 0 + } else { + self.on_heap().unwrap().as_slice().iter().map(|w| w.count_ones() as usize).sum() + } + } + + /// Returns an iterator over the indices for all elements in this set. + #[inline(always)] + pub fn iter_usizes(&self) -> BitIter<'_, usize> { + if self.is_inline() { + let x = unsafe { self.inline }; + // Remove the tag bit. + let without_tag_bit = x ^ Self::IS_INLINE_TAG_BIT; + BitIter::from_single_word(without_tag_bit) + } else if let Some(on_heap) = self.on_heap() { + BitIter::from_slice(on_heap.as_slice()) + } else { + debug_assert!(self.is_empty_unallocated()); + BitIter::from_single_word(0) + } + } + + /// Insert the elem with index `idx`. Returns `true` if the set has changed. + #[inline(always)] + fn insert_usize(&mut self, idx: usize) -> bool { + // Insert the `i`th bit in a word and return `true` if it changed. + let insert_bit = |word: &mut Word, bit_idx: u32| { + let mask = 0x01 << bit_idx; + let old = *word; + *word |= mask; + *word != old + }; + + if self.is_inline() { + let x = unsafe { &mut self.inline }; + debug_assert!(idx < Self::INLINE_CAPACITY, "index too large: {idx}"); + insert_bit(x, idx as u32) + } else { + let words = self.on_heap_get_or_alloc().as_mut_slice(); + + let word_idx = idx / WORD_BITS; + let bit_idx = (idx % WORD_BITS) as u32; + let word = &mut words[word_idx]; + insert_bit(word, bit_idx) + } + } + + /// Insert `0..domain_size` in the set. + /// + /// We would like an insert all function that doesn't require the domain size, but the exact + /// domain size is not stored so that is not possible. + #[inline(always)] + pub fn insert_all(&mut self, domain_size: usize) { + if self.is_inline() { + debug_assert!(domain_size <= Self::INLINE_CAPACITY); + unsafe { + self.inline |= Word::MAX.unbounded_shr(WORD_BITS as u32 - domain_size as u32) + }; + } else { + let on_heap = self.on_heap_get_or_alloc(); + debug_assert!(on_heap.capacity() >= domain_size, "domain size too big"); + let words = on_heap.as_mut_slice(); + + let (end_word_index, end_mask) = word_index_and_mask(domain_size - 1); + + for word_index in 0..end_word_index { + words[word_index] = Word::MAX; + } + + words[end_word_index] |= end_mask | (end_mask - 1); + } + } + + /// Sets `self = self | !other` for all elements less than `domain_size`. + #[inline(always)] + pub fn union_not(&mut self, other: &Self, domain_size: usize) { + if self.is_inline() { + assert!(other.is_inline()); + + let self_word = unsafe { &mut self.inline }; + let other_word = unsafe { other.inline }; + + debug_assert!(domain_size <= Self::INLINE_CAPACITY); + + *self_word |= !other_word & Word::MAX.unbounded_shr((WORD_BITS - domain_size) as u32); + } else if other.is_empty_unallocated() { + self.insert_all(domain_size); + } else { + let self_words = self.on_heap_get_or_alloc().as_mut_slice(); + let other_words = other.on_heap().unwrap().as_slice(); + + // Set all but the last word if domain_size is not divisible by `WORD_BITS`. + for (self_word, other_word) in + self_words.iter_mut().zip(other_words).take(domain_size / WORD_BITS) + { + *self_word |= !other_word; + } + + let remaining_bits = domain_size % WORD_BITS; + if remaining_bits > 0 { + let last_idx = domain_size / WORD_BITS; + self_words[last_idx] |= !other_words[last_idx] & !(Word::MAX << remaining_bits); + } + } + } + + /// Common function for union/intersection-like operations. + /// + /// This function takes two bit sets—one mutably, one immutably. Neither must be the + /// `empty_unallocated` variant. It asserts that they have the same `domain_size`, then applies a function to + /// each pair of words, effectively performing a zip-like operation. + /// It checks whether `self` has changed; if so, it returns `true`, otherwise `false`. + /// + /// ## Safety + /// + /// - Neither set must be `self.empty_unallocated`. + /// - If the sets are inlined, this will leave the tag bit set to 1. You must not modify it—doing so + /// results in undefined behaviour. This may be inconvenient for operations such as subtraction; + /// in such cases, use `binary_operation_safe` instead. + #[inline(always)] + unsafe fn binary_operation(&mut self, other: &Self, op: impl Fn(&mut Word, Word)) -> bool { + debug_assert!(!self.is_empty_unallocated()); + debug_assert!(!other.is_empty_unallocated()); + + // Apply `op` and return if the word changed. + let apply_and_check_change = |x: &mut Word, y: Word| -> bool { + let old = *x; + op(x, y); + *x != old + }; + + if self.is_inline() { + let x = unsafe { &mut self.inline }; + assert!(other.is_inline(), "bit sets has different domain sizes"); + let y = unsafe { other.inline }; + apply_and_check_change(x, y) + } else { + let self_on_heap = unsafe { &mut self.on_heap }; + assert!(!other.is_inline(), "bit sets has different domain sizes"); + let other_on_heap = unsafe { &other.on_heap }; + let self_slice = self_on_heap.as_mut_slice(); + let other_slice = other_on_heap.as_slice(); + assert_eq!(self_slice.len(), other_slice.len(), "bit sets have different domain sizes"); + let mut has_changed = false; + for (x, y) in self_slice.iter_mut().zip(other_slice) { + has_changed |= apply_and_check_change(x, *y); + } + has_changed + } + } + + /// Similar to [`Self::binary_operation`], but restores the tag bit if it has changed. + /// + /// Note that the tag bit will still be set in the call to `op`, but there is no danger in + /// changing it as it will be restored afterwords. + /// + /// ## Safety + /// + /// Neither set must be `self.empty_unallocated`. + #[inline(always)] + unsafe fn binary_operation_safe(&mut self, other: &Self, op: impl Fn(&mut Word, Word)) -> bool { + debug_assert!(!self.is_empty_unallocated()); + debug_assert!(!other.is_empty_unallocated()); + + if self.is_inline() { + let x = unsafe { &mut self.inline }; + assert!(other.is_inline(), "bit sets has different domain sizes"); + let y = unsafe { other.inline }; + + let old = *x; + op(x, y); + *x |= Self::IS_INLINE_TAG_BIT; + old != *x + } else { + let self_on_heap = unsafe { &mut self.on_heap }; + assert!(!other.is_inline(), "bit sets has different domain sizes"); + let other_on_heap = unsafe { &other.on_heap }; + let self_slice = self_on_heap.as_mut_slice(); + let other_slice = other_on_heap.as_slice(); + assert_eq!(self_slice.len(), other_slice.len(), "bit sets have different domain sizes"); + let mut has_changed = false; + for (x, y) in self_slice.iter_mut().zip(other_slice) { + let old = *x; + op(x, *y); + has_changed |= old != *x; + } + has_changed + } + } + + super::bit_relations_inherent_impls! {} +} + +impl BitRelations> for DenseBitSet { + #[inline(always)] + fn union(&mut self, other: &Self) -> bool { + if self.is_empty_unallocated() { + debug_assert!(!other.is_inline()); + *self = other.clone(); + !self.is_empty() + } else if other.is_empty_unallocated() { + false + } else { + // SAFETY: The union operation does not remove any bit set to 1, so the tag bit is + // unaffected. + unsafe { self.binary_operation(other, |x, y| *x |= y) } + } + } + + #[inline(always)] + fn intersect(&mut self, other: &Self) -> bool { + if self.is_empty_unallocated() { + false + } else if other.is_empty_unallocated() { + debug_assert!(!self.is_inline()); + let was_empty = self.is_empty(); + self.clear(); + !was_empty + } else { + // SAFETY: Since the tag bit is set in both `self` and `other`, the intersection won't + // remove it. + unsafe { self.binary_operation(other, |x, y| *x &= y) } + } + } + + #[inline(always)] + fn subtract(&mut self, other: &Self) -> bool { + if self.is_empty_unallocated() || other.is_empty_unallocated() { + false + } else { + unsafe { self.binary_operation_safe(other, |x, y| *x &= !y) } + } + } +} + +impl DenseBitSet { + /// Checks if the bit set contains `elem`. + #[inline(always)] + pub fn contains(&self, elem: T) -> bool { + // Check if the `i`th bit is set in a word. + let contains_bit = |word: Word, bit_idx: u32| { + let mask = 0x01 << bit_idx; + (word & mask) != 0 + }; + + let idx = elem.index(); + if self.is_inline() { + let x = unsafe { self.inline }; + debug_assert!(idx < Self::INLINE_CAPACITY, "index too large: {idx}"); + contains_bit(x, idx as u32) + } else if let Some(on_heap) = self.on_heap() { + let word_idx = idx / WORD_BITS; + let bit_idx = (idx % WORD_BITS) as u32; + let word = on_heap.as_slice()[word_idx]; + contains_bit(word, bit_idx) + } else { + debug_assert!(self.is_empty_unallocated()); + false + } + } + + /// Insert `elem`. Returns `true` if the set has changed. + #[inline(always)] + pub fn insert(&mut self, elem: T) -> bool { + self.insert_usize(elem.index()) + } + + /// Remove `elem`. Returns `true` if the set has changed. + #[inline(always)] + pub fn remove(&mut self, elem: T) -> bool { + // Remove the `i`th bit in a word and return `true` if it changed. + let remove_bit = |word: &mut Word, bit_idx: u32| { + let mask = !(0x01 << bit_idx); + let old = *word; + *word &= mask; + *word != old + }; + + let idx = elem.index(); + if self.is_inline() { + let x = unsafe { &mut self.inline }; + debug_assert!(idx < Self::INLINE_CAPACITY, "index too large: {idx}"); + remove_bit(x, idx as u32) + } else if let Some(on_heap) = self.on_heap_mut() { + let word_idx = idx / WORD_BITS; + let bit_idx = (idx % WORD_BITS) as u32; + let word = &mut on_heap.as_mut_slice()[word_idx]; + remove_bit(word, bit_idx) + } else { + debug_assert!(self.is_empty_unallocated()); + // Nothing to be removed. + false + } + } + + /// Returns an iterator over all elements in this set. + #[inline(always)] + pub fn iter(&self) -> BitIter<'_, T> { + if self.is_inline() { + let x = unsafe { self.inline }; + // Remove the tag bit. + let without_tag_bit = x ^ Self::IS_INLINE_TAG_BIT; + BitIter::from_single_word(without_tag_bit) + } else if let Some(on_heap) = self.on_heap() { + BitIter::from_slice(on_heap.as_slice()) + } else { + debug_assert!(self.is_empty_unallocated()); + BitIter::from_single_word(0) + } + } + + /// Returns `Some(elem)` if the set contains exactly one elemement otherwise returns `None`. + #[inline(always)] + pub fn only_one_elem(&self) -> Option { + if self.is_inline() { + let word = unsafe { self.inline } ^ Self::IS_INLINE_TAG_BIT; + if word.is_power_of_two() { Some(T::new(word.trailing_zeros() as usize)) } else { None } + } else if self.is_empty_unallocated() { + None + } else { + let words = self.on_heap().unwrap().as_slice(); + let mut found_elem = None; + for (i, &word) in words.iter().enumerate() { + if word > 0 { + if found_elem.is_some() { + return None; + } + if word.is_power_of_two() { + found_elem = + Some(T::new(i * WORD_BITS as usize + word.trailing_zeros() as usize)); + } else { + return None; + } + } + } + found_elem + } + } + + #[inline] + pub fn insert_range(&mut self, range: Range) { + if let Some(end) = range.end.index().checked_sub(1) { + self.insert_range_inclusive(RangeInclusive::new(range.start, Idx::new(end))); + } + } + + #[inline(always)] + pub fn insert_range_inclusive(&mut self, range: RangeInclusive) { + let start = range.start().index(); + let end = range.end().index(); + + if start > end { + return; + } + + if self.is_inline() { + debug_assert!(end < Self::INLINE_CAPACITY); + let mask = (1 << end) | ((1 << end) - (1 << start)); + unsafe { self.inline |= mask }; + } else { + let words = self.on_heap_get_or_alloc().as_mut_slice(); + + let (start_word_index, start_mask) = word_index_and_mask(start); + let (end_word_index, end_mask) = word_index_and_mask(end); + + // Set all words in between start and end (exclusively of both). + for word_index in (start_word_index + 1)..end_word_index { + words[word_index] = !0; + } + + if start_word_index != end_word_index { + // Start and end are in different words, so we handle each in turn. + // + // We set all leading bits. This includes the start_mask bit. + words[start_word_index] |= !(start_mask - 1); + // And all trailing bits (i.e. from 0..=end) in the end word, + // including the end. + words[end_word_index] |= end_mask | (end_mask - 1); + } else { + words[start_word_index] |= end_mask | (end_mask - start_mask); + } + } + } + + #[inline(always)] + pub fn last_set_in(&self, range: RangeInclusive) -> Option { + let start = range.start().index(); + let end = range.end().index(); + + if start > end { + return None; + } + + if self.is_inline() { + debug_assert!(end < Self::INLINE_CAPACITY); + let mut word = unsafe { self.inline } ^ Self::IS_INLINE_TAG_BIT; + let end_bit = 1 << end; + // Set all bits mor significant than `end_bit` to zero. + word &= end_bit | (end_bit - 1); + if word != 0 { + let pos = max_bit(word); + if start <= pos { Some(T::new(pos)) } else { None } + } else { + None + } + } else if let Some(on_heap) = self.on_heap() { + let words = on_heap.as_slice(); + + let (start_word_index, _) = word_index_and_mask(start); + let (end_word_index, end_mask) = word_index_and_mask(end); + + let end_word = words[end_word_index] & (end_mask | (end_mask - 1)); + if end_word != 0 { + let pos = max_bit(end_word) + WORD_BITS * end_word_index; + if start <= pos { + return Some(T::new(pos)); + } + } + + // We exclude end_word_index from the range here, because we don't want + // to limit ourselves to *just* the last word: the bits set it in may be + // after `end`, so it may not work out. + if let Some(offset) = + words[start_word_index..end_word_index].iter().rposition(|&w| w != 0) + { + let word_idx = start_word_index + offset; + let start_word = words[word_idx]; + let pos = max_bit(start_word) + WORD_BITS * word_idx; + if start <= pos { Some(T::new(pos)) } else { None } + } else { + None + } + } else { + debug_assert!(self.is_empty_unallocated()); + None + } + } +} + +impl BitRelations> for DenseBitSet { + fn union(&mut self, other: &ChunkedBitSet) -> bool { + other.iter().fold(false, |changed, elem| self.insert(elem) || changed) + } + + fn subtract(&mut self, _other: &ChunkedBitSet) -> bool { + unimplemented!("implement if/when necessary"); + } + + fn intersect(&mut self, other: &ChunkedBitSet) -> bool { + if self.is_inline() { + assert!(other.domain_size <= Self::INLINE_CAPACITY); + if other.domain_size == 0 { + return false; + } + + let word = unsafe { &mut self.inline }; + let old_word = *word; + match &other.chunks[0] { + Chunk::Zeros(d) => { + debug_assert_eq!(usize::from(*d), other.domain_size); + let mask = Word::MAX << other.domain_size(); + *word &= mask; + } + Chunk::Ones(_) => (), + Chunk::Mixed(d, _, words) => { + debug_assert_eq!(usize::from(*d), other.domain_size); + *word &= words[0] | Self::IS_INLINE_TAG_BIT; + } + } + *word != old_word + } else if let Some(on_heap) = self.on_heap_mut() { + let all_words = on_heap.as_mut_slice(); + + let mut changed = false; + for (i, chunk) in other.chunks.iter().enumerate() { + let mut words = &mut all_words[i * CHUNK_WORDS..]; + if words.len() > CHUNK_WORDS { + words = &mut words[..CHUNK_WORDS]; + } + match chunk { + Chunk::Zeros(..) => { + for word in words { + if *word != 0 { + changed = true; + *word = 0; + } + } + } + Chunk::Ones(..) => (), + Chunk::Mixed(_, _, data) => { + for (i, word) in words.iter_mut().enumerate() { + let new_val = *word & data[i]; + if new_val != *word { + changed = true; + *word = new_val; + } + } + } + } + } + changed + } else { + debug_assert!(self.is_empty_unallocated()); + false + } + } +} + +impl Encodable for DenseBitSet { + #[inline(never)] // FIXME: For profiling purposes + fn encode(&self, s: &mut S) { + // The encoding is as follows: + // + // The `inline` and `empty_unallocated` variants are encoded as a single `Word`. Here, we + // consider the `empty_unallocated` variant as the `inline` variant because + // `empty_unallocated: usize`, `inline: Word`, and `usize` is smaller than `Word`. + // + // The `on_heap` variant is encoded as follows: First, the number of `Word`s are encoded + // with a single `Word`. We assert that the two most significant bits of this number are 0 + // to distinguish it from the `inline` and `empty_unallocated` variants. Then all the words are + // encoded in sequence. + + if let Some(on_heap) = self.on_heap() { + let n_words: Word = on_heap.n_words(); + debug_assert_eq!( + n_words >> WORD_BITS - 2, + 0x0, + "the two most significant bits must be 0" + ); + n_words.encode(s); + debug_assert_eq!(n_words as usize, on_heap.as_slice().len()); + for word in on_heap.as_slice().iter() { + word.encode(s); + } + } else { + let word = unsafe { self.inline }; + debug_assert!(word >> WORD_BITS - 2 != 0, "the 2 most significant bits must not be 0"); + word.encode(s); + } + } +} + +impl Decodable for DenseBitSet { + #[inline(never)] // FIXME: For profiling purposes + fn decode(d: &mut D) -> Self { + // First we read one `Word` and check the variant. + let word = Word::decode(d); + if word >> WORD_BITS - 2 == 0x0 { + // If the two most significant bits are 0, then this is the `on_heap` variant and the + // number of words is encoded by `word`. + let n_words = word as usize; + assert!( + n_words > 0, + "DenseBitSet decoder error: At least one word must be stored with the `on_heap` variant." + ); + let mut on_heap = BitSetOnHeap::new_empty(n_words); + + let words = on_heap.as_mut_slice(); + // All `words` are now initialised to 0x0. + debug_assert_eq!(words.len(), n_words); + + // Decode the words one-by-one. + for word in words.iter_mut() { + *word = Word::decode(d); + } + + DenseBitSet { on_heap: ManuallyDrop::new(on_heap) } + } else { + // Both the `inline` and `empty_unallocated` variants are encoded by one `Word`. We can + // just assume the `inline` variant because the `empty_unallocated` variant is smaller + // and the union is `repr(C)`. + Self { inline: word } + } + } +} + +impl Clone for DenseBitSet { + #[inline(always)] + fn clone(&self) -> Self { + if self.is_inline() { + let inline = unsafe { self.inline }; + Self { inline } + } else if self.is_empty_unallocated() { + let empty_unallocated = unsafe { self.empty_unallocated }; + Self { empty_unallocated } + } else { + let old_on_heap = unsafe { &self.on_heap }; + let on_heap = old_on_heap.clone(); + Self { on_heap } + } + } +} + +impl Drop for DenseBitSet { + #[inline(always)] + fn drop(&mut self) { + // Deallocate if `self` is not inlined. + if let Some(on_heap) = self.on_heap_mut() { + unsafe { + ManuallyDrop::drop(on_heap); + } + } + } +} + +/// A pointer to a dense bit set stored on the heap. +/// +/// This struct is a `usize`, with its two most significant bits always set to 0. If the value is +/// shifted left by 2 bits, it yields a pointer to a sequence of words on the heap. The first word +/// in this sequence represents the length—it indicates how many words follow. These subsequent +/// words make up the actual bit set. +/// +/// For example, suppose the bit set should support a domain size of 240 bits. We first determine +/// how many words are needed to store 240 bits—that’s 4 words, assuming `[WORD_BITS] == 64`. +/// The pointer in this struct then points to a sequence of five words allocated on the heap. The +/// first word has the value 4 (the length), and the remaining four words comprise the bit set. +#[repr(transparent)] +struct BitSetOnHeap(usize); + +impl BitSetOnHeap { + fn new_empty(len: usize) -> Self { + debug_assert!(len >= 1); + + // The first word is used to store the total number of words. The rest of the words + // store the bits. + let num_words = len + 1; + + let layout = Layout::array::(num_words).expect("Bit set too large"); + // SAFETY: `num_words` is always at least `1` so we never allocate zero size. + let ptr = unsafe { alloc_zeroed(layout).cast::() }; + let Some(ptr) = NonNull::::new(ptr) else { + handle_alloc_error(layout); + }; + + // Store the length in the first word. + unsafe { ptr.write(len as Word) }; + + // Convert `ptr` to a `usize` and shift it two bits to the right. + BitSetOnHeap((ptr.as_ptr() as usize) >> 2) + } + + /// Get a slice with all bits in this bit set. + /// + /// Note that the number of bits in the set is rounded up to the next power of `Usize::BITS`. So + /// if the user requested a domain_size of 216 bits, a slice with 4 words will be returned on a + /// 64-bit machine. + #[inline] + fn as_slice(&self) -> &[Word] { + let ptr = (self.0 << 2) as *const Word; + let len = unsafe { ptr.read() } as usize; + // The slice starts at the second word. + unsafe { slice::from_raw_parts(ptr.add(1), len) } + } + + /// Get a mutable slice with all bits in this bit set. + /// + /// Note that the number of bits in the set is rounded up to the next power of `Usize::BITS`. So + /// if the user requested a domain_size of 216 bits, a slice with 4 words will be returned on a + /// 64-bit machine. + #[inline] + fn as_mut_slice(&mut self) -> &mut [Word] { + let ptr = (self.0 << 2) as *mut Word; + let len = unsafe { ptr.read() } as usize; + // The slice starts at the second word. + unsafe { slice::from_raw_parts_mut(ptr.add(1), len) } + } + + /// Check if the set is empty. + fn is_empty(&self) -> bool { + self.as_slice().iter().all(|&x| x == 0) + } + + /// Get the number of words. + #[allow(dead_code)] // FIXME + #[inline] + fn n_words(&self) -> Word { + let ptr = (self.0 << 2) as *const Word; + unsafe { ptr.read() } + } + + /// Get the capacity, that is the number of elements that can be stored in this set. + fn capacity(&self) -> usize { + let ptr = (self.0 << 2) as *const Word; + let len = unsafe { ptr.read() } as usize; + len * WORD_BITS + } + + /// Make sure the set can hold at least `min_domain_size` elements. Reallocate if necessary. + fn ensure_capacity(&mut self, min_domain_size: usize) { + let len = min_domain_size.div_ceil(WORD_BITS); + + let old_ptr = (self.0 << 2) as *const Word; + let old_len = unsafe { old_ptr.read() } as usize; + + if len <= old_len { + return; + } + + // The first word is used to store the total number of words. The rest of the words + // store the bits. + let num_words = len + 1; + let old_num_words = old_len + 1; + + let new_layout = Layout::array::(num_words).expect("Bit set too large"); + let old_layout = Layout::array::(old_num_words).expect("Bit set too large"); + + // SAFETY: `num_words` is always at least `1` so we never allocate zero size. + let ptr = + unsafe { realloc(old_ptr as *mut u8, old_layout, new_layout.size()).cast::() }; + let Some(ptr) = NonNull::::new(ptr) else { + handle_alloc_error(new_layout); + }; + + // Store the length in the first word. + unsafe { ptr.write(len as Word) }; + + // Set all the new words to 0. + for word_idx in old_num_words..num_words { + unsafe { ptr.add(word_idx).write(0x0) } + } + + // Convert `ptr` to a `usize` and shift it two bits to the right. + self.0 = (ptr.as_ptr() as usize) >> 2 + } +} + +impl Clone for BitSetOnHeap { + fn clone(&self) -> Self { + let ptr = (self.0 << 2) as *const Word; + let len = unsafe { ptr.read() } as usize; + let num_words = len + 1; + + let layout = Layout::array::(num_words).expect("Bit set too large"); + // SAFETY: `num_words` is always at least `1` so we never allocate zero size. + let new_ptr = unsafe { alloc(layout).cast::() }; + let Some(new_ptr) = NonNull::::new(new_ptr) else { + handle_alloc_error(layout); + }; + + unsafe { ptr.copy_to_nonoverlapping(new_ptr.as_ptr(), num_words) }; + + BitSetOnHeap((new_ptr.as_ptr() as usize) >> 2) + } +} + +impl Drop for BitSetOnHeap { + fn drop(&mut self) { + let ptr = (self.0 << 2) as *mut Word; + + // SAFETY: The first word stores the number of words for the bit set. We have to add 1 + // because the first word storing the length is allocated as well. + let num_words = unsafe { ptr.read() } as usize + 1; + let layout = Layout::array::(num_words).expect("Bit set too large"); + // SAFETY: We know that `on_heap` has been allocated with the same layout. See the + // `new` method for reference. + unsafe { dealloc(ptr.cast::(), layout) }; + } +} + +pub struct BitIter<'a, T: Idx> { + /// A copy of the current word, but with any already-visited bits cleared. + /// (This lets us use `trailing_zeros()` to find the next set bit.) When it + /// is reduced to 0, we move onto the next word. + word: Word, + + /// The offset (measured in bits) of the current word. + offset: usize, + + /// Underlying iterator over the words. + iter: slice::Iter<'a, Word>, + + marker: PhantomData, +} + +impl<'a, T: Idx> BitIter<'a, T> { + pub(super) fn from_slice(words: &'a [Word]) -> Self { + // We initialize `word` and `offset` to degenerate values. On the first + // call to `next()` we will fall through to getting the first word from + // `iter`, which sets `word` to the first word (if there is one) and + // `offset` to 0. Doing it this way saves us from having to maintain + // additional state about whether we have started. + Self { + word: 0, + offset: usize::MAX - (WORD_BITS - 1), + iter: words.iter(), + marker: PhantomData, + } + } + + #[inline(always)] + fn from_single_word(word: Word) -> Self { + Self { word, offset: 0, iter: [].iter(), marker: PhantomData } + } +} + +impl<'a, T: Idx> Iterator for BitIter<'a, T> { + type Item = T; + + #[inline(always)] + fn next(&mut self) -> Option { + loop { + if self.word != 0 { + // Get the position of the next set bit in the current word, + // then clear the bit. + let bit_pos = self.word.trailing_zeros() as usize; + self.word ^= 0x01 << bit_pos; + return Some(T::new(bit_pos + self.offset)); + } + + // Move onto the next word. `wrapping_add()` is needed to handle + // the degenerate initial value given to `offset` in `new()`. + self.word = *self.iter.next()?; + self.offset = self.offset.wrapping_add(WORD_BITS); + } + } +} + +impl<'a, T: Idx> FusedIterator for BitIter<'a, T> {} + +impl fmt::Debug for DenseBitSet { + fn fmt(&self, w: &mut fmt::Formatter<'_>) -> fmt::Result { + w.debug_list().entries(self.iter()).finish() + } +} + +impl PartialEq for DenseBitSet { + #[inline] + fn eq(&self, other: &Self) -> bool { + if self.is_inline() { + if other.is_inline() { + unsafe { self.inline == other.inline } + } else if other.is_empty_unallocated() { + self.is_empty() + } else { + let other_words = other.on_heap().unwrap().as_slice(); + let self_word = unsafe { self.inline } ^ Self::IS_INLINE_TAG_BIT; + other_words[0] == self_word && other_words[1..].iter().all(|&w| w == 0) + } + } else if self.is_empty_unallocated() { + other.is_empty() + } else { + let self_words = self.on_heap().unwrap().as_slice(); + if other.is_empty_unallocated() { + self_words.iter().all(|&w| w == 0) + } else if other.is_inline() { + let other_word = unsafe { other.inline } ^ Self::IS_INLINE_TAG_BIT; + self_words[0] == other_word && self_words[1..].iter().all(|&w| w == 0) + } else { + let mut self_words = self_words.iter(); + let mut other_words = other.on_heap().unwrap().as_slice().iter(); + loop { + match (self_words.next(), other_words.next()) { + (Some(w1), Some(w2)) if w1 == w2 => (), + (Some(_), Some(_)) => break false, + (Some(0), None) | (None, Some(0)) => (), + (Some(_), None) | (None, Some(_)) => break false, + (None, None) => break true, + } + } + } + } + } +} + +impl Eq for DenseBitSet {} + +impl Hash for DenseBitSet { + #[inline] + fn hash(&self, hasher: &mut H) { + if self.is_inline() { + let inline = unsafe { self.inline }; + inline.hash(hasher); + } else if let Some(num_words) = self.empty_unallocated_get_num_words() { + // Now we hash 0 for `num_words` times so that this hash should be equal to a cleared + // set with the `on_heap` variant. + for _ in 0..num_words { + let zero_word: Word = 0x0; + zero_word.hash(hasher); + } + } else { + let words = self.on_heap().unwrap().as_slice(); + for word in words { + word.hash(hasher); + } + } + } +} + +/// A resizable bitset type with a dense representation. +/// +/// `T` is an index type, typically a newtyped `usize` wrapper, but it can also +/// just be `usize`. +/// +/// All operations that involve an element will panic if the element is equal +/// to or greater than the domain size. +#[derive(Clone, PartialEq)] +pub struct GrowableBitSet { + bit_set: DenseBitSet, +} + +impl Default for GrowableBitSet { + fn default() -> Self { + GrowableBitSet::new_empty() + } +} + +impl GrowableBitSet { + /// Ensure that the set can hold at least `min_domain_size` elements. + pub fn ensure(&mut self, min_domain_size: usize) { + if min_domain_size <= self.bit_set.capacity() { + return; + } + + if self.bit_set.is_inline() { + // The set must change from being inlined to allocate on the heap. + debug_assert!(min_domain_size > DenseBitSet::::INLINE_CAPACITY); + + let mut new_bit_set = DenseBitSet::new_empty(min_domain_size); + if !self.bit_set.is_empty() { + // SAFETY: We know that `self.is_inline()` is true. + let word = unsafe { self.bit_set.inline } ^ DenseBitSet::::IS_INLINE_TAG_BIT; + new_bit_set.on_heap_get_or_alloc().as_mut_slice()[0] = word; + } + self.bit_set = new_bit_set; + } else if self.bit_set.is_empty_unallocated() { + self.bit_set = DenseBitSet::new_empty(min_domain_size); + } else { + self.bit_set.on_heap_mut().unwrap().ensure_capacity(min_domain_size); + } + } + + pub fn new_empty() -> GrowableBitSet { + GrowableBitSet { bit_set: DenseBitSet::new_empty(0) } + } + + pub fn with_capacity(capacity: usize) -> GrowableBitSet { + GrowableBitSet { bit_set: DenseBitSet::new_empty(capacity) } + } + + /// Insert the element with index `idx`. Returns `true` if the set has changed. + #[inline] + pub fn insert_usize(&mut self, idx: usize) -> bool { + self.ensure(idx + 1); + self.bit_set.insert_usize(idx) + } +} + +impl GrowableBitSet { + /// Insert `elem` into the set, resizing if necessary. Returns `true` if the set has changed. + #[inline] + pub fn insert(&mut self, elem: T) -> bool { + self.insert_usize(elem.index()) + } + + /// Returns `true` if the set has changed. + #[inline] + pub fn remove(&mut self, elem: T) -> bool { + self.ensure(elem.index() + 1); + self.bit_set.remove(elem) + } + + #[inline] + pub fn is_empty(&self) -> bool { + self.bit_set.is_empty() + } + + #[inline] + pub fn contains(&self, elem: T) -> bool { + elem.index() < self.bit_set.capacity() && self.bit_set.contains(elem) + } + + #[inline] + pub fn iter(&self) -> BitIter<'_, T> { + self.bit_set.iter() + } + + #[inline] + pub fn len(&self) -> usize { + self.bit_set.count() + } +} + +impl From> for GrowableBitSet { + fn from(bit_set: DenseBitSet) -> Self { + Self { bit_set } + } +} + +impl From> for DenseBitSet { + fn from(bit_set: GrowableBitSet) -> Self { + bit_set.bit_set + } +} + +impl fmt::Debug for GrowableBitSet { + fn fmt(&self, w: &mut fmt::Formatter<'_>) -> fmt::Result { + self.bit_set.fmt(w) + } +} + +#[inline] +fn max_bit(word: Word) -> usize { + WORD_BITS - 1 - word.leading_zeros() as usize +} diff --git a/compiler/rustc_index/src/bit_set/tests.rs b/compiler/rustc_index/src/bit_set/tests.rs index 323a66ddc6f20..eea19cb11a101 100644 --- a/compiler/rustc_index/src/bit_set/tests.rs +++ b/compiler/rustc_index/src/bit_set/tests.rs @@ -1,9 +1,583 @@ +use std::collections::BTreeSet; +use std::hash::{BuildHasher, BuildHasherDefault, DefaultHasher}; +use std::hint::black_box; +use std::ops::{Range, RangeBounds, RangeInclusive}; + +use rustc_serialize::{Decodable, Decoder, Encodable, Encoder}; +use test::Bencher; + use super::*; +use crate::IndexVec; extern crate test; -use std::hint::black_box; -use test::Bencher; +/// A very simple pseudo random generator using linear xorshift. +/// +/// [See Wikipedia](https://en.wikipedia.org/wiki/Xorshift). This has 64-bit state and a period +/// of `2^64 - 1`. +struct Rng(u64); + +impl Rng { + fn new(seed: u64) -> Self { + Rng(seed) + } + + fn next(&mut self) -> usize { + self.0 ^= self.0 << 7; + self.0 ^= self.0 >> 9; + self.0 as usize + } + + fn next_bool(&mut self) -> bool { + self.next() % 2 == 0 + } + + /// Sample a range, a subset of `0..=max`. + /// + /// The purpose of this method is to make edge cases such as `0..=max` more common. + fn sample_range(&mut self, max: usize) -> RangeInclusive { + let start = match self.next() % 3 { + 0 => 0, + 1 => max, + 2 => self.next() % (max + 1), + _ => unreachable!(), + }; + let end = match self.next() % 3 { + 0 => 0, + 1 => max, + 2 => self.next() % (max + 1), + _ => unreachable!(), + }; + RangeInclusive::new(start, end) + } +} + +#[derive(Default)] +struct EncoderLittleEndian { + bytes: Vec, +} + +impl Encoder for EncoderLittleEndian { + fn emit_usize(&mut self, v: usize) { + self.bytes.extend(v.to_le_bytes()); + } + fn emit_u8(&mut self, v: u8) { + self.bytes.extend(v.to_le_bytes()); + } + fn emit_u16(&mut self, v: u16) { + self.bytes.extend(v.to_le_bytes()); + } + fn emit_u32(&mut self, v: u32) { + self.bytes.extend(v.to_le_bytes()); + } + fn emit_u64(&mut self, v: u64) { + self.bytes.extend(v.to_le_bytes()); + } + fn emit_u128(&mut self, v: u128) { + self.bytes.extend(v.to_le_bytes()); + } + fn emit_isize(&mut self, v: isize) { + self.bytes.extend(v.to_le_bytes()); + } + fn emit_i8(&mut self, v: i8) { + self.bytes.extend(v.to_le_bytes()); + } + fn emit_i16(&mut self, v: i16) { + self.bytes.extend(v.to_le_bytes()); + } + fn emit_i32(&mut self, v: i32) { + self.bytes.extend(v.to_le_bytes()); + } + fn emit_i64(&mut self, v: i64) { + self.bytes.extend(v.to_le_bytes()); + } + fn emit_i128(&mut self, v: i128) { + self.bytes.extend(v.to_le_bytes()); + } + fn emit_raw_bytes(&mut self, v: &[u8]) { + self.bytes.extend(v); + } +} + +struct DecoderLittleEndian<'a> { + bytes: &'a [u8], + /// Remember the original `bytes.len()` so we can calculate how many bytes we've read. + original_len: usize, +} + +impl<'a> DecoderLittleEndian<'a> { + fn new(bytes: &'a [u8]) -> Self { + Self { bytes, original_len: bytes.len() } + } +} + +impl<'a> Decoder for DecoderLittleEndian<'a> { + fn read_usize(&mut self) -> usize { + let (int_bytes, rest) = self.bytes.split_at(size_of::()); + self.bytes = rest; + usize::from_le_bytes(int_bytes.try_into().unwrap()) + } + fn read_u128(&mut self) -> u128 { + let (int_bytes, rest) = self.bytes.split_at(size_of::()); + self.bytes = rest; + u128::from_le_bytes(int_bytes.try_into().unwrap()) + } + fn read_u64(&mut self) -> u64 { + let (int_bytes, rest) = self.bytes.split_at(size_of::()); + self.bytes = rest; + u64::from_le_bytes(int_bytes.try_into().unwrap()) + } + fn read_u32(&mut self) -> u32 { + let (int_bytes, rest) = self.bytes.split_at(size_of::()); + self.bytes = rest; + u32::from_le_bytes(int_bytes.try_into().unwrap()) + } + fn read_u16(&mut self) -> u16 { + let (int_bytes, rest) = self.bytes.split_at(size_of::()); + self.bytes = rest; + u16::from_le_bytes(int_bytes.try_into().unwrap()) + } + fn read_u8(&mut self) -> u8 { + let (int_bytes, rest) = self.bytes.split_at(size_of::()); + self.bytes = rest; + u8::from_le_bytes(int_bytes.try_into().unwrap()) + } + fn read_isize(&mut self) -> isize { + let (int_bytes, rest) = self.bytes.split_at(size_of::()); + self.bytes = rest; + isize::from_le_bytes(int_bytes.try_into().unwrap()) + } + fn read_i128(&mut self) -> i128 { + let (int_bytes, rest) = self.bytes.split_at(size_of::()); + self.bytes = rest; + i128::from_le_bytes(int_bytes.try_into().unwrap()) + } + fn read_i64(&mut self) -> i64 { + let (int_bytes, rest) = self.bytes.split_at(size_of::()); + self.bytes = rest; + i64::from_le_bytes(int_bytes.try_into().unwrap()) + } + fn read_i32(&mut self) -> i32 { + let (int_bytes, rest) = self.bytes.split_at(size_of::()); + self.bytes = rest; + i32::from_le_bytes(int_bytes.try_into().unwrap()) + } + fn read_i16(&mut self) -> i16 { + let (int_bytes, rest) = self.bytes.split_at(size_of::()); + self.bytes = rest; + i16::from_le_bytes(int_bytes.try_into().unwrap()) + } + fn read_raw_bytes(&mut self, len: usize) -> &[u8] { + let (bytes, rest) = self.bytes.split_at(len); + self.bytes = rest; + bytes + } + fn peek_byte(&self) -> u8 { + self.bytes[0] + } + fn position(&self) -> usize { + self.original_len - self.bytes.len() + } +} + +fn test_with_domain_size(domain_size: usize) { + const TEST_ITERATIONS: u32 = 512; + + let mut set_1 = DenseBitSet::::new_empty(domain_size); + let mut set_1_reference = IndexVec::::from_elem_n(false, domain_size); + let mut set_2 = DenseBitSet::::new_empty(domain_size); + let mut set_2_reference = IndexVec::::from_elem_n(false, domain_size); + + let hasher = BuildHasherDefault::::new(); + + let mut encoder = EncoderLittleEndian::default(); + + let mut rng = Rng::new(42); + + for _ in 0..TEST_ITERATIONS { + // Make a random operation. + match rng.next() % 100 { + 0..20 => { + // Insert in one of the sets. + if domain_size == 0 { + continue; + } + let elem = rng.next() % domain_size; + // Choose set to insert into. + if rng.next_bool() { + assert_eq!(!set_1.contains(elem), set_1.insert(elem)); + set_1_reference[elem] = true; + } else { + assert_eq!(!set_2.contains(elem), set_2.insert(elem)); + set_2_reference[elem] = true; + } + } + 20..40 => { + // Insert a range in one of the sets. + if domain_size == 0 { + continue; + } + + let range = rng.sample_range(domain_size - 1); + // Choose set to insert into. + if rng.next_bool() { + set_1.insert_range_inclusive(range.clone()); + for i in range { + set_1_reference[i] = true; + } + } else { + set_2.insert_range_inclusive(range.clone()); + for i in range { + set_2_reference[i] = true; + } + } + } + 40..50 => { + // Test insert_all(). + if rng.next_bool() { + set_1.insert_all(domain_size); + for x in set_1_reference.iter_mut() { + *x = true; + } + } else { + set_2.insert_all(domain_size); + for x in set_2_reference.iter_mut() { + *x = true; + } + } + } + 50..70 => { + // Remove from one of the sets. + if domain_size == 0 { + continue; + } + let elem = rng.next() % domain_size; + // Choose set to remove into. + if rng.next_bool() { + assert_eq!(set_1.contains(elem), set_1.remove(elem),); + set_1_reference[elem] = false; + } else { + assert_eq!(set_2.contains(elem), set_2.remove(elem),); + set_2_reference[elem] = false; + } + } + 70..76 => { + // Union + let old_set_1 = set_1.clone(); + let changed = set_1.union(&set_2); + assert_eq!(changed, old_set_1 != set_1); + + // Adjust the reference sets. + for (x, val) in set_2_reference.iter_enumerated() { + set_1_reference[x] |= val; + } + } + 76..82 => { + // Intersection + let old_set_1 = set_1.clone(); + let changed = set_1.intersect(&set_2); + assert_eq!(changed, old_set_1 != set_1); + + // Adjust the reference sets. + for (x, val) in set_2_reference.iter_enumerated() { + set_1_reference[x] &= val; + } + } + 82..88 => { + // Subtraction + let old_set_1 = set_1.clone(); + let changed = set_1.subtract(&set_2); + assert_eq!(changed, old_set_1 != set_1); + + // Adjust the reference sets. + for (x, val) in set_2_reference.iter_enumerated() { + set_1_reference[x] &= !val; + } + } + 88..94 => { + // Union_not + set_1.union_not(&set_2, domain_size); + + // Adjust the reference sets. + for (x, val) in set_2_reference.iter_enumerated() { + set_1_reference[x] |= !val; + } + } + 94..97 => { + // Clear + if rng.next_bool() { + set_1.clear(); + for x in set_1_reference.iter_mut() { + *x = false; + } + } else { + set_2.clear(); + for x in set_2_reference.iter_mut() { + *x = false; + } + } + } + 97..100 => { + // Test new_filled(). + if rng.next_bool() { + set_1 = DenseBitSet::new_filled(domain_size); + for x in set_1_reference.iter_mut() { + *x = true; + } + } else { + set_2 = DenseBitSet::new_filled(domain_size); + for x in set_2_reference.iter_mut() { + *x = true; + } + } + } + _ => unreachable!(), + } + + // Check the contains function. + for i in 0..domain_size { + assert_eq!(set_1.contains(i), set_1_reference[i]); + assert_eq!(set_2.contains(i), set_2_reference[i]); + } + + // Check iter function. + assert!( + set_1.iter().eq(set_1_reference.iter_enumerated().filter(|&(_, &v)| v).map(|(x, _)| x)) + ); + assert!( + set_2.iter().eq(set_2_reference.iter_enumerated().filter(|&(_, &v)| v).map(|(x, _)| x)) + ); + + // Check the superset relation. + assert_eq!(set_1.superset(&set_2), set_2.iter().all(|x| set_1.contains(x))); + + // Check the `==` operator. + assert_eq!(set_1 == set_2, set_1_reference == set_2_reference); + + // Check the `hash()` function. + // If the `set_1` and `set_2` are equal, then their hashes must also be equal. + if set_1 == set_2 { + assert_eq!(hasher.hash_one(&set_1), hasher.hash_one(&set_2)); + } + + // Check the count function. + assert_eq!(set_1.count(), set_1_reference.iter().filter(|&&x| x).count()); + assert_eq!(set_2.count(), set_2_reference.iter().filter(|&&x| x).count()); + + // Check `only_one_elem()`. + if let Some(elem) = set_1.only_one_elem() { + assert_eq!(set_1.count(), 1); + assert_eq!(elem, set_1.iter().next().unwrap()); + } else { + assert_ne!(set_1.count(), 1); + } + + // Check `last_set_in()`. + if domain_size > 0 { + let range = rng.sample_range(domain_size - 1); + assert_eq!( + set_1.last_set_in(range.clone()), + range.clone().filter(|&i| set_1.contains(i)).last() + ); + assert_eq!( + set_2.last_set_in(range.clone()), + range.filter(|&i| set_2.contains(i)).last() + ); + } + + // Check `Encodable` and `Decodable` implementations. + if rng.next() as u32 % TEST_ITERATIONS < 128 { + set_1.encode(&mut encoder); + + let mut decoder = DecoderLittleEndian::new(&encoder.bytes); + let decoded = DenseBitSet::::decode(&mut decoder); + assert_eq!( + decoder.position(), + encoder.bytes.len(), + "All bytes must be read when decoding." + ); + + assert_eq!(set_1, decoded); + + encoder.bytes.clear(); + } + } +} + +fn test_relations_with_chunked_set(domain_size: usize) { + const TEST_ITERATIONS: u32 = 64; + + let mut dense_set = DenseBitSet::::new_empty(domain_size); + let mut chunked_set = ChunkedBitSet::new_empty(domain_size); + + let mut rng = Rng::new(42); + + for _ in 0..TEST_ITERATIONS { + // Make a random operation. + match rng.next() % 10 { + 0..3 => { + // Insert in one of the sets. + if domain_size == 0 { + continue; + } + let elem = rng.next() % domain_size; + // Choose set to insert into. + if rng.next_bool() { + dense_set.insert(elem); + } else { + chunked_set.insert(elem); + } + } + 3..6 => { + // Remove from one of the sets. + if domain_size == 0 { + continue; + } + let elem = rng.next() % domain_size; + // Choose set to remove into. + if rng.next_bool() { + dense_set.remove(elem); + } else { + chunked_set.remove(elem); + } + } + 6 => { + // Clear + if rng.next_bool() { + dense_set.clear(); + } else { + chunked_set.clear(); + } + } + 7 => { + // Fill. + if rng.next_bool() { + dense_set.insert_all(domain_size); + } else { + chunked_set.insert_all(); + } + } + 8 => { + // Union + let old_dense_set = dense_set.clone(); + let changed = dense_set.union(&chunked_set); + assert_eq!(old_dense_set != dense_set, changed); + assert!(dense_set.superset(&old_dense_set)); + assert!(chunked_set.iter().all(|x| dense_set.contains(x))); + + // Check that all the added elements come from `chunked_set`. + let mut difference = dense_set.clone(); + difference.subtract(&old_dense_set); + assert!(difference.iter().all(|x| chunked_set.contains(x))); + } + 9 => { + // Intersection + let old_dense_set = dense_set.clone(); + let changed = dense_set.intersect(&chunked_set); + assert_eq!(old_dense_set != dense_set, changed); + assert!(old_dense_set.superset(&dense_set)); + assert!(dense_set.iter().all(|x| chunked_set.contains(x))); + + // Check that no of the removed elements comes from `chunked_set`. + let mut difference = old_dense_set; // Just renaming. + difference.subtract(&dense_set); + assert!(difference.iter().all(|x| !chunked_set.contains(x))); + } + _ => unreachable!(), + } + } +} + +#[test] +fn test_dense_bit_set() { + assert_eq!( + size_of::>(), + size_of::(), + "DenseBitSet should have the same size as a Word" + ); + + test_with_domain_size(0); + test_with_domain_size(1); + test_with_domain_size(63); + test_with_domain_size(64); + test_with_domain_size(65); + test_with_domain_size(127); + test_with_domain_size(128); + test_with_domain_size(129); + + test_relations_with_chunked_set(0); + test_relations_with_chunked_set(1); + test_relations_with_chunked_set(CHUNK_BITS - 1); + test_relations_with_chunked_set(CHUNK_BITS); + test_relations_with_chunked_set(CHUNK_BITS + 2); + test_relations_with_chunked_set(3 * CHUNK_BITS - 2); + test_relations_with_chunked_set(3 * CHUNK_BITS); + test_relations_with_chunked_set(3 * CHUNK_BITS + 1); +} + +#[test] +fn test_growable_bit_set() { + const TEST_ITERATIONS: u32 = 512; + const MAX_ELEMS: usize = 314; + + let mut set = GrowableBitSet::::new_empty(); + let mut reference_set = BTreeSet::::new(); + + let mut rng = Rng::new(42); + + for _ in 0..TEST_ITERATIONS { + match rng.next() % 100 { + 0..30 => { + // Insert an element in the `0..=(DenseBitSet::INLINE_CAPACITY + 2)` range. + let elem = rng.next() % (DenseBitSet::::INLINE_CAPACITY + 3); + set.insert(elem); + reference_set.insert(elem); + } + 30..50 => { + // Insert an element in the `0..MAX_ELEMS` range. + let elem = rng.next() % MAX_ELEMS; + set.insert(elem); + reference_set.insert(elem); + } + 50..70 => { + // Remove an existing element. + let len = set.len(); + if len == 0 { + continue; + } + let elem = set.iter().nth(rng.next() % len).unwrap(); + set.remove(elem); + reference_set.remove(&elem); + } + 70..90 => { + // Remove an arbitrary element in the `0..MAX_ELEMS` range. + let elem = rng.next() % MAX_ELEMS; + set.remove(elem); + reference_set.remove(&elem); + } + 90..100 => { + // Make sure the `with_capacity()` function works. + let capacity = rng.next() % MAX_ELEMS; + set = GrowableBitSet::with_capacity(capacity); + reference_set.clear(); + } + _ => unreachable!(), + } + + // Check the `is_empty()` function. + assert_eq!(set.is_empty(), reference_set.is_empty()); + + // Check the `iter` function. + assert!(set.iter().eq(reference_set.iter().copied())); + + // Check the contains function with a 20 % probability. + if rng.next() % 5 == 0 { + for x in 0..MAX_ELEMS { + assert_eq!(set.contains(x), reference_set.contains(&x)); + } + } + } +} #[test] fn test_new_filled() { @@ -50,11 +624,11 @@ fn bitset_clone_from() { let mut b = DenseBitSet::new_empty(2); b.clone_from(&a); - assert_eq!(b.domain_size(), 10); + assert!(b.capacity() >= 10); assert_eq!(b.iter().collect::>(), [4, 7, 9]); b.clone_from(&DenseBitSet::new_empty(40)); - assert_eq!(b.domain_size(), 40); + assert!(b.capacity() >= 40); assert_eq!(b.iter().collect::>(), []); } @@ -91,7 +665,7 @@ fn union_not() { b.insert(81); // Already in `a`. b.insert(90); - a.union_not(&b); + a.union_not(&b, 100); // After union-not, `a` should contain all values in the domain, except for // the ones that are in `b` and were _not_ already in `a`. @@ -600,10 +1174,7 @@ fn sparse_matrix_operations() { #[test] fn dense_insert_range() { #[track_caller] - fn check(domain: usize, range: R) - where - R: RangeBounds + Clone + IntoIterator + std::fmt::Debug, - { + fn check_range(domain: usize, range: Range) { let mut set = DenseBitSet::new_empty(domain); set.insert_range(range.clone()); for i in set.iter() { @@ -613,32 +1184,45 @@ fn dense_insert_range() { assert!(set.contains(i), "{} in {:?}, inserted {:?}", i, set, range); } } - check(300, 10..10); - check(300, WORD_BITS..WORD_BITS * 2); - check(300, WORD_BITS - 1..WORD_BITS * 2); - check(300, WORD_BITS - 1..WORD_BITS); - check(300, 10..100); - check(300, 10..30); - check(300, 0..5); - check(300, 0..250); - check(300, 200..250); - - check(300, 10..=10); - check(300, WORD_BITS..=WORD_BITS * 2); - check(300, WORD_BITS - 1..=WORD_BITS * 2); - check(300, WORD_BITS - 1..=WORD_BITS); - check(300, 10..=100); - check(300, 10..=30); - check(300, 0..=5); - check(300, 0..=250); - check(300, 200..=250); + + #[track_caller] + fn check_range_inclusive(domain: usize, range: RangeInclusive) { + let mut set = DenseBitSet::new_empty(domain); + set.insert_range_inclusive(range.clone()); + for i in set.iter() { + assert!(range.contains(&i)); + } + for i in range.clone() { + assert!(set.contains(i), "{} in {:?}, inserted {:?}", i, set, range); + } + } + + check_range(300, 10..10); + check_range(300, WORD_BITS..WORD_BITS * 2); + check_range(300, WORD_BITS - 1..WORD_BITS * 2); + check_range(300, WORD_BITS - 1..WORD_BITS); + check_range(300, 10..100); + check_range(300, 10..30); + check_range(300, 0..5); + check_range(300, 0..250); + check_range(300, 200..250); + + check_range_inclusive(300, 10..=10); + check_range_inclusive(300, WORD_BITS..=WORD_BITS * 2); + check_range_inclusive(300, WORD_BITS - 1..=WORD_BITS * 2); + check_range_inclusive(300, WORD_BITS - 1..=WORD_BITS); + check_range_inclusive(300, 10..=100); + check_range_inclusive(300, 10..=30); + check_range_inclusive(300, 0..=5); + check_range_inclusive(300, 0..=250); + check_range_inclusive(300, 200..=250); for i in 0..WORD_BITS * 2 { for j in i..WORD_BITS * 2 { - check(WORD_BITS * 2, i..j); - check(WORD_BITS * 2, i..=j); - check(300, i..j); - check(300, i..=j); + check_range(WORD_BITS * 2, i..j); + check_range_inclusive(WORD_BITS * 2, i..=j); + check_range(300, i..j); + check_range_inclusive(300, i..=j); } } } @@ -656,7 +1240,7 @@ fn dense_last_set_before() { } #[track_caller] - fn cmp(set: &DenseBitSet, needle: impl RangeBounds + Clone + std::fmt::Debug) { + fn cmp(set: &DenseBitSet, needle: RangeInclusive) { assert_eq!( set.last_set_in(needle.clone()), easy(set, needle.clone()), @@ -672,20 +1256,18 @@ fn dense_last_set_before() { set.insert(WORD_BITS - 1); cmp(&set, 0..=WORD_BITS - 1); cmp(&set, 0..=5); - cmp(&set, 10..100); + cmp(&set, 10..=99); set.insert(100); - cmp(&set, 100..110); - cmp(&set, 99..100); + cmp(&set, 100..=119); + cmp(&set, 99..=99); cmp(&set, 99..=100); for i in 0..=WORD_BITS * 2 { for j in i..=WORD_BITS * 2 { for k in 0..WORD_BITS * 2 { let mut set = DenseBitSet::new_empty(300); - cmp(&set, i..j); cmp(&set, i..=j); set.insert(k); - cmp(&set, i..j); cmp(&set, i..=j); } } diff --git a/compiler/rustc_middle/src/values.rs b/compiler/rustc_middle/src/values.rs index 4d70a70873267..46bcc25dc0710 100644 --- a/compiler/rustc_middle/src/values.rs +++ b/compiler/rustc_middle/src/values.rs @@ -376,12 +376,8 @@ fn find_item_ty_spans( }); if check_params && let Some(args) = path.segments.last().unwrap().args { let params_in_repr = tcx.params_in_repr(def_id); - // the domain size check is needed because the HIR may not be well-formed at this point - for (i, arg) in args.args.iter().enumerate().take(params_in_repr.domain_size()) - { - if let hir::GenericArg::Type(ty) = arg - && params_in_repr.contains(i as u32) - { + for arg in params_in_repr.iter().map_while(|i| args.args.get(i as usize)) { + if let hir::GenericArg::Type(ty) = arg { find_item_ty_spans( tcx, ty.as_unambig_ty(), diff --git a/compiler/rustc_mir_dataflow/src/framework/cursor.rs b/compiler/rustc_mir_dataflow/src/framework/cursor.rs index 3f6e7a0661921..d45509d9de758 100644 --- a/compiler/rustc_mir_dataflow/src/framework/cursor.rs +++ b/compiler/rustc_mir_dataflow/src/framework/cursor.rs @@ -127,7 +127,7 @@ where #[cfg(test)] pub(crate) fn allow_unreachable(&mut self) { #[cfg(debug_assertions)] - self.reachable_blocks.insert_all() + self.reachable_blocks.insert_all(self.body().basic_blocks.len()) } /// Returns the `Analysis` used to generate the underlying `Results`. diff --git a/compiler/rustc_mir_dataflow/src/framework/fmt.rs b/compiler/rustc_mir_dataflow/src/framework/fmt.rs index 38599cd094933..8a5d3c35f2f52 100644 --- a/compiler/rustc_mir_dataflow/src/framework/fmt.rs +++ b/compiler/rustc_mir_dataflow/src/framework/fmt.rs @@ -82,21 +82,12 @@ where } fn fmt_diff_with(&self, old: &Self, ctxt: &C, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let size = self.domain_size(); - assert_eq!(size, old.domain_size()); - - let mut set_in_self = MixedBitSet::new_empty(size); - let mut cleared_in_self = MixedBitSet::new_empty(size); - - for i in (0..size).map(T::new) { - match (self.contains(i), old.contains(i)) { - (true, false) => set_in_self.insert(i), - (false, true) => cleared_in_self.insert(i), - _ => continue, - }; - } + let mut set_in_self = self.clone(); + set_in_self.subtract(old); + let mut cleared_in_self = old.clone(); + cleared_in_self.subtract(self); - fmt_diff(&set_in_self, &cleared_in_self, ctxt, f) + fmt_diff(&MixedBitSet::Small(set_in_self), &MixedBitSet::Small(cleared_in_self), ctxt, f) } } diff --git a/compiler/rustc_mir_dataflow/src/impls/initialized.rs b/compiler/rustc_mir_dataflow/src/impls/initialized.rs index 18165b0b9bd08..4f77deb8526d2 100644 --- a/compiler/rustc_mir_dataflow/src/impls/initialized.rs +++ b/compiler/rustc_mir_dataflow/src/impls/initialized.rs @@ -464,7 +464,7 @@ impl<'tcx> Analysis<'tcx> for MaybeUninitializedPlaces<'_, 'tcx> { // sets state bits for Arg places fn initialize_start_block(&self, _: &mir::Body<'tcx>, state: &mut Self::Domain) { // set all bits to 1 (uninit) before gathering counter-evidence - state.insert_all(); + state.insert_all(self.move_data().move_paths.len()); drop_flag_effects_for_function_entry(self.body, self.move_data, |path, s| { assert!(s == DropFlagState::Present); diff --git a/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs b/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs index e3aa8f5a62014..896b86156c8a2 100644 --- a/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs +++ b/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs @@ -87,7 +87,6 @@ impl<'a, 'tcx> Analysis<'tcx> for MaybeStorageDead<'a> { } fn initialize_start_block(&self, body: &Body<'tcx>, state: &mut Self::Domain) { - assert_eq!(body.local_decls.len(), self.always_live_locals.domain_size()); // Do not iterate on return place and args, as they are trivially always live. for local in body.vars_and_temps_iter() { if !self.always_live_locals.contains(local) { diff --git a/compiler/rustc_mir_transform/src/copy_prop.rs b/compiler/rustc_mir_transform/src/copy_prop.rs index 27af5818982d0..42cac41d8cb6d 100644 --- a/compiler/rustc_mir_transform/src/copy_prop.rs +++ b/compiler/rustc_mir_transform/src/copy_prop.rs @@ -34,7 +34,7 @@ impl<'tcx> crate::MirPass<'tcx> for CopyProp { let fully_moved = fully_moved_locals(&ssa, body); debug!(?fully_moved); - let mut storage_to_remove = DenseBitSet::new_empty(fully_moved.domain_size()); + let mut storage_to_remove = DenseBitSet::new_empty(body.local_decls.len()); for (local, &head) in ssa.copy_classes().iter_enumerated() { if local != head { storage_to_remove.insert(head); diff --git a/compiler/rustc_mir_transform/src/coroutine.rs b/compiler/rustc_mir_transform/src/coroutine.rs index cddb2f8477858..1b58b7dedf739 100644 --- a/compiler/rustc_mir_transform/src/coroutine.rs +++ b/compiler/rustc_mir_transform/src/coroutine.rs @@ -211,6 +211,9 @@ struct TransformVisitor<'tcx> { old_yield_ty: Ty<'tcx>, old_ret_ty: Ty<'tcx>, + + /// The number of locals in the [`Body`]. + n_locals: usize, } impl<'tcx> TransformVisitor<'tcx> { @@ -440,7 +443,7 @@ impl<'tcx> MutVisitor<'tcx> for TransformVisitor<'tcx> { let storage_liveness: GrowableBitSet = self.storage_liveness[block].clone().unwrap().into(); - for i in 0..self.always_live_locals.domain_size() { + for i in 0..self.n_locals { let l = Local::new(i); let needs_storage_dead = storage_liveness.contains(l) && !self.remap.contains(l) @@ -845,8 +848,6 @@ fn compute_storage_conflicts<'mir, 'tcx>( analysis: &mut MaybeRequiresStorage<'mir, 'tcx>, results: &Results>, ) -> BitMatrix { - assert_eq!(body.local_decls.len(), saved_locals.domain_size()); - debug!("compute_storage_conflicts({:?})", body.span); debug!("always_live = {:?}", always_live_locals); @@ -859,7 +860,11 @@ fn compute_storage_conflicts<'mir, 'tcx>( let mut visitor = StorageConflictVisitor { body, saved_locals, - local_conflicts: BitMatrix::from_row_n(&ineligible_locals, body.local_decls.len()), + local_conflicts: BitMatrix::from_row_n( + &ineligible_locals, + body.local_decls.len(), + body.local_decls.len(), + ), eligible_storage_live: DenseBitSet::new_empty(body.local_decls.len()), }; @@ -1010,7 +1015,7 @@ fn compute_layout<'tcx>( // Create a map from local indices to coroutine struct indices. let mut variant_fields: IndexVec> = iter::repeat(IndexVec::new()).take(CoroutineArgs::RESERVED_VARIANTS).collect(); - let mut remap = IndexVec::from_elem_n(None, saved_locals.domain_size()); + let mut remap = IndexVec::from_elem_n(None, body.local_decls.len()); for (suspension_point_idx, live_locals) in live_locals_at_suspension_points.iter().enumerate() { let variant_index = VariantIdx::from(CoroutineArgs::RESERVED_VARIANTS + suspension_point_idx); @@ -1585,6 +1590,7 @@ impl<'tcx> crate::MirPass<'tcx> for StateTransform { discr_ty, old_ret_ty, old_yield_ty, + n_locals: body.local_decls.len(), }; transform.visit_body(body); diff --git a/compiler/rustc_mir_transform/src/coverage/counters.rs b/compiler/rustc_mir_transform/src/coverage/counters.rs index 5568d42ab8f3c..c226f9b89ceb8 100644 --- a/compiler/rustc_mir_transform/src/coverage/counters.rs +++ b/compiler/rustc_mir_transform/src/coverage/counters.rs @@ -81,8 +81,9 @@ pub(crate) fn transcribe_counters( old: &NodeCounters, bcb_needs_counter: &DenseBitSet, bcbs_seen: &DenseBitSet, + num_bcbs: usize, ) -> CoverageCounters { - let mut new = CoverageCounters::with_num_bcbs(bcb_needs_counter.domain_size()); + let mut new = CoverageCounters::with_num_bcbs(num_bcbs); for bcb in bcb_needs_counter.iter() { if !bcbs_seen.contains(bcb) { diff --git a/compiler/rustc_mir_transform/src/coverage/counters/balanced_flow.rs b/compiler/rustc_mir_transform/src/coverage/counters/balanced_flow.rs index 4c20722a04347..e31cec383cfe4 100644 --- a/compiler/rustc_mir_transform/src/coverage/counters/balanced_flow.rs +++ b/compiler/rustc_mir_transform/src/coverage/counters/balanced_flow.rs @@ -72,7 +72,7 @@ impl BalancedFlowGraph { // Next, find all nodes that are currently not reverse-reachable from // `sink_edge_nodes`, and add them to the set as well. dfs.complete_search(); - sink_edge_nodes.union_not(dfs.visited_set()); + sink_edge_nodes.union_not(dfs.visited_set(), graph.num_nodes()); // The sink node is 1 higher than the highest real node. let sink = G::Node::new(graph.num_nodes()); diff --git a/compiler/rustc_mir_transform/src/coverage/query.rs b/compiler/rustc_mir_transform/src/coverage/query.rs index ccf76dc710874..003082d396d1b 100644 --- a/compiler/rustc_mir_transform/src/coverage/query.rs +++ b/compiler/rustc_mir_transform/src/coverage/query.rs @@ -136,7 +136,12 @@ fn coverage_ids_info<'tcx>( priority_list[1..].sort_by_key(|&bcb| !bcbs_seen.contains(bcb)); let node_counters = make_node_counters(&fn_cov_info.node_flow_data, &priority_list); - let coverage_counters = transcribe_counters(&node_counters, &bcb_needs_counter, &bcbs_seen); + let coverage_counters = transcribe_counters( + &node_counters, + &bcb_needs_counter, + &bcbs_seen, + fn_cov_info.priority_list.len(), + ); let CoverageCounters { phys_counter_for_node, next_counter_id, node_counters, expressions, .. diff --git a/compiler/rustc_mir_transform/src/deduce_param_attrs.rs b/compiler/rustc_mir_transform/src/deduce_param_attrs.rs index a0db8bdb7ed88..b2f077d5206fc 100644 --- a/compiler/rustc_mir_transform/src/deduce_param_attrs.rs +++ b/compiler/rustc_mir_transform/src/deduce_param_attrs.rs @@ -19,19 +19,20 @@ struct DeduceReadOnly { /// 1). The bit is true if the argument may have been mutated or false if we know it hasn't /// been up to the point we're at. mutable_args: DenseBitSet, + arg_count: usize, } impl DeduceReadOnly { /// Returns a new DeduceReadOnly instance. fn new(arg_count: usize) -> Self { - Self { mutable_args: DenseBitSet::new_empty(arg_count) } + Self { mutable_args: DenseBitSet::new_empty(arg_count), arg_count } } } impl<'tcx> Visitor<'tcx> for DeduceReadOnly { fn visit_place(&mut self, place: &Place<'tcx>, context: PlaceContext, _location: Location) { // We're only interested in arguments. - if place.local == RETURN_PLACE || place.local.index() > self.mutable_args.domain_size() { + if place.local == RETURN_PLACE || place.local.index() > self.arg_count { return; } @@ -86,7 +87,7 @@ impl<'tcx> Visitor<'tcx> for DeduceReadOnly { let local = place.local; if place.is_indirect() || local == RETURN_PLACE - || local.index() > self.mutable_args.domain_size() + || local.index() > self.arg_count { continue; } diff --git a/compiler/rustc_mir_transform/src/lint_tail_expr_drop_order.rs b/compiler/rustc_mir_transform/src/lint_tail_expr_drop_order.rs index 75f351f05c30e..0d4bfd2f78b8a 100644 --- a/compiler/rustc_mir_transform/src/lint_tail_expr_drop_order.rs +++ b/compiler/rustc_mir_transform/src/lint_tail_expr_drop_order.rs @@ -274,7 +274,7 @@ pub(crate) fn run_lint<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId, body: &Body< // We shall now exclude some local bindings for the following cases. { - let mut to_exclude = MixedBitSet::new_empty(all_locals_dropped.domain_size()); + let mut to_exclude = MixedBitSet::new_empty(move_data.move_paths.len()); // We will now do subtraction from the candidate dropped locals, because of the // following reasons. for path_idx in all_locals_dropped.iter() { diff --git a/compiler/rustc_mir_transform/src/single_use_consts.rs b/compiler/rustc_mir_transform/src/single_use_consts.rs index 02caa92ad3fc8..d88f8607f8484 100644 --- a/compiler/rustc_mir_transform/src/single_use_consts.rs +++ b/compiler/rustc_mir_transform/src/single_use_consts.rs @@ -33,7 +33,9 @@ impl<'tcx> crate::MirPass<'tcx> for SingleUseConsts { locals_in_debug_info: DenseBitSet::new_empty(body.local_decls.len()), }; - finder.ineligible_locals.insert_range(..=Local::from_usize(body.arg_count)); + finder + .ineligible_locals + .insert_range_inclusive(Local::from_usize(0)..=Local::from_usize(body.arg_count)); finder.visit_body(body); diff --git a/compiler/rustc_mir_transform/src/sroa.rs b/compiler/rustc_mir_transform/src/sroa.rs index 7c6ccc89c4f30..c390c993c5aed 100644 --- a/compiler/rustc_mir_transform/src/sroa.rs +++ b/compiler/rustc_mir_transform/src/sroa.rs @@ -103,7 +103,7 @@ fn escaping_locals<'tcx>( }; let mut set = DenseBitSet::new_empty(body.local_decls.len()); - set.insert_range(RETURN_PLACE..=Local::from_usize(body.arg_count)); + set.insert_range_inclusive(RETURN_PLACE..=Local::from_usize(body.arg_count)); for (local, decl) in body.local_decls().iter_enumerated() { if excluded.contains(local) || is_excluded_ty(decl.ty) { set.insert(local); From 280bee50439eb644ef1afc624a95c0d8fb32b0d4 Mon Sep 17 00:00:00 2001 From: Tage Johansson Date: Wed, 21 May 2025 14:15:35 +0200 Subject: [PATCH 08/12] DenseBitSet: Solve a few bugs related to overflow in SHR. --- compiler/rustc_index/src/bit_set/dense_bit_set.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_index/src/bit_set/dense_bit_set.rs b/compiler/rustc_index/src/bit_set/dense_bit_set.rs index 334dfadbf3952..9e128fb204235 100644 --- a/compiler/rustc_index/src/bit_set/dense_bit_set.rs +++ b/compiler/rustc_index/src/bit_set/dense_bit_set.rs @@ -81,7 +81,7 @@ impl DenseBitSet { /// The tag for the `empty_unallocated` variant. The two most significant bits are /// `[0, 1]`. - const EMPTY_UNALLOCATED_TAG_BITS: usize = 0b01 << (WORD_BITS - 2); + const EMPTY_UNALLOCATED_TAG_BITS: usize = 0b01 << (usize::BITS - 2); /// Create a new empty bit set with a given domain_size. /// @@ -115,7 +115,9 @@ impl DenseBitSet { *word = Word::MAX; } // Remove excessive bits on the last word. - *words.last_mut().unwrap() >>= WORD_BITS - domain_size % WORD_BITS; + // Trust me: this mask is correct. + let last_word_mask = Word::MAX.wrapping_shr(domain_size.wrapping_neg() as u32); + *words.last_mut().unwrap() &= last_word_mask; Self { on_heap: ManuallyDrop::new(on_heap) } } } @@ -135,8 +137,8 @@ impl DenseBitSet { // safe to assume `self.inline`, or `self.on_heap`. #[inline(always)] pub const fn is_empty_unallocated(&self) -> bool { - (unsafe { self.empty_unallocated }) >> usize::BITS as u32 - 2 - == Self::EMPTY_UNALLOCATED_TAG_BITS >> usize::BITS as u32 - 2 + const MASK: usize = usize::MAX << usize::BITS - 2; + (unsafe { self.empty_unallocated } & MASK) == Self::EMPTY_UNALLOCATED_TAG_BITS } /// Check if `self` is `empty_unallocated` and if so return the number of words required to From 5081f73ff24c940637032cdd1547c45101911514 Mon Sep 17 00:00:00 2001 From: Tage Johansson Date: Wed, 21 May 2025 15:06:59 +0200 Subject: [PATCH 09/12] fix documentation errors --- compiler/rustc_index/src/bit_set/dense_bit_set.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_index/src/bit_set/dense_bit_set.rs b/compiler/rustc_index/src/bit_set/dense_bit_set.rs index 9e128fb204235..0a9890e65bac5 100644 --- a/compiler/rustc_index/src/bit_set/dense_bit_set.rs +++ b/compiler/rustc_index/src/bit_set/dense_bit_set.rs @@ -26,11 +26,13 @@ use crate::Idx; /// variant for the empty set. In this case, the domain size is stored inline along with a few /// bits indicating that the set is empty. Allocation is deferred until needed, such as on /// the first insert or remove operation. This avoids the need to wrap a lazily initialised bit set -/// in a [`OnceCell`] or an [`Option`]—you can simply create an empty set and populate it if needed. +/// in a [`OnceCell`](std::cell::OnceCell) or an [`Option`]—you can simply create an empty set and +/// populate it if needed. /// /// Note 1: Since this bitset is dense, if your domain is large and/or relatively homogeneous (e.g. -/// long runs of set or unset bits), it may be more efficient to use a [MixedBitSet] or an -/// [IntervalSet](crate::interval::IntervalSet), which are better suited for sparse or highly +/// long runs of set or unset bits), it may be more efficient to use a +/// [`MixedBitSet`](crate::bit_set::MixedBitSet) or an +/// [`IntervalSet`](crate::interval::IntervalSet), which are better suited for sparse or highly /// compressible domains. /// /// Note 2: Use [`GrowableBitSet`] if you require support for resizing after creation. From f5f88ea53e3739f9bb2a7a57c9f540f06f9d3485 Mon Sep 17 00:00:00 2001 From: Tage Johansson Date: Wed, 21 May 2025 17:00:43 +0200 Subject: [PATCH 10/12] remove a test checking that the hash for bit sets of different domain sizes are different The new implementation of DenseBitSet doesn't store the exact domain size, so of course the hash values for identical sets with different domain sizes may be equal. --- .../rustc_data_structures/src/stable_hasher/tests.rs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/compiler/rustc_data_structures/src/stable_hasher/tests.rs b/compiler/rustc_data_structures/src/stable_hasher/tests.rs index 635f241847c43..9f854ca9735eb 100644 --- a/compiler/rustc_data_structures/src/stable_hasher/tests.rs +++ b/compiler/rustc_data_structures/src/stable_hasher/tests.rs @@ -14,16 +14,6 @@ fn hash>(t: &T) -> Hash128 { h.finish() } -// Check that bit set hash includes the domain size. -#[test] -fn test_hash_bit_set() { - use rustc_index::bit_set::DenseBitSet; - let a: DenseBitSet = DenseBitSet::new_empty(1); - let b: DenseBitSet = DenseBitSet::new_empty(2); - assert_ne!(a, b); - assert_ne!(hash(&a), hash(&b)); -} - // Check that bit matrix hash includes the matrix dimensions. #[test] fn test_hash_bit_matrix() { From 042debb484beff5a76c3dab8f9898fc93a216fb9 Mon Sep 17 00:00:00 2001 From: Tage Johansson Date: Thu, 22 May 2025 10:57:46 +0200 Subject: [PATCH 11/12] in rustc_index: fix compilation error by changing the rustc_serialize dependency to be required instead of optional --- compiler/rustc_index/Cargo.toml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/compiler/rustc_index/Cargo.toml b/compiler/rustc_index/Cargo.toml index 9aa24e668b6b7..7ea9cfdb63549 100644 --- a/compiler/rustc_index/Cargo.toml +++ b/compiler/rustc_index/Cargo.toml @@ -8,7 +8,7 @@ edition = "2024" itertools = "0.12" rustc_index_macros = { path = "../rustc_index_macros" } rustc_macros = { path = "../rustc_macros", optional = true } -rustc_serialize = { path = "../rustc_serialize", optional = true } +rustc_serialize = { path = "../rustc_serialize" } smallvec = "1.8.1" # tidy-alphabetical-end @@ -16,7 +16,6 @@ smallvec = "1.8.1" # tidy-alphabetical-start default = ["nightly"] nightly = [ - "dep:rustc_serialize", "dep:rustc_macros", "rustc_index_macros/nightly", ] From f2585eb7fe0169cdd2a55998dcd9b91787f24593 Mon Sep 17 00:00:00 2001 From: Tage Johansson Date: Mon, 26 May 2025 12:40:53 +0200 Subject: [PATCH 12/12] in rustc_index: revert "fix compilation error by changing the rustc_serialize dependency to be required instead of optional", and introduce conditional compilation instead. --- compiler/rustc_index/Cargo.toml | 3 ++- compiler/rustc_index/src/bit_set/dense_bit_set.rs | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_index/Cargo.toml b/compiler/rustc_index/Cargo.toml index 7ea9cfdb63549..9aa24e668b6b7 100644 --- a/compiler/rustc_index/Cargo.toml +++ b/compiler/rustc_index/Cargo.toml @@ -8,7 +8,7 @@ edition = "2024" itertools = "0.12" rustc_index_macros = { path = "../rustc_index_macros" } rustc_macros = { path = "../rustc_macros", optional = true } -rustc_serialize = { path = "../rustc_serialize" } +rustc_serialize = { path = "../rustc_serialize", optional = true } smallvec = "1.8.1" # tidy-alphabetical-end @@ -16,6 +16,7 @@ smallvec = "1.8.1" # tidy-alphabetical-start default = ["nightly"] nightly = [ + "dep:rustc_serialize", "dep:rustc_macros", "rustc_index_macros/nightly", ] diff --git a/compiler/rustc_index/src/bit_set/dense_bit_set.rs b/compiler/rustc_index/src/bit_set/dense_bit_set.rs index 0a9890e65bac5..7ff6ef285b3ea 100644 --- a/compiler/rustc_index/src/bit_set/dense_bit_set.rs +++ b/compiler/rustc_index/src/bit_set/dense_bit_set.rs @@ -8,6 +8,7 @@ use std::ptr::NonNull; use std::{fmt, iter, slice}; use itertools::Either; +#[cfg(feature = "nightly")] use rustc_serialize::{Decodable, Decoder, Encodable, Encoder}; use super::{ @@ -781,6 +782,7 @@ impl BitRelations> for DenseBitSet { } } +#[cfg(feature = "nightly")] impl Encodable for DenseBitSet { #[inline(never)] // FIXME: For profiling purposes fn encode(&self, s: &mut S) { @@ -815,6 +817,7 @@ impl Encodable for DenseBitSet { } } +#[cfg(feature = "nightly")] impl Decodable for DenseBitSet { #[inline(never)] // FIXME: For profiling purposes fn decode(d: &mut D) -> Self {