From eca2789f579113629877d78fb1be8b05a3a30044 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Thu, 23 Nov 2023 10:42:19 +0000 Subject: [PATCH 01/13] remove useless local variables --- compiler/rustc_borrowck/src/lib.rs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index 822519f2f08..46e5356ad2c 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -215,8 +215,7 @@ fn do_mir_borrowck<'tcx>( nll::replace_regions_in_mir(&infcx, param_env, &mut body_owned, &mut promoted); let body = &body_owned; // no further changes - let location_table_owned = LocationTable::new(body); - let location_table = &location_table_owned; + let location_table = LocationTable::new(body); let move_data = MoveData::gather_moves(body, tcx, param_env, |_| true); let promoted_move_data = promoted @@ -248,7 +247,7 @@ fn do_mir_borrowck<'tcx>( free_regions, body, &promoted, - location_table, + &location_table, param_env, &mut flow_inits, &mdpe.move_data, @@ -312,7 +311,7 @@ fn do_mir_borrowck<'tcx>( param_env, body: promoted_body, move_data: &move_data, - location_table, // no need to create a real one for the promoted, it is not used + location_table: &location_table, // no need to create a real one for the promoted, it is not used movable_coroutine, fn_self_span_reported: Default::default(), locals_are_invalidated_at_exit, @@ -353,7 +352,7 @@ fn do_mir_borrowck<'tcx>( param_env, body, move_data: &mdpe.move_data, - location_table, + location_table: &location_table, movable_coroutine, locals_are_invalidated_at_exit, fn_self_span_reported: Default::default(), @@ -455,7 +454,7 @@ fn do_mir_borrowck<'tcx>( promoted, borrow_set, region_inference_context: regioncx, - location_table: polonius_input.as_ref().map(|_| location_table_owned), + location_table: polonius_input.as_ref().map(|_| location_table), input_facts: polonius_input, output_facts, })) @@ -1040,9 +1039,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { flow_state: &Flows<'cx, 'tcx>, ) -> bool { let mut error_reported = false; - let tcx = self.infcx.tcx; - let body = self.body; - let borrow_set = self.borrow_set.clone(); + let borrow_set = Rc::clone(&self.borrow_set); // Use polonius output if it has been enabled. let mut polonius_output; @@ -1059,8 +1056,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { each_borrow_involving_path( self, - tcx, - body, + self.infcx.tcx, + self.body, location, (sd, place_span.0), &borrow_set, From 3de68e074a04b08abe8cd874528583e5fbbb9df9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Thu, 23 Nov 2023 11:11:18 +0000 Subject: [PATCH 02/13] extract polonius move fact generation --- compiler/rustc_borrowck/src/nll.rs | 92 ++------------------- compiler/rustc_borrowck/src/nll/polonius.rs | 84 +++++++++++++++++++ 2 files changed, 91 insertions(+), 85 deletions(-) create mode 100644 compiler/rustc_borrowck/src/nll/polonius.rs diff --git a/compiler/rustc_borrowck/src/nll.rs b/compiler/rustc_borrowck/src/nll.rs index 08db3a62ece..7a5e918ec43 100644 --- a/compiler/rustc_borrowck/src/nll.rs +++ b/compiler/rustc_borrowck/src/nll.rs @@ -2,16 +2,17 @@ #![deny(rustc::diagnostic_outside_of_impl)] //! The entry point of the NLL borrow checker. +use polonius_engine::{Algorithm, Output}; use rustc_data_structures::fx::FxIndexMap; use rustc_hir::def_id::LocalDefId; use rustc_index::IndexSlice; use rustc_middle::mir::{create_dump_file, dump_enabled, dump_mir, PassWhere}; -use rustc_middle::mir::{ - Body, ClosureOutlivesSubject, ClosureRegionRequirements, LocalKind, Location, Promoted, - START_BLOCK, -}; +use rustc_middle::mir::{Body, ClosureOutlivesSubject, ClosureRegionRequirements, Promoted}; use rustc_middle::ty::print::with_no_trimmed_paths; use rustc_middle::ty::{self, OpaqueHiddenType, TyCtxt}; +use rustc_mir_dataflow::impls::MaybeInitializedPlaces; +use rustc_mir_dataflow::move_paths::MoveData; +use rustc_mir_dataflow::ResultsCursor; use rustc_span::symbol::sym; use std::env; use std::io; @@ -19,11 +20,7 @@ use std::path::PathBuf; use std::rc::Rc; use std::str::FromStr; -use polonius_engine::{Algorithm, Output}; - -use rustc_mir_dataflow::impls::MaybeInitializedPlaces; -use rustc_mir_dataflow::move_paths::{InitKind, InitLocation, MoveData}; -use rustc_mir_dataflow::ResultsCursor; +mod polonius; use crate::{ borrow_set::BorrowSet, @@ -78,81 +75,6 @@ pub(crate) fn replace_regions_in_mir<'tcx>( universal_regions } -// This function populates an AllFacts instance with base facts related to -// MovePaths and needed for the move analysis. -fn populate_polonius_move_facts( - all_facts: &mut AllFacts, - move_data: &MoveData<'_>, - location_table: &LocationTable, - body: &Body<'_>, -) { - all_facts - .path_is_var - .extend(move_data.rev_lookup.iter_locals_enumerated().map(|(l, r)| (r, l))); - - for (child, move_path) in move_data.move_paths.iter_enumerated() { - if let Some(parent) = move_path.parent { - all_facts.child_path.push((child, parent)); - } - } - - let fn_entry_start = - location_table.start_index(Location { block: START_BLOCK, statement_index: 0 }); - - // initialized_at - for init in move_data.inits.iter() { - match init.location { - InitLocation::Statement(location) => { - let block_data = &body[location.block]; - let is_terminator = location.statement_index == block_data.statements.len(); - - if is_terminator && init.kind == InitKind::NonPanicPathOnly { - // We are at the terminator of an init that has a panic path, - // and where the init should not happen on panic - - for successor in block_data.terminator().successors() { - if body[successor].is_cleanup { - continue; - } - - // The initialization happened in (or rather, when arriving at) - // the successors, but not in the unwind block. - let first_statement = Location { block: successor, statement_index: 0 }; - all_facts - .path_assigned_at_base - .push((init.path, location_table.start_index(first_statement))); - } - } else { - // In all other cases, the initialization just happens at the - // midpoint, like any other effect. - all_facts - .path_assigned_at_base - .push((init.path, location_table.mid_index(location))); - } - } - // Arguments are initialized on function entry - InitLocation::Argument(local) => { - assert!(body.local_kind(local) == LocalKind::Arg); - all_facts.path_assigned_at_base.push((init.path, fn_entry_start)); - } - } - } - - for (local, path) in move_data.rev_lookup.iter_locals_enumerated() { - if body.local_kind(local) != LocalKind::Arg { - // Non-arguments start out deinitialised; we simulate this with an - // initial move: - all_facts.path_moved_at_base.push((path, fn_entry_start)); - } - } - - // moved_out_at - // deinitialisation is assumed to always happen! - all_facts - .path_moved_at_base - .extend(move_data.moves.iter().map(|mo| (mo.path, location_table.mid_index(mo.source)))); -} - /// Computes the (non-lexical) regions from the input MIR. /// /// This may result in errors being reported. @@ -206,7 +128,7 @@ pub(crate) fn compute_regions<'cx, 'tcx>( if let Some(all_facts) = &mut all_facts { let _prof_timer = infcx.tcx.prof.generic_activity("polonius_fact_generation"); all_facts.universal_region.extend(universal_regions.universal_regions()); - populate_polonius_move_facts(all_facts, move_data, location_table, body); + polonius::emit_move_facts(all_facts, move_data, location_table, body); // Emit universal regions facts, and their relations, for Polonius. // diff --git a/compiler/rustc_borrowck/src/nll/polonius.rs b/compiler/rustc_borrowck/src/nll/polonius.rs new file mode 100644 index 00000000000..78c744384fd --- /dev/null +++ b/compiler/rustc_borrowck/src/nll/polonius.rs @@ -0,0 +1,84 @@ +//! Functions dedicated to fact generation for the `-Zpolonius=legacy` datalog implementation. +//! +//! Will be removed in the future, once the in-tree `-Zpolonius=next` implementation reaches feature +//! parity. + +use rustc_middle::mir::{Body, LocalKind, Location, START_BLOCK}; +use rustc_mir_dataflow::move_paths::{InitKind, InitLocation, MoveData}; + +use crate::facts::AllFacts; +use crate::location::LocationTable; + +/// Emit polonius facts needed for move/init analysis: moves and assignments. +pub(crate) fn emit_move_facts( + all_facts: &mut AllFacts, + move_data: &MoveData<'_>, + location_table: &LocationTable, + body: &Body<'_>, +) { + all_facts + .path_is_var + .extend(move_data.rev_lookup.iter_locals_enumerated().map(|(l, r)| (r, l))); + + for (child, move_path) in move_data.move_paths.iter_enumerated() { + if let Some(parent) = move_path.parent { + all_facts.child_path.push((child, parent)); + } + } + + let fn_entry_start = + location_table.start_index(Location { block: START_BLOCK, statement_index: 0 }); + + // initialized_at + for init in move_data.inits.iter() { + match init.location { + InitLocation::Statement(location) => { + let block_data = &body[location.block]; + let is_terminator = location.statement_index == block_data.statements.len(); + + if is_terminator && init.kind == InitKind::NonPanicPathOnly { + // We are at the terminator of an init that has a panic path, + // and where the init should not happen on panic + + for successor in block_data.terminator().successors() { + if body[successor].is_cleanup { + continue; + } + + // The initialization happened in (or rather, when arriving at) + // the successors, but not in the unwind block. + let first_statement = Location { block: successor, statement_index: 0 }; + all_facts + .path_assigned_at_base + .push((init.path, location_table.start_index(first_statement))); + } + } else { + // In all other cases, the initialization just happens at the + // midpoint, like any other effect. + all_facts + .path_assigned_at_base + .push((init.path, location_table.mid_index(location))); + } + } + // Arguments are initialized on function entry + InitLocation::Argument(local) => { + assert!(body.local_kind(local) == LocalKind::Arg); + all_facts.path_assigned_at_base.push((init.path, fn_entry_start)); + } + } + } + + for (local, path) in move_data.rev_lookup.iter_locals_enumerated() { + if body.local_kind(local) != LocalKind::Arg { + // Non-arguments start out deinitialised; we simulate this with an + // initial move: + all_facts.path_moved_at_base.push((path, fn_entry_start)); + } + } + + // moved_out_at + // deinitialisation is assumed to always happen! + all_facts + .path_moved_at_base + .extend(move_data.moves.iter().map(|mo| (mo.path, location_table.mid_index(mo.source)))); +} From 459a616c4fffc39df76fda7679873624ae9073d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Thu, 23 Nov 2023 11:28:53 +0000 Subject: [PATCH 03/13] extract polonius universal regions fact generation --- compiler/rustc_borrowck/src/nll.rs | 39 +++-------------- compiler/rustc_borrowck/src/nll/polonius.rs | 46 ++++++++++++++++++++- 2 files changed, 50 insertions(+), 35 deletions(-) diff --git a/compiler/rustc_borrowck/src/nll.rs b/compiler/rustc_borrowck/src/nll.rs index 7a5e918ec43..705e56ea984 100644 --- a/compiler/rustc_borrowck/src/nll.rs +++ b/compiler/rustc_borrowck/src/nll.rs @@ -127,42 +127,13 @@ pub(crate) fn compute_regions<'cx, 'tcx>( if let Some(all_facts) = &mut all_facts { let _prof_timer = infcx.tcx.prof.generic_activity("polonius_fact_generation"); - all_facts.universal_region.extend(universal_regions.universal_regions()); polonius::emit_move_facts(all_facts, move_data, location_table, body); - - // Emit universal regions facts, and their relations, for Polonius. - // - // 1: universal regions are modeled in Polonius as a pair: - // - the universal region vid itself. - // - a "placeholder loan" associated to this universal region. Since they don't exist in - // the `borrow_set`, their `BorrowIndex` are synthesized as the universal region index - // added to the existing number of loans, as if they succeeded them in the set. - // - let borrow_count = borrow_set.len(); - debug!( - "compute_regions: polonius placeholders, num_universals={}, borrow_count={}", - universal_regions.len(), - borrow_count + polonius::emit_universal_region_facts( + all_facts, + borrow_set, + &universal_regions, + &universal_region_relations, ); - - for universal_region in universal_regions.universal_regions() { - let universal_region_idx = universal_region.index(); - let placeholder_loan_idx = borrow_count + universal_region_idx; - all_facts.placeholder.push((universal_region, placeholder_loan_idx.into())); - } - - // 2: the universal region relations `outlives` constraints are emitted as - // `known_placeholder_subset` facts. - for (fr1, fr2) in universal_region_relations.known_outlives() { - if fr1 != fr2 { - debug!( - "compute_regions: emitting polonius `known_placeholder_subset` \ - fr1={:?}, fr2={:?}", - fr1, fr2 - ); - all_facts.known_placeholder_subset.push((fr1, fr2)); - } - } } // Create the region inference context, taking ownership of the diff --git a/compiler/rustc_borrowck/src/nll/polonius.rs b/compiler/rustc_borrowck/src/nll/polonius.rs index 78c744384fd..d493bead772 100644 --- a/compiler/rustc_borrowck/src/nll/polonius.rs +++ b/compiler/rustc_borrowck/src/nll/polonius.rs @@ -6,10 +6,13 @@ use rustc_middle::mir::{Body, LocalKind, Location, START_BLOCK}; use rustc_mir_dataflow::move_paths::{InitKind, InitLocation, MoveData}; +use crate::borrow_set::BorrowSet; use crate::facts::AllFacts; use crate::location::LocationTable; +use crate::type_check::free_region_relations::UniversalRegionRelations; +use crate::universal_regions::UniversalRegions; -/// Emit polonius facts needed for move/init analysis: moves and assignments. +/// Emit facts needed for move/init analysis: moves and assignments. pub(crate) fn emit_move_facts( all_facts: &mut AllFacts, move_data: &MoveData<'_>, @@ -82,3 +85,44 @@ pub(crate) fn emit_move_facts( .path_moved_at_base .extend(move_data.moves.iter().map(|mo| (mo.path, location_table.mid_index(mo.source)))); } + +/// Emit universal regions facts, and their relations. +pub(crate) fn emit_universal_region_facts( + all_facts: &mut AllFacts, + borrow_set: &BorrowSet<'_>, + universal_regions: &UniversalRegions<'_>, + universal_region_relations: &UniversalRegionRelations<'_>, +) { + // 1: universal regions are modeled in Polonius as a pair: + // - the universal region vid itself. + // - a "placeholder loan" associated to this universal region. Since they don't exist in + // the `borrow_set`, their `BorrowIndex` are synthesized as the universal region index + // added to the existing number of loans, as if they succeeded them in the set. + // + all_facts.universal_region.extend(universal_regions.universal_regions()); + let borrow_count = borrow_set.len(); + debug!( + "emit_universal_region_facts: polonius placeholders, num_universals={}, borrow_count={}", + universal_regions.len(), + borrow_count + ); + + for universal_region in universal_regions.universal_regions() { + let universal_region_idx = universal_region.index(); + let placeholder_loan_idx = borrow_count + universal_region_idx; + all_facts.placeholder.push((universal_region, placeholder_loan_idx.into())); + } + + // 2: the universal region relations `outlives` constraints are emitted as + // `known_placeholder_subset` facts. + for (fr1, fr2) in universal_region_relations.known_outlives() { + if fr1 != fr2 { + debug!( + "emit_universal_region_facts: emitting polonius `known_placeholder_subset` \ + fr1={:?}, fr2={:?}", + fr1, fr2 + ); + all_facts.known_placeholder_subset.push((fr1, fr2)); + } + } +} From 16a5da7be23a9cb8297c2baa350ac86c889f49dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Thu, 23 Nov 2023 11:47:41 +0000 Subject: [PATCH 04/13] extract polonius loan invalidations fact generation and move the polonius module to the borrowck root --- compiler/rustc_borrowck/src/lib.rs | 2 +- compiler/rustc_borrowck/src/nll.rs | 12 ++++--- .../src/{ => polonius}/invalidation.rs | 34 ++++++++----------- .../src/{nll/polonius.rs => polonius/mod.rs} | 19 +++++++++++ 4 files changed, 42 insertions(+), 25 deletions(-) rename compiler/rustc_borrowck/src/{ => polonius}/invalidation.rs (96%) rename compiler/rustc_borrowck/src/{nll/polonius.rs => polonius/mod.rs} (91%) diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index 46e5356ad2c..d711a53565d 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -70,13 +70,13 @@ mod dataflow; mod def_use; mod diagnostics; mod facts; -mod invalidation; mod location; mod member_constraints; mod nll; mod path_utils; mod place_ext; mod places_conflict; +mod polonius; mod prefixes; mod region_infer; mod renumber; diff --git a/compiler/rustc_borrowck/src/nll.rs b/compiler/rustc_borrowck/src/nll.rs index 705e56ea984..0e1f48902e3 100644 --- a/compiler/rustc_borrowck/src/nll.rs +++ b/compiler/rustc_borrowck/src/nll.rs @@ -20,16 +20,14 @@ use std::path::PathBuf; use std::rc::Rc; use std::str::FromStr; -mod polonius; - use crate::{ borrow_set::BorrowSet, constraint_generation, consumers::ConsumerOptions, diagnostics::RegionErrors, facts::{AllFacts, AllFactsExt, RustcFacts}, - invalidation, location::LocationTable, + polonius, region_infer::{values::RegionValueElements, RegionInferenceContext}, renumber, type_check::{self, MirTypeckRegionConstraints, MirTypeckResults}, @@ -176,7 +174,13 @@ pub(crate) fn compute_regions<'cx, 'tcx>( ); // Generate various additional constraints. - invalidation::generate_invalidates(infcx.tcx, &mut all_facts, location_table, body, borrow_set); + polonius::emit_loan_invalidations_facts( + infcx.tcx, + &mut all_facts, + location_table, + body, + borrow_set, + ); let def_id = body.source.def_id(); diff --git a/compiler/rustc_borrowck/src/invalidation.rs b/compiler/rustc_borrowck/src/polonius/invalidation.rs similarity index 96% rename from compiler/rustc_borrowck/src/invalidation.rs rename to compiler/rustc_borrowck/src/polonius/invalidation.rs index a3db101311f..43119a97bee 100644 --- a/compiler/rustc_borrowck/src/invalidation.rs +++ b/compiler/rustc_borrowck/src/polonius/invalidation.rs @@ -14,31 +14,25 @@ use crate::{ ReadOrWrite, Reservation, Shallow, Write, WriteKind, }; -pub(super) fn generate_invalidates<'tcx>( +/// Emit `loan_invalidated_at` facts. +pub(super) fn emit_loan_invalidations<'tcx>( tcx: TyCtxt<'tcx>, - all_facts: &mut Option, + all_facts: &mut AllFacts, location_table: &LocationTable, body: &Body<'tcx>, borrow_set: &BorrowSet<'tcx>, ) { - if all_facts.is_none() { - // Nothing to do if we don't have any facts - return; - } - - if let Some(all_facts) = all_facts { - let _prof_timer = tcx.prof.generic_activity("polonius_fact_generation"); - let dominators = body.basic_blocks.dominators(); - let mut ig = InvalidationGenerator { - all_facts, - borrow_set, - tcx, - location_table, - body: body, - dominators, - }; - ig.visit_body(body); - } + let _prof_timer = tcx.prof.generic_activity("polonius_fact_generation"); + let dominators = body.basic_blocks.dominators(); + let mut ig = InvalidationGenerator { + all_facts, + borrow_set, + tcx, + location_table, + body: body, + dominators, + }; + ig.visit_body(body); } struct InvalidationGenerator<'cx, 'tcx> { diff --git a/compiler/rustc_borrowck/src/nll/polonius.rs b/compiler/rustc_borrowck/src/polonius/mod.rs similarity index 91% rename from compiler/rustc_borrowck/src/nll/polonius.rs rename to compiler/rustc_borrowck/src/polonius/mod.rs index d493bead772..887c5fac16b 100644 --- a/compiler/rustc_borrowck/src/nll/polonius.rs +++ b/compiler/rustc_borrowck/src/polonius/mod.rs @@ -4,6 +4,7 @@ //! parity. use rustc_middle::mir::{Body, LocalKind, Location, START_BLOCK}; +use rustc_middle::ty::TyCtxt; use rustc_mir_dataflow::move_paths::{InitKind, InitLocation, MoveData}; use crate::borrow_set::BorrowSet; @@ -12,6 +13,8 @@ use crate::location::LocationTable; use crate::type_check::free_region_relations::UniversalRegionRelations; use crate::universal_regions::UniversalRegions; +mod invalidation; + /// Emit facts needed for move/init analysis: moves and assignments. pub(crate) fn emit_move_facts( all_facts: &mut AllFacts, @@ -126,3 +129,19 @@ pub(crate) fn emit_universal_region_facts( } } } + +/// Emit facts about loan invalidations +pub(crate) fn emit_loan_invalidations_facts<'tcx>( + tcx: TyCtxt<'tcx>, + all_facts: &mut Option, + location_table: &LocationTable, + body: &Body<'tcx>, + borrow_set: &BorrowSet<'tcx>, +) { + let Some(all_facts) = all_facts else { + // Nothing to do if we don't have any facts to fill + return; + }; + + invalidation::emit_loan_invalidations(tcx, all_facts, location_table, body, borrow_set); +} From 49010833e93ba52ee94934a5af6233631f14873a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Thu, 23 Nov 2023 11:54:49 +0000 Subject: [PATCH 05/13] another trivial cleanup fix a comment and move a variable where it's used --- compiler/rustc_borrowck/src/nll.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_borrowck/src/nll.rs b/compiler/rustc_borrowck/src/nll.rs index 0e1f48902e3..7fa702c2882 100644 --- a/compiler/rustc_borrowck/src/nll.rs +++ b/compiler/rustc_borrowck/src/nll.rs @@ -182,11 +182,10 @@ pub(crate) fn compute_regions<'cx, 'tcx>( borrow_set, ); - let def_id = body.source.def_id(); - - // Dump facts if requested. + // If requested: dump NLL facts, and run legacy polonius analysis. let polonius_output = all_facts.as_ref().and_then(|all_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()); From 9f14698680b6ed94e27d1e71a8b456a87946f181 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Thu, 23 Nov 2023 12:31:29 +0000 Subject: [PATCH 06/13] extract polonius "constraint generation" to help review, this duplicates the existing NLL + polonius constraint generation component, before splitting them up to only do what they individually need. --- compiler/rustc_borrowck/src/nll.rs | 8 + .../src/polonius/constraint_generation.rs | 248 ++++++++++++++++++ compiler/rustc_borrowck/src/polonius/mod.rs | 26 +- 3 files changed, 280 insertions(+), 2 deletions(-) create mode 100644 compiler/rustc_borrowck/src/polonius/constraint_generation.rs diff --git a/compiler/rustc_borrowck/src/nll.rs b/compiler/rustc_borrowck/src/nll.rs index 7fa702c2882..2c32b9310b0 100644 --- a/compiler/rustc_borrowck/src/nll.rs +++ b/compiler/rustc_borrowck/src/nll.rs @@ -157,6 +157,14 @@ pub(crate) fn compute_regions<'cx, 'tcx>( body, borrow_set, ); + polonius::emit_cfg_and_loan_kills_facts( + infcx, + &mut liveness_constraints, + &mut all_facts, + location_table, + body, + borrow_set, + ); let mut regioncx = RegionInferenceContext::new( infcx, diff --git a/compiler/rustc_borrowck/src/polonius/constraint_generation.rs b/compiler/rustc_borrowck/src/polonius/constraint_generation.rs new file mode 100644 index 00000000000..1f642099f08 --- /dev/null +++ b/compiler/rustc_borrowck/src/polonius/constraint_generation.rs @@ -0,0 +1,248 @@ +#![deny(rustc::untranslatable_diagnostic)] +#![deny(rustc::diagnostic_outside_of_impl)] +use rustc_infer::infer::InferCtxt; +use rustc_middle::mir::visit::TyContext; +use rustc_middle::mir::visit::Visitor; +use rustc_middle::mir::{ + Body, Local, Location, Place, PlaceRef, ProjectionElem, Rvalue, SourceInfo, Statement, + StatementKind, Terminator, TerminatorKind, UserTypeProjection, +}; +use rustc_middle::ty::visit::TypeVisitable; +use rustc_middle::ty::GenericArgsRef; +use rustc_middle::ty::{self, RegionVid, Ty, TyCtxt}; + +use crate::{ + borrow_set::BorrowSet, facts::AllFacts, location::LocationTable, places_conflict, + region_infer::values::LivenessValues, +}; + +pub(super) fn generate_constraints<'tcx>( + infcx: &InferCtxt<'tcx>, + liveness_constraints: &mut LivenessValues, + all_facts: &mut Option, + location_table: &LocationTable, + body: &Body<'tcx>, + borrow_set: &BorrowSet<'tcx>, +) { + let mut cg = ConstraintGeneration { + borrow_set, + infcx, + liveness_constraints, + location_table, + all_facts, + body, + }; + + for (bb, data) in body.basic_blocks.iter_enumerated() { + cg.visit_basic_block_data(bb, data); + } +} + +/// 'cg = the duration of the constraint generation process itself. +struct ConstraintGeneration<'cg, 'tcx> { + infcx: &'cg InferCtxt<'tcx>, + all_facts: &'cg mut Option, + location_table: &'cg LocationTable, + liveness_constraints: &'cg mut LivenessValues, + borrow_set: &'cg BorrowSet<'tcx>, + body: &'cg Body<'tcx>, +} + +impl<'cg, 'tcx> Visitor<'tcx> for ConstraintGeneration<'cg, '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: Location) { + self.add_regular_live_constraint(*args, location); + self.super_args(args); + } + + /// We sometimes have `region` within an rvalue, or within a + /// call. Make them live at the location where they appear. + fn visit_region(&mut self, region: ty::Region<'tcx>, location: Location) { + self.add_regular_live_constraint(region, location); + self.super_region(region); + } + + /// We sometimes have `ty` 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::UserTy(span) + | TyContext::LocalDecl { source_info: SourceInfo { span, .. }, .. } => { + span_bug!(span, "should not be visiting outside of the CFG: {:?}", ty_context); + } + TyContext::Location(location) => { + self.add_regular_live_constraint(ty, location); + } + } + + self.super_ty(ty); + } + + fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) { + if let Some(all_facts) = self.all_facts { + let _prof_timer = self.infcx.tcx.prof.generic_activity("polonius_fact_generation"); + all_facts.cfg_edge.push(( + self.location_table.start_index(location), + self.location_table.mid_index(location), + )); + + all_facts.cfg_edge.push(( + self.location_table.mid_index(location), + self.location_table.start_index(location.successor_within_block()), + )); + + // If there are borrows on this now dead local, we need to record them as `killed`. + if let StatementKind::StorageDead(local) = statement.kind { + record_killed_borrows_for_local( + all_facts, + self.borrow_set, + self.location_table, + local, + location, + ); + } + } + + self.super_statement(statement, location); + } + + fn visit_assign(&mut self, place: &Place<'tcx>, rvalue: &Rvalue<'tcx>, location: Location) { + // When we see `X = ...`, then kill borrows of + // `(*X).foo` and so forth. + self.record_killed_borrows_for_place(*place, location); + + self.super_assign(place, rvalue, location); + } + + fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location) { + if let Some(all_facts) = self.all_facts { + let _prof_timer = self.infcx.tcx.prof.generic_activity("polonius_fact_generation"); + all_facts.cfg_edge.push(( + self.location_table.start_index(location), + self.location_table.mid_index(location), + )); + + let successor_blocks = terminator.successors(); + all_facts.cfg_edge.reserve(successor_blocks.size_hint().0); + for successor_block in successor_blocks { + all_facts.cfg_edge.push(( + self.location_table.mid_index(location), + self.location_table.start_index(successor_block.start_location()), + )); + } + } + + // A `Call` terminator's return value can be a local which has borrows, + // so we need to record those as `killed` as well. + if let TerminatorKind::Call { destination, .. } = terminator.kind { + self.record_killed_borrows_for_place(destination, location); + } + + self.super_terminator(terminator, location); + } + + fn visit_ascribe_user_ty( + &mut self, + _place: &Place<'tcx>, + _variance: ty::Variance, + _user_ty: &UserTypeProjection, + _location: Location, + ) { + } +} + +impl<'cx, 'tcx> ConstraintGeneration<'cx, 'tcx> { + /// Some variable with type `live_ty` is "regular live" at + /// `location` -- i.e., it may be used later. This means that all + /// regions appearing in the type `live_ty` must be live at + /// `location`. + fn add_regular_live_constraint(&mut self, live_ty: T, location: Location) + where + T: TypeVisitable>, + { + debug!("add_regular_live_constraint(live_ty={:?}, location={:?})", live_ty, location); + + self.infcx.tcx.for_each_free_region(&live_ty, |live_region| { + let vid = live_region.as_var(); + self.liveness_constraints.add_element(vid, location); + }); + } + + /// When recording facts for Polonius, records the borrows on the specified place + /// as `killed`. For example, when assigning to a local, or on a call's return destination. + fn record_killed_borrows_for_place(&mut self, place: Place<'tcx>, location: Location) { + if let Some(all_facts) = self.all_facts { + let _prof_timer = self.infcx.tcx.prof.generic_activity("polonius_fact_generation"); + + // Depending on the `Place` we're killing: + // - if it's a local, or a single deref of a local, + // we kill all the borrows on the local. + // - if it's a deeper projection, we have to filter which + // of the borrows are killed: the ones whose `borrowed_place` + // conflicts with the `place`. + match place.as_ref() { + PlaceRef { local, projection: &[] } + | PlaceRef { local, projection: &[ProjectionElem::Deref] } => { + debug!( + "Recording `killed` facts for borrows of local={:?} at location={:?}", + local, location + ); + + record_killed_borrows_for_local( + all_facts, + self.borrow_set, + self.location_table, + local, + location, + ); + } + + PlaceRef { local, projection: &[.., _] } => { + // Kill conflicting borrows of the innermost local. + debug!( + "Recording `killed` facts for borrows of \ + innermost projected local={:?} at location={:?}", + local, location + ); + + if let Some(borrow_indices) = self.borrow_set.local_map.get(&local) { + for &borrow_index in borrow_indices { + let places_conflict = places_conflict::places_conflict( + self.infcx.tcx, + self.body, + self.borrow_set[borrow_index].borrowed_place, + place, + places_conflict::PlaceConflictBias::NoOverlap, + ); + + if places_conflict { + let location_index = self.location_table.mid_index(location); + all_facts.loan_killed_at.push((borrow_index, location_index)); + } + } + } + } + } + } + } +} + +/// When recording facts for Polonius, records the borrows on the specified local as `killed`. +fn record_killed_borrows_for_local( + all_facts: &mut AllFacts, + borrow_set: &BorrowSet<'_>, + location_table: &LocationTable, + local: Local, + location: Location, +) { + if let Some(borrow_indices) = borrow_set.local_map.get(&local) { + all_facts.loan_killed_at.reserve(borrow_indices.len()); + for &borrow_index in borrow_indices { + let location_index = location_table.mid_index(location); + all_facts.loan_killed_at.push((borrow_index, location_index)); + } + } +} diff --git a/compiler/rustc_borrowck/src/polonius/mod.rs b/compiler/rustc_borrowck/src/polonius/mod.rs index 887c5fac16b..b440b9723f0 100644 --- a/compiler/rustc_borrowck/src/polonius/mod.rs +++ b/compiler/rustc_borrowck/src/polonius/mod.rs @@ -3,16 +3,19 @@ //! Will be removed in the future, once the in-tree `-Zpolonius=next` implementation reaches feature //! parity. +use rustc_infer::infer::InferCtxt; use rustc_middle::mir::{Body, LocalKind, Location, START_BLOCK}; -use rustc_middle::ty::TyCtxt; +use rustc_middle::ty::{RegionVid, TyCtxt}; use rustc_mir_dataflow::move_paths::{InitKind, InitLocation, MoveData}; use crate::borrow_set::BorrowSet; use crate::facts::AllFacts; use crate::location::LocationTable; +use crate::region_infer::values::LivenessValues; use crate::type_check::free_region_relations::UniversalRegionRelations; use crate::universal_regions::UniversalRegions; +mod constraint_generation; mod invalidation; /// Emit facts needed for move/init analysis: moves and assignments. @@ -130,7 +133,7 @@ pub(crate) fn emit_universal_region_facts( } } -/// Emit facts about loan invalidations +/// Emit facts about loan invalidations. pub(crate) fn emit_loan_invalidations_facts<'tcx>( tcx: TyCtxt<'tcx>, all_facts: &mut Option, @@ -145,3 +148,22 @@ pub(crate) fn emit_loan_invalidations_facts<'tcx>( invalidation::emit_loan_invalidations(tcx, all_facts, location_table, body, borrow_set); } + +/// Emit facts about CFG points and edges, as well as locations where loans are killed. +pub(crate) fn emit_cfg_and_loan_kills_facts<'tcx>( + infcx: &InferCtxt<'tcx>, + liveness_constraints: &mut LivenessValues, + all_facts: &mut Option, + location_table: &LocationTable, + body: &Body<'tcx>, + borrow_set: &BorrowSet<'tcx>, +) { + constraint_generation::generate_constraints( + infcx, + liveness_constraints, + all_facts, + location_table, + body, + borrow_set, + ); +} From f53412ab7c2c0cea2f82b5d6519f115e73ac807e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Thu, 23 Nov 2023 12:34:15 +0000 Subject: [PATCH 07/13] remove NLL liveness from polonius constraint generation --- compiler/rustc_borrowck/src/nll.rs | 1 - .../src/polonius/constraint_generation.rs | 74 ++----------------- compiler/rustc_borrowck/src/polonius/mod.rs | 13 +--- 3 files changed, 7 insertions(+), 81 deletions(-) diff --git a/compiler/rustc_borrowck/src/nll.rs b/compiler/rustc_borrowck/src/nll.rs index 2c32b9310b0..1b4c92e3ee2 100644 --- a/compiler/rustc_borrowck/src/nll.rs +++ b/compiler/rustc_borrowck/src/nll.rs @@ -159,7 +159,6 @@ pub(crate) fn compute_regions<'cx, 'tcx>( ); polonius::emit_cfg_and_loan_kills_facts( infcx, - &mut liveness_constraints, &mut all_facts, location_table, body, diff --git a/compiler/rustc_borrowck/src/polonius/constraint_generation.rs b/compiler/rustc_borrowck/src/polonius/constraint_generation.rs index 1f642099f08..ac9401924fb 100644 --- a/compiler/rustc_borrowck/src/polonius/constraint_generation.rs +++ b/compiler/rustc_borrowck/src/polonius/constraint_generation.rs @@ -1,38 +1,23 @@ #![deny(rustc::untranslatable_diagnostic)] #![deny(rustc::diagnostic_outside_of_impl)] use rustc_infer::infer::InferCtxt; -use rustc_middle::mir::visit::TyContext; use rustc_middle::mir::visit::Visitor; use rustc_middle::mir::{ - Body, Local, Location, Place, PlaceRef, ProjectionElem, Rvalue, SourceInfo, Statement, - StatementKind, Terminator, TerminatorKind, UserTypeProjection, + Body, Local, Location, Place, PlaceRef, ProjectionElem, Rvalue, Statement, StatementKind, + Terminator, TerminatorKind, UserTypeProjection, }; -use rustc_middle::ty::visit::TypeVisitable; -use rustc_middle::ty::GenericArgsRef; -use rustc_middle::ty::{self, RegionVid, Ty, TyCtxt}; +use rustc_middle::ty::{self}; -use crate::{ - borrow_set::BorrowSet, facts::AllFacts, location::LocationTable, places_conflict, - region_infer::values::LivenessValues, -}; +use crate::{borrow_set::BorrowSet, facts::AllFacts, location::LocationTable, places_conflict}; pub(super) fn generate_constraints<'tcx>( infcx: &InferCtxt<'tcx>, - liveness_constraints: &mut LivenessValues, all_facts: &mut Option, location_table: &LocationTable, body: &Body<'tcx>, borrow_set: &BorrowSet<'tcx>, ) { - let mut cg = ConstraintGeneration { - borrow_set, - infcx, - liveness_constraints, - location_table, - all_facts, - body, - }; - + let mut cg = ConstraintGeneration { borrow_set, infcx, location_table, all_facts, body }; for (bb, data) in body.basic_blocks.iter_enumerated() { cg.visit_basic_block_data(bb, data); } @@ -43,44 +28,11 @@ struct ConstraintGeneration<'cg, 'tcx> { infcx: &'cg InferCtxt<'tcx>, all_facts: &'cg mut Option, location_table: &'cg LocationTable, - liveness_constraints: &'cg mut LivenessValues, borrow_set: &'cg BorrowSet<'tcx>, body: &'cg Body<'tcx>, } impl<'cg, 'tcx> Visitor<'tcx> for ConstraintGeneration<'cg, '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: Location) { - self.add_regular_live_constraint(*args, location); - self.super_args(args); - } - - /// We sometimes have `region` within an rvalue, or within a - /// call. Make them live at the location where they appear. - fn visit_region(&mut self, region: ty::Region<'tcx>, location: Location) { - self.add_regular_live_constraint(region, location); - self.super_region(region); - } - - /// We sometimes have `ty` 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::UserTy(span) - | TyContext::LocalDecl { source_info: SourceInfo { span, .. }, .. } => { - span_bug!(span, "should not be visiting outside of the CFG: {:?}", ty_context); - } - TyContext::Location(location) => { - self.add_regular_live_constraint(ty, location); - } - } - - self.super_ty(ty); - } - fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) { if let Some(all_facts) = self.all_facts { let _prof_timer = self.infcx.tcx.prof.generic_activity("polonius_fact_generation"); @@ -155,22 +107,6 @@ impl<'cg, 'tcx> Visitor<'tcx> for ConstraintGeneration<'cg, 'tcx> { } impl<'cx, 'tcx> ConstraintGeneration<'cx, 'tcx> { - /// Some variable with type `live_ty` is "regular live" at - /// `location` -- i.e., it may be used later. This means that all - /// regions appearing in the type `live_ty` must be live at - /// `location`. - fn add_regular_live_constraint(&mut self, live_ty: T, location: Location) - where - T: TypeVisitable>, - { - debug!("add_regular_live_constraint(live_ty={:?}, location={:?})", live_ty, location); - - self.infcx.tcx.for_each_free_region(&live_ty, |live_region| { - let vid = live_region.as_var(); - self.liveness_constraints.add_element(vid, location); - }); - } - /// When recording facts for Polonius, records the borrows on the specified place /// as `killed`. For example, when assigning to a local, or on a call's return destination. fn record_killed_borrows_for_place(&mut self, place: Place<'tcx>, location: Location) { diff --git a/compiler/rustc_borrowck/src/polonius/mod.rs b/compiler/rustc_borrowck/src/polonius/mod.rs index b440b9723f0..c41afc8078a 100644 --- a/compiler/rustc_borrowck/src/polonius/mod.rs +++ b/compiler/rustc_borrowck/src/polonius/mod.rs @@ -5,13 +5,12 @@ use rustc_infer::infer::InferCtxt; use rustc_middle::mir::{Body, LocalKind, Location, START_BLOCK}; -use rustc_middle::ty::{RegionVid, TyCtxt}; +use rustc_middle::ty::TyCtxt; use rustc_mir_dataflow::move_paths::{InitKind, InitLocation, MoveData}; use crate::borrow_set::BorrowSet; use crate::facts::AllFacts; use crate::location::LocationTable; -use crate::region_infer::values::LivenessValues; use crate::type_check::free_region_relations::UniversalRegionRelations; use crate::universal_regions::UniversalRegions; @@ -152,18 +151,10 @@ pub(crate) fn emit_loan_invalidations_facts<'tcx>( /// Emit facts about CFG points and edges, as well as locations where loans are killed. pub(crate) fn emit_cfg_and_loan_kills_facts<'tcx>( infcx: &InferCtxt<'tcx>, - liveness_constraints: &mut LivenessValues, all_facts: &mut Option, location_table: &LocationTable, body: &Body<'tcx>, borrow_set: &BorrowSet<'tcx>, ) { - constraint_generation::generate_constraints( - infcx, - liveness_constraints, - all_facts, - location_table, - body, - borrow_set, - ); + constraint_generation::generate_constraints(infcx, all_facts, location_table, body, borrow_set); } From bfd88b0bf1515fb11dbb8f624b20e43ab74d0001 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Thu, 23 Nov 2023 13:49:14 +0000 Subject: [PATCH 08/13] simplify polonius constraint generation --- compiler/rustc_borrowck/src/nll.rs | 2 +- .../src/polonius/constraint_generation.rs | 170 ++++++++---------- compiler/rustc_borrowck/src/polonius/mod.rs | 10 +- 3 files changed, 79 insertions(+), 103 deletions(-) diff --git a/compiler/rustc_borrowck/src/nll.rs b/compiler/rustc_borrowck/src/nll.rs index 1b4c92e3ee2..0c88f5cf363 100644 --- a/compiler/rustc_borrowck/src/nll.rs +++ b/compiler/rustc_borrowck/src/nll.rs @@ -158,7 +158,7 @@ pub(crate) fn compute_regions<'cx, 'tcx>( borrow_set, ); polonius::emit_cfg_and_loan_kills_facts( - infcx, + infcx.tcx, &mut all_facts, location_table, body, diff --git a/compiler/rustc_borrowck/src/polonius/constraint_generation.rs b/compiler/rustc_borrowck/src/polonius/constraint_generation.rs index ac9401924fb..27efc3c4108 100644 --- a/compiler/rustc_borrowck/src/polonius/constraint_generation.rs +++ b/compiler/rustc_borrowck/src/polonius/constraint_generation.rs @@ -1,23 +1,23 @@ #![deny(rustc::untranslatable_diagnostic)] #![deny(rustc::diagnostic_outside_of_impl)] -use rustc_infer::infer::InferCtxt; use rustc_middle::mir::visit::Visitor; use rustc_middle::mir::{ Body, Local, Location, Place, PlaceRef, ProjectionElem, Rvalue, Statement, StatementKind, Terminator, TerminatorKind, UserTypeProjection, }; -use rustc_middle::ty::{self}; +use rustc_middle::ty::{TyCtxt, Variance}; use crate::{borrow_set::BorrowSet, facts::AllFacts, location::LocationTable, places_conflict}; pub(super) fn generate_constraints<'tcx>( - infcx: &InferCtxt<'tcx>, - all_facts: &mut Option, + tcx: TyCtxt<'tcx>, + all_facts: &mut AllFacts, location_table: &LocationTable, body: &Body<'tcx>, borrow_set: &BorrowSet<'tcx>, ) { - let mut cg = ConstraintGeneration { borrow_set, infcx, location_table, all_facts, body }; + let _prof_timer = tcx.prof.generic_activity("polonius_fact_generation"); + let mut cg = ConstraintGeneration { borrow_set, tcx, location_table, all_facts, body }; for (bb, data) in body.basic_blocks.iter_enumerated() { cg.visit_basic_block_data(bb, data); } @@ -25,8 +25,8 @@ pub(super) fn generate_constraints<'tcx>( /// 'cg = the duration of the constraint generation process itself. struct ConstraintGeneration<'cg, 'tcx> { - infcx: &'cg InferCtxt<'tcx>, - all_facts: &'cg mut Option, + tcx: TyCtxt<'tcx>, + all_facts: &'cg mut AllFacts, location_table: &'cg LocationTable, borrow_set: &'cg BorrowSet<'tcx>, body: &'cg Body<'tcx>, @@ -34,28 +34,19 @@ struct ConstraintGeneration<'cg, 'tcx> { impl<'cg, 'tcx> Visitor<'tcx> for ConstraintGeneration<'cg, 'tcx> { fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) { - if let Some(all_facts) = self.all_facts { - let _prof_timer = self.infcx.tcx.prof.generic_activity("polonius_fact_generation"); - all_facts.cfg_edge.push(( - self.location_table.start_index(location), - self.location_table.mid_index(location), - )); + self.all_facts.cfg_edge.push(( + self.location_table.start_index(location), + self.location_table.mid_index(location), + )); - all_facts.cfg_edge.push(( - self.location_table.mid_index(location), - self.location_table.start_index(location.successor_within_block()), - )); + self.all_facts.cfg_edge.push(( + self.location_table.mid_index(location), + self.location_table.start_index(location.successor_within_block()), + )); - // If there are borrows on this now dead local, we need to record them as `killed`. - if let StatementKind::StorageDead(local) = statement.kind { - record_killed_borrows_for_local( - all_facts, - self.borrow_set, - self.location_table, - local, - location, - ); - } + // If there are borrows on this now dead local, we need to record them as `killed`. + if let StatementKind::StorageDead(local) = statement.kind { + self.record_killed_borrows_for_local(local, location); } self.super_statement(statement, location); @@ -70,21 +61,18 @@ impl<'cg, 'tcx> Visitor<'tcx> for ConstraintGeneration<'cg, 'tcx> { } fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location) { - if let Some(all_facts) = self.all_facts { - let _prof_timer = self.infcx.tcx.prof.generic_activity("polonius_fact_generation"); - all_facts.cfg_edge.push(( - self.location_table.start_index(location), - self.location_table.mid_index(location), - )); + self.all_facts.cfg_edge.push(( + self.location_table.start_index(location), + self.location_table.mid_index(location), + )); - let successor_blocks = terminator.successors(); - all_facts.cfg_edge.reserve(successor_blocks.size_hint().0); - for successor_block in successor_blocks { - all_facts.cfg_edge.push(( - self.location_table.mid_index(location), - self.location_table.start_index(successor_block.start_location()), - )); - } + let successor_blocks = terminator.successors(); + self.all_facts.cfg_edge.reserve(successor_blocks.size_hint().0); + for successor_block in successor_blocks { + self.all_facts.cfg_edge.push(( + self.location_table.mid_index(location), + self.location_table.start_index(successor_block.start_location()), + )); } // A `Call` terminator's return value can be a local which has borrows, @@ -99,7 +87,7 @@ impl<'cg, 'tcx> Visitor<'tcx> for ConstraintGeneration<'cg, 'tcx> { fn visit_ascribe_user_ty( &mut self, _place: &Place<'tcx>, - _variance: ty::Variance, + _variance: Variance, _user_ty: &UserTypeProjection, _location: Location, ) { @@ -110,75 +98,59 @@ impl<'cx, 'tcx> ConstraintGeneration<'cx, 'tcx> { /// When recording facts for Polonius, records the borrows on the specified place /// as `killed`. For example, when assigning to a local, or on a call's return destination. fn record_killed_borrows_for_place(&mut self, place: Place<'tcx>, location: Location) { - if let Some(all_facts) = self.all_facts { - let _prof_timer = self.infcx.tcx.prof.generic_activity("polonius_fact_generation"); + // Depending on the `Place` we're killing: + // - if it's a local, or a single deref of a local, + // we kill all the borrows on the local. + // - if it's a deeper projection, we have to filter which + // of the borrows are killed: the ones whose `borrowed_place` + // conflicts with the `place`. + match place.as_ref() { + PlaceRef { local, projection: &[] } + | PlaceRef { local, projection: &[ProjectionElem::Deref] } => { + debug!( + "Recording `killed` facts for borrows of local={:?} at location={:?}", + local, location + ); - // Depending on the `Place` we're killing: - // - if it's a local, or a single deref of a local, - // we kill all the borrows on the local. - // - if it's a deeper projection, we have to filter which - // of the borrows are killed: the ones whose `borrowed_place` - // conflicts with the `place`. - match place.as_ref() { - PlaceRef { local, projection: &[] } - | PlaceRef { local, projection: &[ProjectionElem::Deref] } => { - debug!( - "Recording `killed` facts for borrows of local={:?} at location={:?}", - local, location - ); + self.record_killed_borrows_for_local(local, location); + } - record_killed_borrows_for_local( - all_facts, - self.borrow_set, - self.location_table, - local, - location, - ); - } - - PlaceRef { local, projection: &[.., _] } => { - // Kill conflicting borrows of the innermost local. - debug!( - "Recording `killed` facts for borrows of \ + PlaceRef { local, projection: &[.., _] } => { + // Kill conflicting borrows of the innermost local. + debug!( + "Recording `killed` facts for borrows of \ innermost projected local={:?} at location={:?}", - local, location - ); + local, location + ); - if let Some(borrow_indices) = self.borrow_set.local_map.get(&local) { - for &borrow_index in borrow_indices { - let places_conflict = places_conflict::places_conflict( - self.infcx.tcx, - self.body, - self.borrow_set[borrow_index].borrowed_place, - place, - places_conflict::PlaceConflictBias::NoOverlap, - ); + if let Some(borrow_indices) = self.borrow_set.local_map.get(&local) { + for &borrow_index in borrow_indices { + let places_conflict = places_conflict::places_conflict( + self.tcx, + self.body, + self.borrow_set[borrow_index].borrowed_place, + place, + places_conflict::PlaceConflictBias::NoOverlap, + ); - if places_conflict { - let location_index = self.location_table.mid_index(location); - all_facts.loan_killed_at.push((borrow_index, location_index)); - } + if places_conflict { + let location_index = self.location_table.mid_index(location); + self.all_facts.loan_killed_at.push((borrow_index, location_index)); } } } } } } -} -/// When recording facts for Polonius, records the borrows on the specified local as `killed`. -fn record_killed_borrows_for_local( - all_facts: &mut AllFacts, - borrow_set: &BorrowSet<'_>, - location_table: &LocationTable, - local: Local, - location: Location, -) { - if let Some(borrow_indices) = borrow_set.local_map.get(&local) { - all_facts.loan_killed_at.reserve(borrow_indices.len()); - for &borrow_index in borrow_indices { - let location_index = location_table.mid_index(location); - all_facts.loan_killed_at.push((borrow_index, location_index)); + /// When recording facts for Polonius, records the borrows on the specified local as `killed`. + fn record_killed_borrows_for_local(&mut self, local: Local, location: Location) { + if let Some(borrow_indices) = self.borrow_set.local_map.get(&local) { + let location_index = self.location_table.mid_index(location); + self.all_facts.loan_killed_at.reserve(borrow_indices.len()); + for &borrow_index in borrow_indices { + self.all_facts.loan_killed_at.push((borrow_index, location_index)); + } } } } diff --git a/compiler/rustc_borrowck/src/polonius/mod.rs b/compiler/rustc_borrowck/src/polonius/mod.rs index c41afc8078a..0fbe221ad10 100644 --- a/compiler/rustc_borrowck/src/polonius/mod.rs +++ b/compiler/rustc_borrowck/src/polonius/mod.rs @@ -3,7 +3,6 @@ //! Will be removed in the future, once the in-tree `-Zpolonius=next` implementation reaches feature //! parity. -use rustc_infer::infer::InferCtxt; use rustc_middle::mir::{Body, LocalKind, Location, START_BLOCK}; use rustc_middle::ty::TyCtxt; use rustc_mir_dataflow::move_paths::{InitKind, InitLocation, MoveData}; @@ -150,11 +149,16 @@ pub(crate) fn emit_loan_invalidations_facts<'tcx>( /// Emit facts about CFG points and edges, as well as locations where loans are killed. pub(crate) fn emit_cfg_and_loan_kills_facts<'tcx>( - infcx: &InferCtxt<'tcx>, + tcx: TyCtxt<'tcx>, all_facts: &mut Option, location_table: &LocationTable, body: &Body<'tcx>, borrow_set: &BorrowSet<'tcx>, ) { - constraint_generation::generate_constraints(infcx, all_facts, location_table, body, borrow_set); + let Some(all_facts) = all_facts else { + // Nothing to do if we don't have any facts to fill + return; + }; + + constraint_generation::generate_constraints(tcx, all_facts, location_table, body, borrow_set); } From 951901bedc25b77d3f1959266225911d499d7003 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Thu, 23 Nov 2023 13:58:55 +0000 Subject: [PATCH 09/13] rename polonius constraint generation to what it actually does: emit loan kills --- ...constraint_generation.rs => loan_kills.rs} | 44 ++++++++----------- compiler/rustc_borrowck/src/polonius/mod.rs | 4 +- 2 files changed, 20 insertions(+), 28 deletions(-) rename compiler/rustc_borrowck/src/polonius/{constraint_generation.rs => loan_kills.rs} (82%) diff --git a/compiler/rustc_borrowck/src/polonius/constraint_generation.rs b/compiler/rustc_borrowck/src/polonius/loan_kills.rs similarity index 82% rename from compiler/rustc_borrowck/src/polonius/constraint_generation.rs rename to compiler/rustc_borrowck/src/polonius/loan_kills.rs index 27efc3c4108..cc836115c31 100644 --- a/compiler/rustc_borrowck/src/polonius/constraint_generation.rs +++ b/compiler/rustc_borrowck/src/polonius/loan_kills.rs @@ -3,13 +3,14 @@ use rustc_middle::mir::visit::Visitor; use rustc_middle::mir::{ Body, Local, Location, Place, PlaceRef, ProjectionElem, Rvalue, Statement, StatementKind, - Terminator, TerminatorKind, UserTypeProjection, + Terminator, TerminatorKind, }; -use rustc_middle::ty::{TyCtxt, Variance}; +use rustc_middle::ty::TyCtxt; use crate::{borrow_set::BorrowSet, facts::AllFacts, location::LocationTable, places_conflict}; -pub(super) fn generate_constraints<'tcx>( +/// Emit `loan_killed_at` and `cfg_edge` facts at the same time. +pub(super) fn emit_loan_kills<'tcx>( tcx: TyCtxt<'tcx>, all_facts: &mut AllFacts, location_table: &LocationTable, @@ -17,23 +18,23 @@ pub(super) fn generate_constraints<'tcx>( borrow_set: &BorrowSet<'tcx>, ) { let _prof_timer = tcx.prof.generic_activity("polonius_fact_generation"); - let mut cg = ConstraintGeneration { borrow_set, tcx, location_table, all_facts, body }; + let mut visitor = LoanKillsGenerator { borrow_set, tcx, location_table, all_facts, body }; for (bb, data) in body.basic_blocks.iter_enumerated() { - cg.visit_basic_block_data(bb, data); + visitor.visit_basic_block_data(bb, data); } } -/// 'cg = the duration of the constraint generation process itself. -struct ConstraintGeneration<'cg, 'tcx> { +struct LoanKillsGenerator<'cx, 'tcx> { tcx: TyCtxt<'tcx>, - all_facts: &'cg mut AllFacts, - location_table: &'cg LocationTable, - borrow_set: &'cg BorrowSet<'tcx>, - body: &'cg Body<'tcx>, + all_facts: &'cx mut AllFacts, + location_table: &'cx LocationTable, + borrow_set: &'cx BorrowSet<'tcx>, + body: &'cx Body<'tcx>, } -impl<'cg, 'tcx> Visitor<'tcx> for ConstraintGeneration<'cg, 'tcx> { +impl<'cx, 'tcx> Visitor<'tcx> for LoanKillsGenerator<'cx, 'tcx> { fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) { + // Also record CFG facts here. self.all_facts.cfg_edge.push(( self.location_table.start_index(location), self.location_table.mid_index(location), @@ -56,11 +57,11 @@ impl<'cg, 'tcx> Visitor<'tcx> for ConstraintGeneration<'cg, 'tcx> { // When we see `X = ...`, then kill borrows of // `(*X).foo` and so forth. self.record_killed_borrows_for_place(*place, location); - self.super_assign(place, rvalue, location); } fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location) { + // Also record CFG facts here. self.all_facts.cfg_edge.push(( self.location_table.start_index(location), self.location_table.mid_index(location), @@ -83,20 +84,11 @@ impl<'cg, 'tcx> Visitor<'tcx> for ConstraintGeneration<'cg, 'tcx> { self.super_terminator(terminator, location); } - - fn visit_ascribe_user_ty( - &mut self, - _place: &Place<'tcx>, - _variance: Variance, - _user_ty: &UserTypeProjection, - _location: Location, - ) { - } } -impl<'cx, 'tcx> ConstraintGeneration<'cx, 'tcx> { - /// When recording facts for Polonius, records the borrows on the specified place - /// as `killed`. For example, when assigning to a local, or on a call's return destination. +impl<'tcx> LoanKillsGenerator<'_, 'tcx> { + /// Records the borrows on the specified place as `killed`. For example, when assigning to a + /// local, or on a call's return destination. fn record_killed_borrows_for_place(&mut self, place: Place<'tcx>, location: Location) { // Depending on the `Place` we're killing: // - if it's a local, or a single deref of a local, @@ -143,7 +135,7 @@ impl<'cx, 'tcx> ConstraintGeneration<'cx, 'tcx> { } } - /// When recording facts for Polonius, records the borrows on the specified local as `killed`. + /// Records the borrows on the specified local as `killed`. fn record_killed_borrows_for_local(&mut self, local: Local, location: Location) { if let Some(borrow_indices) = self.borrow_set.local_map.get(&local) { let location_index = self.location_table.mid_index(location); diff --git a/compiler/rustc_borrowck/src/polonius/mod.rs b/compiler/rustc_borrowck/src/polonius/mod.rs index 0fbe221ad10..c3fdc6c7b96 100644 --- a/compiler/rustc_borrowck/src/polonius/mod.rs +++ b/compiler/rustc_borrowck/src/polonius/mod.rs @@ -13,8 +13,8 @@ use crate::location::LocationTable; use crate::type_check::free_region_relations::UniversalRegionRelations; use crate::universal_regions::UniversalRegions; -mod constraint_generation; mod invalidation; +mod loan_kills; /// Emit facts needed for move/init analysis: moves and assignments. pub(crate) fn emit_move_facts( @@ -160,5 +160,5 @@ pub(crate) fn emit_cfg_and_loan_kills_facts<'tcx>( return; }; - constraint_generation::generate_constraints(tcx, all_facts, location_table, body, borrow_set); + loan_kills::emit_loan_kills(tcx, all_facts, location_table, body, borrow_set); } From c976bc114a36b773484689e702e6a4467bb51f42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Thu, 23 Nov 2023 14:19:04 +0000 Subject: [PATCH 10/13] small polish of loan invalidations fact generation --- ...{invalidation.rs => loan_invalidations.rs} | 30 +++++++------------ compiler/rustc_borrowck/src/polonius/mod.rs | 4 +-- 2 files changed, 12 insertions(+), 22 deletions(-) rename compiler/rustc_borrowck/src/polonius/{invalidation.rs => loan_invalidations.rs} (96%) diff --git a/compiler/rustc_borrowck/src/polonius/invalidation.rs b/compiler/rustc_borrowck/src/polonius/loan_invalidations.rs similarity index 96% rename from compiler/rustc_borrowck/src/polonius/invalidation.rs rename to compiler/rustc_borrowck/src/polonius/loan_invalidations.rs index 43119a97bee..23bf8457816 100644 --- a/compiler/rustc_borrowck/src/polonius/invalidation.rs +++ b/compiler/rustc_borrowck/src/polonius/loan_invalidations.rs @@ -24,18 +24,12 @@ pub(super) fn emit_loan_invalidations<'tcx>( ) { let _prof_timer = tcx.prof.generic_activity("polonius_fact_generation"); let dominators = body.basic_blocks.dominators(); - let mut ig = InvalidationGenerator { - all_facts, - borrow_set, - tcx, - location_table, - body: body, - dominators, - }; - ig.visit_body(body); + let mut visitor = + LoanInvalidationsGenerator { all_facts, borrow_set, tcx, location_table, body, dominators }; + visitor.visit_body(body); } -struct InvalidationGenerator<'cx, 'tcx> { +struct LoanInvalidationsGenerator<'cx, 'tcx> { tcx: TyCtxt<'tcx>, all_facts: &'cx mut AllFacts, location_table: &'cx LocationTable, @@ -46,7 +40,7 @@ struct InvalidationGenerator<'cx, 'tcx> { /// Visits the whole MIR and generates `invalidates()` facts. /// Most of the code implementing this was stolen from `borrow_check/mod.rs`. -impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> { +impl<'cx, 'tcx> Visitor<'tcx> for LoanInvalidationsGenerator<'cx, 'tcx> { fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) { self.check_activations(location); @@ -208,7 +202,7 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> { } } -impl<'cx, 'tcx> InvalidationGenerator<'cx, 'tcx> { +impl<'cx, 'tcx> LoanInvalidationsGenerator<'cx, 'tcx> { /// Simulates mutation of a place. fn mutate_place(&mut self, location: Location, place: Place<'tcx>, kind: AccessDepth) { self.access_place( @@ -342,20 +336,16 @@ impl<'cx, 'tcx> InvalidationGenerator<'cx, 'tcx> { rw: ReadOrWrite, ) { debug!( - "invalidation::check_access_for_conflict(location={:?}, place={:?}, sd={:?}, \ - rw={:?})", + "check_access_for_conflict(location={:?}, place={:?}, sd={:?}, rw={:?})", location, place, sd, rw, ); - let tcx = self.tcx; - let body = self.body; - let borrow_set = self.borrow_set; each_borrow_involving_path( self, - tcx, - body, + self.tcx, + self.body, location, (sd, place), - borrow_set, + self.borrow_set, |_| true, |this, borrow_index, borrow| { match (rw, borrow.kind) { diff --git a/compiler/rustc_borrowck/src/polonius/mod.rs b/compiler/rustc_borrowck/src/polonius/mod.rs index c3fdc6c7b96..9454118d9c4 100644 --- a/compiler/rustc_borrowck/src/polonius/mod.rs +++ b/compiler/rustc_borrowck/src/polonius/mod.rs @@ -13,7 +13,7 @@ use crate::location::LocationTable; use crate::type_check::free_region_relations::UniversalRegionRelations; use crate::universal_regions::UniversalRegions; -mod invalidation; +mod loan_invalidations; mod loan_kills; /// Emit facts needed for move/init analysis: moves and assignments. @@ -144,7 +144,7 @@ pub(crate) fn emit_loan_invalidations_facts<'tcx>( return; }; - invalidation::emit_loan_invalidations(tcx, all_facts, location_table, body, borrow_set); + loan_invalidations::emit_loan_invalidations(tcx, all_facts, location_table, body, borrow_set); } /// Emit facts about CFG points and edges, as well as locations where loans are killed. From 626289a9e36dd6262b90eb7fb33f1fb6b3266cd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Thu, 23 Nov 2023 14:43:27 +0000 Subject: [PATCH 11/13] remove polonius fact generation from NLL constraint generation --- .../src/constraint_generation.rs | 214 ++---------------- compiler/rustc_borrowck/src/nll.rs | 9 +- 2 files changed, 21 insertions(+), 202 deletions(-) diff --git a/compiler/rustc_borrowck/src/constraint_generation.rs b/compiler/rustc_borrowck/src/constraint_generation.rs index 21d367c40cb..445c58a2511 100644 --- a/compiler/rustc_borrowck/src/constraint_generation.rs +++ b/compiler/rustc_borrowck/src/constraint_generation.rs @@ -1,38 +1,18 @@ #![deny(rustc::untranslatable_diagnostic)] #![deny(rustc::diagnostic_outside_of_impl)] -use rustc_infer::infer::InferCtxt; -use rustc_middle::mir::visit::TyContext; -use rustc_middle::mir::visit::Visitor; -use rustc_middle::mir::{ - Body, Local, Location, Place, PlaceRef, ProjectionElem, Rvalue, SourceInfo, Statement, - StatementKind, Terminator, TerminatorKind, UserTypeProjection, -}; +use rustc_middle::mir::visit::{TyContext, Visitor}; +use rustc_middle::mir::{Body, Location, SourceInfo}; use rustc_middle::ty::visit::TypeVisitable; -use rustc_middle::ty::GenericArgsRef; -use rustc_middle::ty::{self, Ty, TyCtxt}; +use rustc_middle::ty::{GenericArgsRef, Region, Ty, TyCtxt}; -use crate::{ - borrow_set::BorrowSet, facts::AllFacts, location::LocationTable, places_conflict, - region_infer::values::LivenessValues, -}; +use crate::region_infer::values::LivenessValues; pub(super) fn generate_constraints<'tcx>( - infcx: &InferCtxt<'tcx>, + tcx: TyCtxt<'tcx>, liveness_constraints: &mut LivenessValues, - all_facts: &mut Option, - location_table: &LocationTable, body: &Body<'tcx>, - borrow_set: &BorrowSet<'tcx>, ) { - let mut cg = ConstraintGeneration { - borrow_set, - infcx, - liveness_constraints, - location_table, - all_facts, - body, - }; - + let mut cg = ConstraintGeneration { tcx, liveness_constraints }; for (bb, data) in body.basic_blocks.iter_enumerated() { cg.visit_basic_block_data(bb, data); } @@ -40,30 +20,26 @@ pub(super) fn generate_constraints<'tcx>( /// 'cg = the duration of the constraint generation process itself. struct ConstraintGeneration<'cg, 'tcx> { - infcx: &'cg InferCtxt<'tcx>, - all_facts: &'cg mut Option, - location_table: &'cg LocationTable, + tcx: TyCtxt<'tcx>, liveness_constraints: &'cg mut LivenessValues, - borrow_set: &'cg BorrowSet<'tcx>, - body: &'cg Body<'tcx>, } impl<'cg, 'tcx> Visitor<'tcx> for ConstraintGeneration<'cg, '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: Location) { - self.add_regular_live_constraint(*args, location); + self.record_regions_live_at(*args, location); self.super_args(args); } - /// We sometimes have `region` within an rvalue, or within a + /// 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: ty::Region<'tcx>, location: Location) { - self.add_regular_live_constraint(region, location); + fn visit_region(&mut self, region: Region<'tcx>, location: Location) { + self.record_regions_live_at(region, location); self.super_region(region); } - /// We sometimes have `ty` within an rvalue, or within a + /// 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 { @@ -74,175 +50,25 @@ impl<'cg, 'tcx> Visitor<'tcx> for ConstraintGeneration<'cg, 'tcx> { span_bug!(span, "should not be visiting outside of the CFG: {:?}", ty_context); } TyContext::Location(location) => { - self.add_regular_live_constraint(ty, location); + self.record_regions_live_at(ty, location); } } self.super_ty(ty); } - - fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) { - if let Some(all_facts) = self.all_facts { - let _prof_timer = self.infcx.tcx.prof.generic_activity("polonius_fact_generation"); - all_facts.cfg_edge.push(( - self.location_table.start_index(location), - self.location_table.mid_index(location), - )); - - all_facts.cfg_edge.push(( - self.location_table.mid_index(location), - self.location_table.start_index(location.successor_within_block()), - )); - - // If there are borrows on this now dead local, we need to record them as `killed`. - if let StatementKind::StorageDead(local) = statement.kind { - record_killed_borrows_for_local( - all_facts, - self.borrow_set, - self.location_table, - local, - location, - ); - } - } - - self.super_statement(statement, location); - } - - fn visit_assign(&mut self, place: &Place<'tcx>, rvalue: &Rvalue<'tcx>, location: Location) { - // When we see `X = ...`, then kill borrows of - // `(*X).foo` and so forth. - self.record_killed_borrows_for_place(*place, location); - - self.super_assign(place, rvalue, location); - } - - fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location) { - if let Some(all_facts) = self.all_facts { - let _prof_timer = self.infcx.tcx.prof.generic_activity("polonius_fact_generation"); - all_facts.cfg_edge.push(( - self.location_table.start_index(location), - self.location_table.mid_index(location), - )); - - let successor_blocks = terminator.successors(); - all_facts.cfg_edge.reserve(successor_blocks.size_hint().0); - for successor_block in successor_blocks { - all_facts.cfg_edge.push(( - self.location_table.mid_index(location), - self.location_table.start_index(successor_block.start_location()), - )); - } - } - - // A `Call` terminator's return value can be a local which has borrows, - // so we need to record those as `killed` as well. - if let TerminatorKind::Call { destination, .. } = terminator.kind { - self.record_killed_borrows_for_place(destination, location); - } - - self.super_terminator(terminator, location); - } - - fn visit_ascribe_user_ty( - &mut self, - _place: &Place<'tcx>, - _variance: ty::Variance, - _user_ty: &UserTypeProjection, - _location: Location, - ) { - } } impl<'cx, 'tcx> ConstraintGeneration<'cx, 'tcx> { - /// Some variable with type `live_ty` is "regular live" at - /// `location` -- i.e., it may be used later. This means that all - /// regions appearing in the type `live_ty` must be live at - /// `location`. - fn add_regular_live_constraint(&mut self, live_ty: T, location: Location) + /// 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, location: Location) where T: TypeVisitable>, { - debug!("add_regular_live_constraint(live_ty={:?}, location={:?})", live_ty, location); - - self.infcx.tcx.for_each_free_region(&live_ty, |live_region| { - let vid = live_region.as_var(); - self.liveness_constraints.add_location(vid, location); + debug!("add_regular_live_constraint(value={:?}, location={:?})", value, location); + self.tcx.for_each_free_region(&value, |live_region| { + let live_region_vid = live_region.as_var(); + self.liveness_constraints.add_location(live_region_vid, location); }); } - - /// When recording facts for Polonius, records the borrows on the specified place - /// as `killed`. For example, when assigning to a local, or on a call's return destination. - fn record_killed_borrows_for_place(&mut self, place: Place<'tcx>, location: Location) { - if let Some(all_facts) = self.all_facts { - let _prof_timer = self.infcx.tcx.prof.generic_activity("polonius_fact_generation"); - - // Depending on the `Place` we're killing: - // - if it's a local, or a single deref of a local, - // we kill all the borrows on the local. - // - if it's a deeper projection, we have to filter which - // of the borrows are killed: the ones whose `borrowed_place` - // conflicts with the `place`. - match place.as_ref() { - PlaceRef { local, projection: &[] } - | PlaceRef { local, projection: &[ProjectionElem::Deref] } => { - debug!( - "Recording `killed` facts for borrows of local={:?} at location={:?}", - local, location - ); - - record_killed_borrows_for_local( - all_facts, - self.borrow_set, - self.location_table, - local, - location, - ); - } - - PlaceRef { local, projection: &[.., _] } => { - // Kill conflicting borrows of the innermost local. - debug!( - "Recording `killed` facts for borrows of \ - innermost projected local={:?} at location={:?}", - local, location - ); - - if let Some(borrow_indices) = self.borrow_set.local_map.get(&local) { - for &borrow_index in borrow_indices { - let places_conflict = places_conflict::places_conflict( - self.infcx.tcx, - self.body, - self.borrow_set[borrow_index].borrowed_place, - place, - places_conflict::PlaceConflictBias::NoOverlap, - ); - - if places_conflict { - let location_index = self.location_table.mid_index(location); - all_facts.loan_killed_at.push((borrow_index, location_index)); - } - } - } - } - } - } - } -} - -/// When recording facts for Polonius, records the borrows on the specified local as `killed`. -fn record_killed_borrows_for_local( - all_facts: &mut AllFacts, - borrow_set: &BorrowSet<'_>, - location_table: &LocationTable, - local: Local, - location: Location, -) { - if let Some(borrow_indices) = borrow_set.local_map.get(&local) { - all_facts.loan_killed_at.reserve(borrow_indices.len()); - for &borrow_index in borrow_indices { - let location_index = location_table.mid_index(location); - all_facts.loan_killed_at.push((borrow_index, location_index)); - } - } } diff --git a/compiler/rustc_borrowck/src/nll.rs b/compiler/rustc_borrowck/src/nll.rs index 0c88f5cf363..156c2bf2936 100644 --- a/compiler/rustc_borrowck/src/nll.rs +++ b/compiler/rustc_borrowck/src/nll.rs @@ -149,14 +149,7 @@ pub(crate) fn compute_regions<'cx, 'tcx>( } = constraints; let placeholder_indices = Rc::new(placeholder_indices); - constraint_generation::generate_constraints( - infcx, - &mut liveness_constraints, - &mut all_facts, - location_table, - body, - borrow_set, - ); + constraint_generation::generate_constraints(infcx.tcx, &mut liveness_constraints, body); polonius::emit_cfg_and_loan_kills_facts( infcx.tcx, &mut all_facts, From 96042bc247cdef3469effad1ab307d4c354366d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Thu, 23 Nov 2023 15:20:12 +0000 Subject: [PATCH 12/13] merge NLL "constraint generation" into liveness --- .../src/constraint_generation.rs | 74 ---------------- compiler/rustc_borrowck/src/lib.rs | 1 - compiler/rustc_borrowck/src/nll.rs | 4 +- .../src/type_check/liveness/mod.rs | 84 ++++++++++++++++++- 4 files changed, 82 insertions(+), 81 deletions(-) delete mode 100644 compiler/rustc_borrowck/src/constraint_generation.rs diff --git a/compiler/rustc_borrowck/src/constraint_generation.rs b/compiler/rustc_borrowck/src/constraint_generation.rs deleted file mode 100644 index 445c58a2511..00000000000 --- a/compiler/rustc_borrowck/src/constraint_generation.rs +++ /dev/null @@ -1,74 +0,0 @@ -#![deny(rustc::untranslatable_diagnostic)] -#![deny(rustc::diagnostic_outside_of_impl)] -use rustc_middle::mir::visit::{TyContext, Visitor}; -use rustc_middle::mir::{Body, Location, SourceInfo}; -use rustc_middle::ty::visit::TypeVisitable; -use rustc_middle::ty::{GenericArgsRef, Region, Ty, TyCtxt}; - -use crate::region_infer::values::LivenessValues; - -pub(super) fn generate_constraints<'tcx>( - tcx: TyCtxt<'tcx>, - liveness_constraints: &mut LivenessValues, - body: &Body<'tcx>, -) { - let mut cg = ConstraintGeneration { tcx, liveness_constraints }; - for (bb, data) in body.basic_blocks.iter_enumerated() { - cg.visit_basic_block_data(bb, data); - } -} - -/// 'cg = the duration of the constraint generation process itself. -struct ConstraintGeneration<'cg, 'tcx> { - tcx: TyCtxt<'tcx>, - liveness_constraints: &'cg mut LivenessValues, -} - -impl<'cg, 'tcx> Visitor<'tcx> for ConstraintGeneration<'cg, '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: Location) { - self.record_regions_live_at(*args, location); - 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: Location) { - self.record_regions_live_at(region, location); - 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::UserTy(span) - | TyContext::LocalDecl { source_info: SourceInfo { span, .. }, .. } => { - span_bug!(span, "should not be visiting outside of the CFG: {:?}", ty_context); - } - TyContext::Location(location) => { - self.record_regions_live_at(ty, location); - } - } - - self.super_ty(ty); - } -} - -impl<'cx, 'tcx> ConstraintGeneration<'cx, '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, location: Location) - where - T: TypeVisitable>, - { - debug!("add_regular_live_constraint(value={:?}, location={:?})", value, location); - self.tcx.for_each_free_region(&value, |live_region| { - let live_region_vid = live_region.as_var(); - self.liveness_constraints.add_location(live_region_vid, location); - }); - } -} diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index d711a53565d..ff064636e2f 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -64,7 +64,6 @@ use self::path_utils::*; pub mod borrow_set; mod borrowck_errors; -mod constraint_generation; mod constraints; mod dataflow; mod def_use; diff --git a/compiler/rustc_borrowck/src/nll.rs b/compiler/rustc_borrowck/src/nll.rs index 156c2bf2936..10a8dbe2927 100644 --- a/compiler/rustc_borrowck/src/nll.rs +++ b/compiler/rustc_borrowck/src/nll.rs @@ -22,7 +22,6 @@ use std::str::FromStr; use crate::{ borrow_set::BorrowSet, - constraint_generation, consumers::ConsumerOptions, diagnostics::RegionErrors, facts::{AllFacts, AllFactsExt, RustcFacts}, @@ -141,7 +140,7 @@ pub(crate) fn compute_regions<'cx, 'tcx>( let MirTypeckRegionConstraints { placeholder_indices, placeholder_index_to_region: _, - mut liveness_constraints, + liveness_constraints, outlives_constraints, member_constraints, universe_causes, @@ -149,7 +148,6 @@ pub(crate) fn compute_regions<'cx, 'tcx>( } = constraints; let placeholder_indices = Rc::new(placeholder_indices); - constraint_generation::generate_constraints(infcx.tcx, &mut liveness_constraints, body); polonius::emit_cfg_and_loan_kills_facts( infcx.tcx, &mut all_facts, diff --git a/compiler/rustc_borrowck/src/type_check/liveness/mod.rs b/compiler/rustc_borrowck/src/type_check/liveness/mod.rs index a970dadc479..dc4695fd2b0 100644 --- a/compiler/rustc_borrowck/src/type_check/liveness/mod.rs +++ b/compiler/rustc_borrowck/src/type_check/liveness/mod.rs @@ -1,7 +1,9 @@ use itertools::{Either, Itertools}; use rustc_data_structures::fx::FxHashSet; -use rustc_middle::mir::{Body, Local}; -use rustc_middle::ty::{RegionVid, TyCtxt}; +use rustc_middle::mir::visit::{TyContext, Visitor}; +use rustc_middle::mir::{Body, Local, Location, SourceInfo}; +use rustc_middle::ty::visit::TypeVisitable; +use rustc_middle::ty::{GenericArgsRef, Region, RegionVid, Ty, TyCtxt}; use rustc_mir_dataflow::impls::MaybeInitializedPlaces; use rustc_mir_dataflow::move_paths::MoveData; use rustc_mir_dataflow::ResultsCursor; @@ -11,7 +13,7 @@ use crate::{ constraints::OutlivesConstraintSet, facts::{AllFacts, AllFactsExt}, location::LocationTable, - region_infer::values::RegionValueElements, + region_infer::values::{LivenessValues, RegionValueElements}, universal_regions::UniversalRegions, }; @@ -65,6 +67,14 @@ pub(super) fn generate<'mir, 'tcx>( boring_locals, polonius_drop_used, ); + + // 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.borrowck_context.constraints.liveness_constraints, + body, + ); } // The purpose of `compute_relevant_live_locals` is to define the subset of `Local` @@ -132,3 +142,71 @@ fn regions_that_outlive_free_regions<'tcx>( // Return the final set of things we visited. outlives_free_region } + +/// 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`. +fn record_regular_live_regions<'tcx>( + tcx: TyCtxt<'tcx>, + liveness_constraints: &mut LivenessValues, + body: &Body<'tcx>, +) { + let mut visitor = LiveVariablesVisitor { tcx, liveness_constraints }; + for (bb, data) in body.basic_blocks.iter_enumerated() { + visitor.visit_basic_block_data(bb, data); + } +} + +/// Visitor looking for regions that should be live within rvalues or calls. +struct LiveVariablesVisitor<'cx, 'tcx> { + tcx: TyCtxt<'tcx>, + liveness_constraints: &'cx mut LivenessValues, +} + +impl<'cx, 'tcx> Visitor<'tcx> for LiveVariablesVisitor<'cx, '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: Location) { + self.record_regions_live_at(*args, location); + 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: Location) { + self.record_regions_live_at(region, location); + 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::UserTy(span) + | TyContext::LocalDecl { source_info: SourceInfo { span, .. }, .. } => { + span_bug!(span, "should not be visiting outside of the CFG: {:?}", ty_context); + } + TyContext::Location(location) => { + self.record_regions_live_at(ty, location); + } + } + + self.super_ty(ty); + } +} + +impl<'cx, 'tcx> LiveVariablesVisitor<'cx, '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, location: Location) + where + T: TypeVisitable>, + { + debug!("record_regions_live_at(value={:?}, location={:?})", value, location); + self.tcx.for_each_free_region(&value, |live_region| { + let live_region_vid = live_region.as_var(); + self.liveness_constraints.add_location(live_region_vid, location); + }); + } +} From f969af2195f38b711faad7b2b2cabffd162b5ec6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Thu, 23 Nov 2023 16:14:25 +0000 Subject: [PATCH 13/13] move remaining legacy polonius fact generation out of NLL module --- compiler/rustc_borrowck/src/nll.rs | 28 ++-------- .../src/polonius/loan_invalidations.rs | 1 - .../rustc_borrowck/src/polonius/loan_kills.rs | 1 - compiler/rustc_borrowck/src/polonius/mod.rs | 56 +++++++++++++------ 4 files changed, 46 insertions(+), 40 deletions(-) diff --git a/compiler/rustc_borrowck/src/nll.rs b/compiler/rustc_borrowck/src/nll.rs index 10a8dbe2927..510f1bcd46c 100644 --- a/compiler/rustc_borrowck/src/nll.rs +++ b/compiler/rustc_borrowck/src/nll.rs @@ -122,17 +122,6 @@ pub(crate) fn compute_regions<'cx, 'tcx>( polonius_input, ); - if let Some(all_facts) = &mut all_facts { - let _prof_timer = infcx.tcx.prof.generic_activity("polonius_fact_generation"); - polonius::emit_move_facts(all_facts, move_data, location_table, body); - polonius::emit_universal_region_facts( - all_facts, - borrow_set, - &universal_regions, - &universal_region_relations, - ); - } - // Create the region inference context, taking ownership of the // region inference data that was contained in `infcx`, and the // base constraints generated by the type-check. @@ -148,12 +137,16 @@ pub(crate) fn compute_regions<'cx, 'tcx>( } = constraints; let placeholder_indices = Rc::new(placeholder_indices); - polonius::emit_cfg_and_loan_kills_facts( - infcx.tcx, + // If requested, emit legacy polonius facts. + polonius::emit_facts( &mut all_facts, + infcx.tcx, location_table, body, borrow_set, + move_data, + &universal_regions, + &universal_region_relations, ); let mut regioncx = RegionInferenceContext::new( @@ -171,15 +164,6 @@ pub(crate) fn compute_regions<'cx, 'tcx>( live_loans, ); - // Generate various additional constraints. - polonius::emit_loan_invalidations_facts( - infcx.tcx, - &mut all_facts, - location_table, - body, - borrow_set, - ); - // If requested: dump NLL facts, and run legacy polonius analysis. let polonius_output = all_facts.as_ref().and_then(|all_facts| { if infcx.tcx.sess.opts.unstable_opts.nll_facts { diff --git a/compiler/rustc_borrowck/src/polonius/loan_invalidations.rs b/compiler/rustc_borrowck/src/polonius/loan_invalidations.rs index 23bf8457816..232bd741825 100644 --- a/compiler/rustc_borrowck/src/polonius/loan_invalidations.rs +++ b/compiler/rustc_borrowck/src/polonius/loan_invalidations.rs @@ -22,7 +22,6 @@ pub(super) fn emit_loan_invalidations<'tcx>( body: &Body<'tcx>, borrow_set: &BorrowSet<'tcx>, ) { - let _prof_timer = tcx.prof.generic_activity("polonius_fact_generation"); let dominators = body.basic_blocks.dominators(); let mut visitor = LoanInvalidationsGenerator { all_facts, borrow_set, tcx, location_table, body, dominators }; diff --git a/compiler/rustc_borrowck/src/polonius/loan_kills.rs b/compiler/rustc_borrowck/src/polonius/loan_kills.rs index cc836115c31..5df94383702 100644 --- a/compiler/rustc_borrowck/src/polonius/loan_kills.rs +++ b/compiler/rustc_borrowck/src/polonius/loan_kills.rs @@ -17,7 +17,6 @@ pub(super) fn emit_loan_kills<'tcx>( body: &Body<'tcx>, borrow_set: &BorrowSet<'tcx>, ) { - let _prof_timer = tcx.prof.generic_activity("polonius_fact_generation"); let mut visitor = LoanKillsGenerator { borrow_set, tcx, location_table, all_facts, body }; for (bb, data) in body.basic_blocks.iter_enumerated() { visitor.visit_basic_block_data(bb, data); diff --git a/compiler/rustc_borrowck/src/polonius/mod.rs b/compiler/rustc_borrowck/src/polonius/mod.rs index 9454118d9c4..40126d50d57 100644 --- a/compiler/rustc_borrowck/src/polonius/mod.rs +++ b/compiler/rustc_borrowck/src/polonius/mod.rs @@ -16,8 +16,42 @@ use crate::universal_regions::UniversalRegions; mod loan_invalidations; mod loan_kills; +/// When requested, emit most of the facts needed by polonius: +/// - moves and assignments +/// - universal regions and their relations +/// - CFG points and edges +/// - loan kills +/// - loan invalidations +/// +/// The rest of the facts are emitted during typeck and liveness. +pub(crate) fn emit_facts<'tcx>( + all_facts: &mut Option, + tcx: TyCtxt<'tcx>, + location_table: &LocationTable, + body: &Body<'tcx>, + borrow_set: &BorrowSet<'tcx>, + move_data: &MoveData<'_>, + universal_regions: &UniversalRegions<'_>, + universal_region_relations: &UniversalRegionRelations<'_>, +) { + let Some(all_facts) = all_facts else { + // We don't do anything if there are no facts to fill. + return; + }; + let _prof_timer = tcx.prof.generic_activity("polonius_fact_generation"); + emit_move_facts(all_facts, move_data, location_table, body); + emit_universal_region_facts( + all_facts, + borrow_set, + &universal_regions, + &universal_region_relations, + ); + emit_cfg_and_loan_kills_facts(all_facts, tcx, location_table, body, borrow_set); + emit_loan_invalidations_facts(all_facts, tcx, location_table, body, borrow_set); +} + /// Emit facts needed for move/init analysis: moves and assignments. -pub(crate) fn emit_move_facts( +fn emit_move_facts( all_facts: &mut AllFacts, move_data: &MoveData<'_>, location_table: &LocationTable, @@ -91,7 +125,7 @@ pub(crate) fn emit_move_facts( } /// Emit universal regions facts, and their relations. -pub(crate) fn emit_universal_region_facts( +fn emit_universal_region_facts( all_facts: &mut AllFacts, borrow_set: &BorrowSet<'_>, universal_regions: &UniversalRegions<'_>, @@ -132,33 +166,23 @@ pub(crate) fn emit_universal_region_facts( } /// Emit facts about loan invalidations. -pub(crate) fn emit_loan_invalidations_facts<'tcx>( +fn emit_loan_invalidations_facts<'tcx>( + all_facts: &mut AllFacts, tcx: TyCtxt<'tcx>, - all_facts: &mut Option, location_table: &LocationTable, body: &Body<'tcx>, borrow_set: &BorrowSet<'tcx>, ) { - let Some(all_facts) = all_facts else { - // Nothing to do if we don't have any facts to fill - return; - }; - loan_invalidations::emit_loan_invalidations(tcx, all_facts, location_table, body, borrow_set); } /// Emit facts about CFG points and edges, as well as locations where loans are killed. -pub(crate) fn emit_cfg_and_loan_kills_facts<'tcx>( +fn emit_cfg_and_loan_kills_facts<'tcx>( + all_facts: &mut AllFacts, tcx: TyCtxt<'tcx>, - all_facts: &mut Option, location_table: &LocationTable, body: &Body<'tcx>, borrow_set: &BorrowSet<'tcx>, ) { - let Some(all_facts) = all_facts else { - // Nothing to do if we don't have any facts to fill - return; - }; - loan_kills::emit_loan_kills(tcx, all_facts, location_table, body, borrow_set); }