From aeb3d103a6913115996d0ddb9d550016ea403488 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Mon, 16 Dec 2024 11:08:00 +0000 Subject: [PATCH] address review comments - move constraints to an Option - check `-Zpolonius=next` only once - rewrite fixme comments to make the actionable part clear --- compiler/rustc_borrowck/src/lib.rs | 2 +- compiler/rustc_borrowck/src/nll.rs | 17 +++++------ compiler/rustc_borrowck/src/polonius/dump.rs | 7 +++-- compiler/rustc_borrowck/src/polonius/mod.rs | 30 +++++++++----------- 4 files changed, 27 insertions(+), 29 deletions(-) diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index 5c8e12a1c69ef..e6dfe4e0ce212 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -322,7 +322,7 @@ fn do_mir_borrowck<'tcx>( body, ®ioncx, &borrow_set, - &localized_outlives_constraints, + localized_outlives_constraints, &opt_closure_req, ); diff --git a/compiler/rustc_borrowck/src/nll.rs b/compiler/rustc_borrowck/src/nll.rs index ec0dbf6aa77b5..abe27555b1864 100644 --- a/compiler/rustc_borrowck/src/nll.rs +++ b/compiler/rustc_borrowck/src/nll.rs @@ -48,7 +48,7 @@ pub(crate) struct NllOutput<'tcx> { pub nll_errors: RegionErrors<'tcx>, /// When using `-Zpolonius=next`: the localized typeck and liveness constraints. - pub localized_outlives_constraints: LocalizedOutlivesConstraintSet, + pub localized_outlives_constraints: Option, } /// Rewrites the regions in the MIR to use NLL variables, also scraping out the set of universal @@ -141,15 +141,12 @@ pub(crate) fn compute_regions<'a, 'tcx>( // If requested for `-Zpolonius=next`, convert NLL constraints to localized outlives // constraints. - let mut localized_outlives_constraints = LocalizedOutlivesConstraintSet::default(); - if infcx.tcx.sess.opts.unstable_opts.polonius.is_next_enabled() { - polonius::create_localized_constraints( - &mut regioncx, - infcx.infcx.tcx, - body, - &mut localized_outlives_constraints, - ); - } + let localized_outlives_constraints = + if infcx.tcx.sess.opts.unstable_opts.polonius.is_next_enabled() { + Some(polonius::create_localized_constraints(&mut regioncx, body)) + } else { + None + }; // If requested: dump NLL facts, and run legacy polonius analysis. let polonius_output = all_facts.as_ref().and_then(|all_facts| { diff --git a/compiler/rustc_borrowck/src/polonius/dump.rs b/compiler/rustc_borrowck/src/polonius/dump.rs index 607e3d92bb1a9..a6d8014903440 100644 --- a/compiler/rustc_borrowck/src/polonius/dump.rs +++ b/compiler/rustc_borrowck/src/polonius/dump.rs @@ -18,7 +18,7 @@ pub(crate) fn dump_polonius_mir<'tcx>( body: &Body<'tcx>, regioncx: &RegionInferenceContext<'tcx>, borrow_set: &BorrowSet<'tcx>, - localized_outlives_constraints: &LocalizedOutlivesConstraintSet, + localized_outlives_constraints: Option, closure_region_requirements: &Option>, ) { let tcx = infcx.tcx; @@ -26,6 +26,9 @@ pub(crate) fn dump_polonius_mir<'tcx>( return; } + let localized_outlives_constraints = localized_outlives_constraints + .expect("missing localized constraints with `-Zpolonius=next`"); + // We want the NLL extra comments printed by default in NLL MIR dumps (they were removed in // #112346). Specifying `-Z mir-include-spans` on the CLI still has priority: for example, // they're always disabled in mir-opt tests to make working with blessed dumps easier. @@ -48,7 +51,7 @@ pub(crate) fn dump_polonius_mir<'tcx>( regioncx, closure_region_requirements, borrow_set, - localized_outlives_constraints, + &localized_outlives_constraints, pass_where, out, ) diff --git a/compiler/rustc_borrowck/src/polonius/mod.rs b/compiler/rustc_borrowck/src/polonius/mod.rs index fa7e08d9db345..eee5e70efe348 100644 --- a/compiler/rustc_borrowck/src/polonius/mod.rs +++ b/compiler/rustc_borrowck/src/polonius/mod.rs @@ -40,7 +40,6 @@ pub(crate) use dump::dump_polonius_mir; pub(crate) mod legacy; use rustc_middle::mir::{Body, Location}; -use rustc_middle::ty::TyCtxt; use rustc_mir_dataflow::points::PointIndex; use crate::RegionInferenceContext; @@ -49,34 +48,31 @@ use crate::region_infer::values::LivenessValues; use crate::type_check::Locations; use crate::universal_regions::UniversalRegions; -/// When using `-Zpolonius=next`, fills the given constraint set by: +/// Creates a constraint set for `-Zpolonius=next` by: /// - converting NLL typeck constraints to be localized /// - encoding liveness constraints pub(crate) fn create_localized_constraints<'tcx>( regioncx: &mut RegionInferenceContext<'tcx>, - tcx: TyCtxt<'tcx>, body: &Body<'tcx>, - localized_outlives_constraints: &mut LocalizedOutlivesConstraintSet, -) { - if !tcx.sess.opts.unstable_opts.polonius.is_next_enabled() { - return; - } - +) -> LocalizedOutlivesConstraintSet { + let mut localized_outlives_constraints = LocalizedOutlivesConstraintSet::default(); convert_typeck_constraints( body, regioncx.liveness_constraints(), regioncx.outlives_constraints(), - localized_outlives_constraints, + &mut localized_outlives_constraints, ); create_liveness_constraints( body, regioncx.liveness_constraints(), regioncx.universal_regions(), - localized_outlives_constraints, + &mut localized_outlives_constraints, ); // FIXME: here, we can trace loan reachability in the constraint graph and record this as loan // liveness for the next step in the chain, the NLL loan scope and active loans computations. + + localized_outlives_constraints } /// Propagate loans throughout the subset graph at a given point (with some subtleties around the @@ -90,8 +86,9 @@ fn convert_typeck_constraints<'tcx>( for outlives_constraint in outlives_constraints { match outlives_constraint.locations { Locations::All(_) => { - // FIXME: for now, turn logical constraints holding at all points into physical - // edges at every point in the graph. We can encode this into *traversal* instead. + // For now, turn logical constraints holding at all points into physical edges at + // every point in the graph. + // FIXME: encode this into *traversal* instead. for (block, bb) in body.basic_blocks.iter_enumerated() { let statement_count = bb.statements.len(); for statement_index in 0..=statement_count { @@ -168,9 +165,10 @@ fn propagate_loans_between_points( localized_outlives_constraints: &mut LocalizedOutlivesConstraintSet, ) { // Universal regions are semantically live at all points. - // FIXME: We always have universal regions but they're not always (or often) involved in the - // subset graph. So for now, we emit this edge, but we only need to emit edges for universal - // regions that existential regions can actually reach. + // Note: we always have universal regions but they're not always (or often) involved in the + // subset graph. For now, we emit all their edges unconditionally, but some of these subgraphs + // will be disconnected from the rest of the graph and thus, unnecessary. + // FIXME: only emit the edges of universal regions that existential regions can reach. for region in universal_regions.universal_regions_iter() { localized_outlives_constraints.push(LocalizedOutlivesConstraint { source: region,