Rollup merge of #136299 - lqd:polonius-next-episode-9, r=jackh726

Ignore NLL boring locals in polonius diagnostics

Another easy one ``@jackh726`` (the diff is inflated by blessed test expectations don't worry :)

NLLs don't compute liveness for boring locals, and therefore cannot find them in causes explaining borrows. In polonius, we don't have this liveness optimization (we may be able to do something partially similar in the future, e.g. for function parameters and the like), so we do encounter these in diagnostics even though we don't want to. This PR:
- restructures the polonius context into per-phase data, in spirit as you requested in an earlier review
- stores the locals NLLs would consider boring into the errors/diagnostics data
- ignores these if a boring local is found when trying to explain borrows

This PR fixes around 80 cases of diagnostics differences between `-Zpolonius=next` and NLLs. I've also added explicit revisions to a few polonius tests (both for the in-tree implementation as well as the datalog implementation -- even if we'll eventually remove them). I didn't do this for all the "dead" expectations that were removed from #136112 for that same reason, it's fine. I'll soon/eventually add explicit revisions where they're needed: there's only a handful of tests left to fix.

r? ``@jackh726``

Author: matthiaskrgr

compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs

@@ -622,8 +622,25 @@ impl<'tcx> MirBorrowckCtxt<'_, '_, 'tcx> {
             }
         }
 
+        // NLL doesn't consider boring locals for liveness, and wouldn't encounter a
+        // `Cause::LiveVar` for such a local. Polonius can't avoid computing liveness for boring
+        // locals yet, and will encounter them when trying to explain why a borrow contains a given
+        // point.
+        //
+        // We want to focus on relevant live locals in diagnostics, so when polonius is enabled, we
+        // ensure that we don't emit live boring locals as explanations.
+        let is_local_boring = |local| {
+            if let Some(polonius_diagnostics) = self.polonius_diagnostics {
+                polonius_diagnostics.boring_nll_locals.contains(&local)
+            } else {
+                assert!(!tcx.sess.opts.unstable_opts.polonius.is_next_enabled());
+
+                // Boring locals are never the cause of a borrow explanation in NLLs.
+                false
+            }
+        };
         match find_use::find(body, regioncx, tcx, region_sub, use_location) {
-            Some(Cause::LiveVar(local, location)) => {
+            Some(Cause::LiveVar(local, location)) if !is_local_boring(local) => {
                 let span = body.source_info(location).span;
                 let spans = self
                     .move_spans(Place::from(local).as_ref(), location)
@@ -666,7 +683,9 @@ impl<'tcx> MirBorrowckCtxt<'_, '_, 'tcx> {
                 }
             }
 
-            None => {
+            Some(Cause::LiveVar(..)) | None => {
+                // Here, under NLL: no cause was found. Under polonius: no cause was found, or a
+                // boring local was found, which we ignore like NLLs do to match its diagnostics.
                 if let Some(region) = self.to_error_region_vid(borrow_region_vid) {
                     let (category, from_closure, span, region_name, path) =
                         self.free_region_constraint_info(borrow_region_vid, region);

compiler/rustc_borrowck/src/lib.rs

@@ -60,6 +60,7 @@ use crate::diagnostics::{
 use crate::path_utils::*;
 use crate::place_ext::PlaceExt;
 use crate::places_conflict::{PlaceConflictBias, places_conflict};
+use crate::polonius::PoloniusDiagnosticsContext;
 use crate::polonius::legacy::{PoloniusLocationTable, PoloniusOutput};
 use crate::prefixes::PrefixSet;
 use crate::region_infer::RegionInferenceContext;
@@ -198,7 +199,7 @@ fn do_mir_borrowck<'tcx>(
         polonius_output,
         opt_closure_req,
         nll_errors,
-        localized_outlives_constraints,
+        polonius_diagnostics,
     } = nll::compute_regions(
         &infcx,
         free_regions,
@@ -270,6 +271,7 @@ fn do_mir_borrowck<'tcx>(
             polonius_output: None,
             move_errors: Vec::new(),
             diags_buffer,
+            polonius_diagnostics: polonius_diagnostics.as_ref(),
         };
         struct MoveVisitor<'a, 'b, 'infcx, 'tcx> {
             ctxt: &'a mut MirBorrowckCtxt<'b, 'infcx, 'tcx>,
@@ -308,6 +310,7 @@ fn do_mir_borrowck<'tcx>(
         polonius_output,
         move_errors: Vec::new(),
         diags_buffer,
+        polonius_diagnostics: polonius_diagnostics.as_ref(),
     };
 
     // Compute and report region errors, if any.
@@ -329,7 +332,7 @@ fn do_mir_borrowck<'tcx>(
         body,
         &regioncx,
         &borrow_set,
-        localized_outlives_constraints,
+        polonius_diagnostics.as_ref(),
         &opt_closure_req,
     );
 
@@ -579,6 +582,9 @@ struct MirBorrowckCtxt<'a, 'infcx, 'tcx> {
 
     diags_buffer: &'a mut BorrowckDiagnosticsBuffer<'infcx, 'tcx>,
     move_errors: Vec<MoveError<'tcx>>,
+
+    /// When using `-Zpolonius=next`: the data used to compute errors and diagnostics.
+    polonius_diagnostics: Option<&'a PoloniusDiagnosticsContext>,
 }
 
 // Check that:

compiler/rustc_borrowck/src/nll.rs

@@ -27,7 +27,7 @@ use tracing::{debug, instrument};
 use crate::borrow_set::BorrowSet;
 use crate::consumers::ConsumerOptions;
 use crate::diagnostics::{BorrowckDiagnosticsBuffer, RegionErrors};
-use crate::polonius::LocalizedOutlivesConstraintSet;
+use crate::polonius::PoloniusDiagnosticsContext;
 use crate::polonius::legacy::{
     PoloniusFacts, PoloniusFactsExt, PoloniusLocationTable, PoloniusOutput,
 };
@@ -46,8 +46,9 @@ pub(crate) struct NllOutput<'tcx> {
     pub opt_closure_req: Option<ClosureRegionRequirements<'tcx>>,
     pub nll_errors: RegionErrors<'tcx>,
 
-    /// When using `-Zpolonius=next`: the localized typeck and liveness constraints.
-    pub localized_outlives_constraints: Option<LocalizedOutlivesConstraintSet>,
+    /// When using `-Zpolonius=next`: the data used to compute errors and diagnostics, e.g.
+    /// localized typeck and liveness constraints.
+    pub polonius_diagnostics: Option<PoloniusDiagnosticsContext>,
 }
 
 /// Rewrites the regions in the MIR to use NLL variables, also scraping out the set of universal
@@ -144,7 +145,7 @@ pub(crate) fn compute_regions<'a, 'tcx>(
 
     // If requested for `-Zpolonius=next`, convert NLL constraints to localized outlives constraints
     // and use them to compute loan liveness.
-    let localized_outlives_constraints = polonius_context.as_ref().map(|polonius_context| {
+    let polonius_diagnostics = polonius_context.map(|polonius_context| {
         polonius_context.compute_loan_liveness(infcx.tcx, &mut regioncx, body, borrow_set)
     });
 
@@ -188,7 +189,7 @@ pub(crate) fn compute_regions<'a, 'tcx>(
         polonius_output,
         opt_closure_req: closure_region_requirements,
         nll_errors,
-        localized_outlives_constraints,
+        polonius_diagnostics,
     }
 }
 

compiler/rustc_borrowck/src/polonius/dump.rs

@@ -12,7 +12,9 @@ use rustc_session::config::MirIncludeSpans;
 
 use crate::borrow_set::BorrowSet;
 use crate::constraints::OutlivesConstraint;
-use crate::polonius::{LocalizedOutlivesConstraint, LocalizedOutlivesConstraintSet};
+use crate::polonius::{
+    LocalizedOutlivesConstraint, LocalizedOutlivesConstraintSet, PoloniusDiagnosticsContext,
+};
 use crate::region_infer::values::LivenessValues;
 use crate::type_check::Locations;
 use crate::{BorrowckInferCtxt, RegionInferenceContext};
@@ -23,7 +25,7 @@ pub(crate) fn dump_polonius_mir<'tcx>(
     body: &Body<'tcx>,
     regioncx: &RegionInferenceContext<'tcx>,
     borrow_set: &BorrowSet<'tcx>,
-    localized_outlives_constraints: Option<LocalizedOutlivesConstraintSet>,
+    polonius_diagnostics: Option<&PoloniusDiagnosticsContext>,
     closure_region_requirements: &Option<ClosureRegionRequirements<'tcx>>,
 ) {
     let tcx = infcx.tcx;
@@ -35,8 +37,8 @@ pub(crate) fn dump_polonius_mir<'tcx>(
         return;
     }
 
-    let localized_outlives_constraints = localized_outlives_constraints
-        .expect("missing localized constraints with `-Zpolonius=next`");
+    let polonius_diagnostics =
+        polonius_diagnostics.expect("missing diagnostics context with `-Zpolonius=next`");
 
     let _: io::Result<()> = try {
         let mut file = create_dump_file(tcx, "html", false, "polonius", &0, body)?;
@@ -45,7 +47,7 @@ pub(crate) fn dump_polonius_mir<'tcx>(
             body,
             regioncx,
             borrow_set,
-            localized_outlives_constraints,
+            &polonius_diagnostics.localized_outlives_constraints,
             closure_region_requirements,
             &mut file,
         )?;
@@ -63,7 +65,7 @@ fn emit_polonius_dump<'tcx>(
     body: &Body<'tcx>,
     regioncx: &RegionInferenceContext<'tcx>,
     borrow_set: &BorrowSet<'tcx>,
-    localized_outlives_constraints: LocalizedOutlivesConstraintSet,
+    localized_outlives_constraints: &LocalizedOutlivesConstraintSet,
     closure_region_requirements: &Option<ClosureRegionRequirements<'tcx>>,
     out: &mut dyn io::Write,
 ) -> io::Result<()> {

compiler/rustc_borrowck/src/polonius/liveness_constraints.rs

@@ -1,20 +1,19 @@
 use std::collections::BTreeMap;
 
 use rustc_index::bit_set::SparseBitMatrix;
-use rustc_index::interval::SparseIntervalMatrix;
 use rustc_middle::mir::{Body, Location};
 use rustc_middle::ty::relate::{self, Relate, RelateResult, TypeRelation};
 use rustc_middle::ty::{self, RegionVid, Ty, TyCtxt, TypeVisitable};
 use rustc_mir_dataflow::points::PointIndex;
 
 use super::{
     ConstraintDirection, LocalizedOutlivesConstraint, LocalizedOutlivesConstraintSet,
-    PoloniusContext,
+    PoloniusLivenessContext,
 };
 use crate::region_infer::values::LivenessValues;
 use crate::universal_regions::UniversalRegions;
 
-impl PoloniusContext {
+impl PoloniusLivenessContext {
     /// Record the variance of each region contained within the given value.
     pub(crate) fn record_live_region_variance<'tcx>(
         &mut self,
@@ -30,25 +29,6 @@ impl PoloniusContext {
         };
         extractor.relate(value, value).expect("Can't have a type error relating to itself");
     }
-
-    /// Unlike NLLs, in polonius we traverse the cfg to look for regions live across an edge, so we
-    /// need to transpose the "points where each region is live" matrix to a "live regions per point"
-    /// matrix.
-    // FIXME: avoid this conversion by always storing liveness data in this shape in the rest of
-    // borrowck.
-    pub(crate) fn record_live_regions_per_point(
-        &mut self,
-        num_regions: usize,
-        points_per_live_region: &SparseIntervalMatrix<RegionVid, PointIndex>,
-    ) {
-        let mut live_regions_per_point = SparseBitMatrix::new(num_regions);
-        for region in points_per_live_region.rows() {
-            for point in points_per_live_region.row(region).unwrap().iter() {
-                live_regions_per_point.insert(point, region);
-            }
-        }
-        self.live_regions = Some(live_regions_per_point);
-    }
 }
 
 /// Propagate loans throughout the CFG: for each statement in the MIR, create localized outlives

compiler/rustc_borrowck/src/polonius/mod.rs

@@ -32,6 +32,16 @@
 //! - <https://smallcultfollowing.com/babysteps/blog/2023/09/22/polonius-part-1/>
 //! - <https://smallcultfollowing.com/babysteps/blog/2023/09/29/polonius-part-2/>
 //!
+//!
+//! Data flows like this:
+//! 1) during MIR typeck, record liveness data needed later: live region variances, as well as the
+//!    usual NLL liveness data (just computed on more locals). That's the [PoloniusLivenessContext].
+//! 2) once that is done, variance data is transferred, and the NLL region liveness is converted to
+//!    the polonius shape. That's the main [PoloniusContext].
+//! 3) during region inference, that data and the NLL outlives constraints are used to create the
+//!    localized outlives constraints, as described above. That's the [PoloniusDiagnosticsContext].
+//! 4) transfer this back to the main borrowck procedure: it handles computing errors and
+//!    diagnostics, debugging and MIR dumping concerns.
 
 mod constraints;
 mod dump;
@@ -42,8 +52,10 @@ mod typeck_constraints;
 
 use std::collections::BTreeMap;
 
+use rustc_data_structures::fx::FxHashSet;
 use rustc_index::bit_set::SparseBitMatrix;
-use rustc_middle::mir::Body;
+use rustc_index::interval::SparseIntervalMatrix;
+use rustc_middle::mir::{Body, Local};
 use rustc_middle::ty::{RegionVid, TyCtxt};
 use rustc_mir_dataflow::points::PointIndex;
 
@@ -57,15 +69,40 @@ use crate::{BorrowSet, RegionInferenceContext};
 
 pub(crate) type LiveLoans = SparseBitMatrix<PointIndex, BorrowIndex>;
 
-/// This struct holds the data needed to create the Polonius localized constraints.
+/// This struct holds the liveness data created during MIR typeck, and which will be used later in
+/// the process, to compute the polonius localized constraints.
+#[derive(Default)]
+pub(crate) struct PoloniusLivenessContext {
+    /// The expected edge direction per live region: the kind of directed edge we'll create as
+    /// liveness constraints depends on the variance of types with respect to each contained region.
+    live_region_variances: BTreeMap<RegionVid, ConstraintDirection>,
+
+    /// The regions that outlive free regions are used to distinguish relevant live locals from
+    /// boring locals. A boring local is one whose type contains only such regions. Polonius
+    /// currently has more boring locals than NLLs so we record the latter to use in errors and
+    /// diagnostics, to focus on the locals we consider relevant and match NLL diagnostics.
+    pub(crate) boring_nll_locals: FxHashSet<Local>,
+}
+
+/// This struct holds the data needed to create the Polonius localized constraints. Its data is
+/// transferred and converted from the [PoloniusLivenessContext] at the end of MIR typeck.
 pub(crate) struct PoloniusContext {
+    /// The liveness data we recorded during MIR typeck.
+    liveness_context: PoloniusLivenessContext,
+
     /// The set of regions that are live at a given point in the CFG, used to create localized
     /// outlives constraints between regions that are live at connected points in the CFG.
-    live_regions: Option<SparseBitMatrix<PointIndex, RegionVid>>,
+    live_regions: SparseBitMatrix<PointIndex, RegionVid>,
+}
 
-    /// The expected edge direction per live region: the kind of directed edge we'll create as
-    /// liveness constraints depends on the variance of types with respect to each contained region.
-    live_region_variances: BTreeMap<RegionVid, ConstraintDirection>,
+/// This struct holds the data needed by the borrowck error computation and diagnostics. Its data is
+/// computed from the [PoloniusContext] when computing NLL regions.
+pub(crate) struct PoloniusDiagnosticsContext {
+    /// The localized outlives constraints that were computed in the main analysis.
+    localized_outlives_constraints: LocalizedOutlivesConstraintSet,
+
+    /// The liveness data computed during MIR typeck: [PoloniusLivenessContext::boring_nll_locals].
+    pub(crate) boring_nll_locals: FxHashSet<Local>,
 }
 
 /// The direction a constraint can flow into. Used to create liveness constraints according to
@@ -83,8 +120,24 @@ enum ConstraintDirection {
 }
 
 impl PoloniusContext {
-    pub(crate) fn new() -> PoloniusContext {
-        Self { live_region_variances: BTreeMap::new(), live_regions: None }
+    /// Unlike NLLs, in polonius we traverse the cfg to look for regions live across an edge, so we
+    /// need to transpose the "points where each region is live" matrix to a "live regions per point"
+    /// matrix.
+    // FIXME: avoid this conversion by always storing liveness data in this shape in the rest of
+    // borrowck.
+    pub(crate) fn create_from_liveness(
+        liveness_context: PoloniusLivenessContext,
+        num_regions: usize,
+        points_per_live_region: &SparseIntervalMatrix<RegionVid, PointIndex>,
+    ) -> PoloniusContext {
+        let mut live_regions_per_point = SparseBitMatrix::new(num_regions);
+        for region in points_per_live_region.rows() {
+            for point in points_per_live_region.row(region).unwrap().iter() {
+                live_regions_per_point.insert(point, region);
+            }
+        }
+
+        PoloniusContext { live_regions: live_regions_per_point, liveness_context }
     }
 
     /// Computes live loans using the set of loans model for `-Zpolonius=next`.
@@ -95,13 +148,18 @@ impl PoloniusContext {
     ///
     /// Then, this graph is traversed, and combined with kills, reachability is recorded as loan
     /// liveness, to be used by the loan scope and active loans computations.
+    ///
+    /// The constraint data will be used to compute errors and diagnostics.
     pub(crate) fn compute_loan_liveness<'tcx>(
-        &self,
+        self,
         tcx: TyCtxt<'tcx>,
         regioncx: &mut RegionInferenceContext<'tcx>,
         body: &Body<'tcx>,
         borrow_set: &BorrowSet<'tcx>,
-    ) -> LocalizedOutlivesConstraintSet {
+    ) -> PoloniusDiagnosticsContext {
+        let PoloniusLivenessContext { live_region_variances, boring_nll_locals } =
+            self.liveness_context;
+
         let mut localized_outlives_constraints = LocalizedOutlivesConstraintSet::default();
         convert_typeck_constraints(
             tcx,
@@ -112,14 +170,11 @@ impl PoloniusContext {
             &mut localized_outlives_constraints,
         );
 
-        let live_regions = self.live_regions.as_ref().expect(
-            "live regions per-point data should have been created at the end of MIR typeck",
-        );
         create_liveness_constraints(
             body,
             regioncx.liveness_constraints(),
-            live_regions,
-            &self.live_region_variances,
+            &self.live_regions,
+            &live_region_variances,
             regioncx.universal_regions(),
             &mut localized_outlives_constraints,
         );
@@ -136,6 +191,6 @@ impl PoloniusContext {
         );
         regioncx.record_live_loans(live_loans);
 
-        localized_outlives_constraints
+        PoloniusDiagnosticsContext { localized_outlives_constraints, boring_nll_locals }
     }
 }