Skip to content

Commit

Permalink
Rollup merge of #135250 - lqd:simple-cleanups, r=matthewjasper
Browse files Browse the repository at this point in the history
A couple simple borrowck cleanups

This PR has a couple simple renamings:
- it's been a long time since the mapping from `Location`s to `PointIndex`es was extracted from `RegionElements` into the `DenseLocationMap`, but only the types were renamed at the time. borrowck still refers to this map as `elements`. That's confusing, especially since sometimes we also use the mapping via `LivenessValues`, and makes more sense as `location_map` instead.
- to clarify `LocationTable` is not as general as it sounds, and is only for datalog polonius. In this branch I didn't rename the handful of `location_table` fields and params to `polonius_table`, but can do that to differentiate it even more from `location_map`. I did try it locally and it looks worthwhile, so if you'd prefer I can also push it here. (Or we could even switch these datalog types and fields to even more explicit names)
- to clarify the incomprehensible `AllFacts`, it is renamed to `PoloniusFacts`. These can be referred to as `facts` within the legacy polonius module, but as `polonius_facts` outside of it to make it clear that they're not about NLLs (nor are they about in-tree polonius but that'll be magically fixed when they're removed in the future)

r? `@matthewjasper`
  • Loading branch information
matthiaskrgr authored Jan 8, 2025
2 parents 748effd + 36ea00c commit 9ea76e4
Show file tree
Hide file tree
Showing 15 changed files with 153 additions and 146 deletions.
8 changes: 4 additions & 4 deletions compiler/rustc_borrowck/src/consumers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ pub use super::dataflow::{BorrowIndex, Borrows, calculate_borrows_out_of_scope_a
pub use super::place_ext::PlaceExt;
pub use super::places_conflict::{PlaceConflictBias, places_conflict};
pub use super::polonius::legacy::{
AllFacts as PoloniusInput, LocationTable, PoloniusOutput, PoloniusRegionVid, RichLocation,
RustcFacts,
PoloniusFacts as PoloniusInput, PoloniusLocationTable, PoloniusOutput, PoloniusRegionVid,
RichLocation, RustcFacts,
};
pub use super::region_infer::RegionInferenceContext;

Expand All @@ -33,7 +33,7 @@ pub enum ConsumerOptions {
/// without significant slowdowns.
///
/// Implies [`RegionInferenceContext`](ConsumerOptions::RegionInferenceContext),
/// and additionally retrieve the [`LocationTable`] and [`PoloniusInput`] that
/// and additionally retrieve the [`PoloniusLocationTable`] and [`PoloniusInput`] that
/// would be given to Polonius. Critically, this does not run Polonius, which
/// one may want to avoid due to performance issues on large bodies.
PoloniusInputFacts,
Expand Down Expand Up @@ -71,7 +71,7 @@ pub struct BodyWithBorrowckFacts<'tcx> {
/// The table that maps Polonius points to locations in the table.
/// Populated when using [`ConsumerOptions::PoloniusInputFacts`]
/// or [`ConsumerOptions::PoloniusOutputFacts`].
pub location_table: Option<LocationTable>,
pub location_table: Option<PoloniusLocationTable>,
/// Polonius input facts.
/// Populated when using [`ConsumerOptions::PoloniusInputFacts`]
/// or [`ConsumerOptions::PoloniusOutputFacts`].
Expand Down
9 changes: 5 additions & 4 deletions compiler/rustc_borrowck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ use crate::diagnostics::{
use crate::path_utils::*;
use crate::place_ext::PlaceExt;
use crate::places_conflict::{PlaceConflictBias, places_conflict};
use crate::polonius::legacy::{LocationTable, PoloniusOutput};
use crate::polonius::legacy::{PoloniusLocationTable, PoloniusOutput};
use crate::prefixes::PrefixSet;
use crate::region_infer::RegionInferenceContext;
use crate::renumber::RegionCtxt;
Expand Down Expand Up @@ -176,7 +176,7 @@ fn do_mir_borrowck<'tcx>(
infcx.register_predefined_opaques_for_next_solver(def);
}

let location_table = LocationTable::new(body);
let location_table = PoloniusLocationTable::new(body);

let move_data = MoveData::gather_moves(body, tcx, |_| true);
let promoted_move_data = promoted
Expand Down Expand Up @@ -247,7 +247,8 @@ fn do_mir_borrowck<'tcx>(
infcx: &infcx,
body: promoted_body,
move_data: &move_data,
location_table: &location_table, // no need to create a real one for the promoted, it is not used
// no need to create a real location table for the promoted, it is not used
location_table: &location_table,
movable_coroutine,
fn_self_span_reported: Default::default(),
locals_are_invalidated_at_exit,
Expand Down Expand Up @@ -513,7 +514,7 @@ struct MirBorrowckCtxt<'a, 'infcx, 'tcx> {

/// Map from MIR `Location` to `LocationIndex`; created
/// when MIR borrowck begins.
location_table: &'a LocationTable,
location_table: &'a PoloniusLocationTable,

movable_coroutine: bool,
/// This keeps track of whether local variables are free-ed when the function
Expand Down
30 changes: 16 additions & 14 deletions compiler/rustc_borrowck/src/nll.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ use crate::borrow_set::BorrowSet;
use crate::consumers::ConsumerOptions;
use crate::diagnostics::{BorrowckDiagnosticsBuffer, RegionErrors};
use crate::polonius::LocalizedOutlivesConstraintSet;
use crate::polonius::legacy::{AllFacts, AllFactsExt, LocationTable, PoloniusOutput};
use crate::polonius::legacy::{
PoloniusFacts, PoloniusFactsExt, PoloniusLocationTable, PoloniusOutput,
};
use crate::region_infer::RegionInferenceContext;
use crate::type_check::{self, MirTypeckResults};
use crate::universal_regions::UniversalRegions;
Expand All @@ -39,7 +41,7 @@ use crate::{BorrowckInferCtxt, polonius, renumber};
pub(crate) struct NllOutput<'tcx> {
pub regioncx: RegionInferenceContext<'tcx>,
pub opaque_type_values: FxIndexMap<LocalDefId, OpaqueHiddenType<'tcx>>,
pub polonius_input: Option<Box<AllFacts>>,
pub polonius_input: Option<Box<PoloniusFacts>>,
pub polonius_output: Option<Box<PoloniusOutput>>,
pub opt_closure_req: Option<ClosureRegionRequirements<'tcx>>,
pub nll_errors: RegionErrors<'tcx>,
Expand Down Expand Up @@ -80,7 +82,7 @@ pub(crate) fn compute_regions<'a, 'tcx>(
universal_regions: UniversalRegions<'tcx>,
body: &Body<'tcx>,
promoted: &IndexSlice<Promoted, Body<'tcx>>,
location_table: &LocationTable,
location_table: &PoloniusLocationTable,
flow_inits: ResultsCursor<'a, 'tcx, MaybeInitializedPlaces<'a, 'tcx>>,
move_data: &MoveData<'tcx>,
borrow_set: &BorrowSet<'tcx>,
Expand All @@ -91,10 +93,10 @@ pub(crate) fn compute_regions<'a, 'tcx>(
|| is_polonius_legacy_enabled;
let polonius_output = consumer_options.map(|c| c.polonius_output()).unwrap_or_default()
|| is_polonius_legacy_enabled;
let mut all_facts =
(polonius_input || AllFacts::enabled(infcx.tcx)).then_some(AllFacts::default());
let mut polonius_facts =
(polonius_input || PoloniusFacts::enabled(infcx.tcx)).then_some(PoloniusFacts::default());

let elements = Rc::new(DenseLocationMap::new(body));
let location_map = Rc::new(DenseLocationMap::new(body));

// Run the MIR type-checker.
let MirTypeckResults {
Expand All @@ -109,10 +111,10 @@ pub(crate) fn compute_regions<'a, 'tcx>(
universal_regions,
location_table,
borrow_set,
&mut all_facts,
&mut polonius_facts,
flow_inits,
move_data,
Rc::clone(&elements),
Rc::clone(&location_map),
);

// Create the region inference context, taking ownership of the
Expand All @@ -122,7 +124,7 @@ pub(crate) fn compute_regions<'a, 'tcx>(

// If requested, emit legacy polonius facts.
polonius::legacy::emit_facts(
&mut all_facts,
&mut polonius_facts,
infcx.tcx,
location_table,
body,
Expand All @@ -137,7 +139,7 @@ pub(crate) fn compute_regions<'a, 'tcx>(
var_infos,
constraints,
universal_region_relations,
elements,
location_map,
);

// If requested for `-Zpolonius=next`, convert NLL constraints to localized outlives
Expand All @@ -147,13 +149,13 @@ pub(crate) fn compute_regions<'a, 'tcx>(
});

// If requested: dump NLL facts, and run legacy polonius analysis.
let polonius_output = all_facts.as_ref().and_then(|all_facts| {
let polonius_output = polonius_facts.as_ref().and_then(|polonius_facts| {
if infcx.tcx.sess.opts.unstable_opts.nll_facts {
let def_id = body.source.def_id();
let def_path = infcx.tcx.def_path(def_id);
let dir_path = PathBuf::from(&infcx.tcx.sess.opts.unstable_opts.nll_facts_dir)
.join(def_path.to_filename_friendly_no_crate());
all_facts.write_to_dir(dir_path, location_table).unwrap();
polonius_facts.write_to_dir(dir_path, location_table).unwrap();
}

if polonius_output {
Expand All @@ -162,7 +164,7 @@ pub(crate) fn compute_regions<'a, 'tcx>(
let algorithm = Algorithm::from_str(&algorithm).unwrap();
debug!("compute_regions: using polonius algorithm {:?}", algorithm);
let _prof_timer = infcx.tcx.prof.generic_activity("polonius_analysis");
Some(Box::new(Output::compute(all_facts, algorithm, false)))
Some(Box::new(Output::compute(polonius_facts, algorithm, false)))
} else {
None
}
Expand All @@ -182,7 +184,7 @@ pub(crate) fn compute_regions<'a, 'tcx>(
NllOutput {
regioncx,
opaque_type_values: remapped_opaque_tys,
polonius_input: all_facts.map(Box::new),
polonius_input: polonius_facts.map(Box::new),
polonius_output,
opt_closure_req: closure_region_requirements,
nll_errors,
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_borrowck/src/polonius/legacy/accesses.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@ use rustc_middle::ty::TyCtxt;
use rustc_mir_dataflow::move_paths::{LookupResult, MoveData};
use tracing::debug;

use super::{AllFacts, LocationIndex, LocationTable};
use super::{LocationIndex, PoloniusFacts, PoloniusLocationTable};
use crate::def_use::{self, DefUse};
use crate::universal_regions::UniversalRegions;

/// Emit polonius facts for variable defs, uses, drops, and path accesses.
pub(crate) fn emit_access_facts<'tcx>(
tcx: TyCtxt<'tcx>,
facts: &mut AllFacts,
facts: &mut PoloniusFacts,
body: &Body<'tcx>,
location_table: &LocationTable,
location_table: &PoloniusLocationTable,
move_data: &MoveData<'tcx>,
universal_regions: &UniversalRegions<'tcx>,
) {
Expand All @@ -31,9 +31,9 @@ pub(crate) fn emit_access_facts<'tcx>(

/// MIR visitor extracting point-wise facts about accesses.
struct AccessFactsExtractor<'a, 'tcx> {
facts: &'a mut AllFacts,
facts: &'a mut PoloniusFacts,
move_data: &'a MoveData<'tcx>,
location_table: &'a LocationTable,
location_table: &'a PoloniusLocationTable,
}

impl<'tcx> AccessFactsExtractor<'_, 'tcx> {
Expand Down
42 changes: 21 additions & 21 deletions compiler/rustc_borrowck/src/polonius/legacy/facts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@ use std::fs::{self, File};
use std::io::Write;
use std::path::Path;

use polonius_engine::{AllFacts as PoloniusFacts, Atom, Output};
use polonius_engine::{AllFacts, Atom, Output};
use rustc_macros::extension;
use rustc_middle::mir::Local;
use rustc_middle::ty::{RegionVid, TyCtxt};
use rustc_mir_dataflow::move_paths::MovePathIndex;

use super::{LocationIndex, LocationTable};
use super::{LocationIndex, PoloniusLocationTable};
use crate::BorrowIndex;

#[derive(Copy, Clone, Debug)]
Expand Down Expand Up @@ -49,11 +49,11 @@ impl polonius_engine::FactTypes for RustcFacts {
type Path = MovePathIndex;
}

pub type AllFacts = PoloniusFacts<RustcFacts>;
pub type PoloniusFacts = AllFacts<RustcFacts>;

#[extension(pub(crate) trait AllFactsExt)]
impl AllFacts {
/// Returns `true` if there is a need to gather `AllFacts` given the
#[extension(pub(crate) trait PoloniusFactsExt)]
impl PoloniusFacts {
/// Returns `true` if there is a need to gather `PoloniusFacts` given the
/// current `-Z` flags.
fn enabled(tcx: TyCtxt<'_>) -> bool {
tcx.sess.opts.unstable_opts.nll_facts
Expand All @@ -63,7 +63,7 @@ impl AllFacts {
fn write_to_dir(
&self,
dir: impl AsRef<Path>,
location_table: &LocationTable,
location_table: &PoloniusLocationTable,
) -> Result<(), Box<dyn Error>> {
let dir: &Path = dir.as_ref();
fs::create_dir_all(dir)?;
Expand Down Expand Up @@ -119,7 +119,7 @@ impl Atom for LocationIndex {
}

struct FactWriter<'w> {
location_table: &'w LocationTable,
location_table: &'w PoloniusLocationTable,
dir: &'w Path,
}

Expand All @@ -141,15 +141,15 @@ trait FactRow {
fn write(
&self,
out: &mut dyn Write,
location_table: &LocationTable,
location_table: &PoloniusLocationTable,
) -> Result<(), Box<dyn Error>>;
}

impl FactRow for PoloniusRegionVid {
fn write(
&self,
out: &mut dyn Write,
location_table: &LocationTable,
location_table: &PoloniusLocationTable,
) -> Result<(), Box<dyn Error>> {
write_row(out, location_table, &[self])
}
Expand All @@ -163,7 +163,7 @@ where
fn write(
&self,
out: &mut dyn Write,
location_table: &LocationTable,
location_table: &PoloniusLocationTable,
) -> Result<(), Box<dyn Error>> {
write_row(out, location_table, &[&self.0, &self.1])
}
Expand All @@ -178,7 +178,7 @@ where
fn write(
&self,
out: &mut dyn Write,
location_table: &LocationTable,
location_table: &PoloniusLocationTable,
) -> Result<(), Box<dyn Error>> {
write_row(out, location_table, &[&self.0, &self.1, &self.2])
}
Expand All @@ -194,15 +194,15 @@ where
fn write(
&self,
out: &mut dyn Write,
location_table: &LocationTable,
location_table: &PoloniusLocationTable,
) -> Result<(), Box<dyn Error>> {
write_row(out, location_table, &[&self.0, &self.1, &self.2, &self.3])
}
}

fn write_row(
out: &mut dyn Write,
location_table: &LocationTable,
location_table: &PoloniusLocationTable,
columns: &[&dyn FactCell],
) -> Result<(), Box<dyn Error>> {
for (index, c) in columns.iter().enumerate() {
Expand All @@ -213,41 +213,41 @@ fn write_row(
}

trait FactCell {
fn to_string(&self, location_table: &LocationTable) -> String;
fn to_string(&self, location_table: &PoloniusLocationTable) -> String;
}

impl FactCell for BorrowIndex {
fn to_string(&self, _location_table: &LocationTable) -> String {
fn to_string(&self, _location_table: &PoloniusLocationTable) -> String {
format!("{self:?}")
}
}

impl FactCell for Local {
fn to_string(&self, _location_table: &LocationTable) -> String {
fn to_string(&self, _location_table: &PoloniusLocationTable) -> String {
format!("{self:?}")
}
}

impl FactCell for MovePathIndex {
fn to_string(&self, _location_table: &LocationTable) -> String {
fn to_string(&self, _location_table: &PoloniusLocationTable) -> String {
format!("{self:?}")
}
}

impl FactCell for PoloniusRegionVid {
fn to_string(&self, _location_table: &LocationTable) -> String {
fn to_string(&self, _location_table: &PoloniusLocationTable) -> String {
format!("{self:?}")
}
}

impl FactCell for RegionVid {
fn to_string(&self, _location_table: &LocationTable) -> String {
fn to_string(&self, _location_table: &PoloniusLocationTable) -> String {
format!("{self:?}")
}
}

impl FactCell for LocationIndex {
fn to_string(&self, location_table: &LocationTable) -> String {
fn to_string(&self, location_table: &PoloniusLocationTable) -> String {
format!("{:?}", location_table.to_rich_location(*self))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use rustc_middle::mir::{
use rustc_middle::ty::TyCtxt;
use tracing::debug;

use super::{AllFacts, LocationTable};
use super::{PoloniusFacts, PoloniusLocationTable};
use crate::borrow_set::BorrowSet;
use crate::path_utils::*;
use crate::{
Expand All @@ -22,9 +22,9 @@ use crate::{
/// Emit `loan_invalidated_at` facts.
pub(super) fn emit_loan_invalidations<'tcx>(
tcx: TyCtxt<'tcx>,
facts: &mut AllFacts,
facts: &mut PoloniusFacts,
body: &Body<'tcx>,
location_table: &LocationTable,
location_table: &PoloniusLocationTable,
borrow_set: &BorrowSet<'tcx>,
) {
let dominators = body.basic_blocks.dominators();
Expand All @@ -35,9 +35,9 @@ pub(super) fn emit_loan_invalidations<'tcx>(

struct LoanInvalidationsGenerator<'a, 'tcx> {
tcx: TyCtxt<'tcx>,
facts: &'a mut AllFacts,
facts: &'a mut PoloniusFacts,
body: &'a Body<'tcx>,
location_table: &'a LocationTable,
location_table: &'a PoloniusLocationTable,
dominators: &'a Dominators<BasicBlock>,
borrow_set: &'a BorrowSet<'tcx>,
}
Expand Down
Loading

0 comments on commit 9ea76e4

Please sign in to comment.