Move handling of placeholders out of the borrow checker

This commit moves all handling of placeholders, including
those in type-tests, into a pre-processing step of the borrowcheck.

Along the way it extends the SCC metadata tracking a lot
and uses the extra information to add p: 'static for
any placeholder that reaches another placeholder. It also
rewrites type-tests that have the same behaviour.

Error reporting is handled by introducing a new constraint kind
that remembers which original outlives constraint caused
an outlives-static. These work a lot like a HTTP redirect response
during constraint search.

`RegionInferenceContext` is now a lot simpler and only tracks
opaque regions without caring about their origin.

It also inlines the few measly bits of `init_free_and_bound_regions()`
that still remain as relevant. This increases the constructor for
`RegionInferenceContext`s somewhat, but I still think it's
readable.

The documentation for `init_free_and_bound_regions()` was out of
date, and the correct, up to date version is available in the various
places where the logic was moved.

As a drive-by it also refactors the blame search somewhat, renames
a few methods, and allows iterating over outgoing constraints
without the implied edges from 'static.

Author: amandasystems

compiler/rustc_borrowck/src/constraints/mod.rs

@@ -1,15 +1,14 @@
 use std::fmt;
 use std::ops::Index;
 
+use rustc_data_structures::graph::scc;
 use rustc_index::{IndexSlice, IndexVec};
 use rustc_middle::mir::ConstraintCategory;
 use rustc_middle::ty::{RegionVid, TyCtxt, VarianceDiagInfo};
 use rustc_span::Span;
-use tracing::{debug, instrument};
+use tracing::debug;
 
-use crate::region_infer::{ConstraintSccs, RegionDefinition, RegionTracker};
 use crate::type_check::Locations;
-use crate::universal_regions::UniversalRegions;
 
 pub(crate) mod graph;
 
@@ -57,105 +56,18 @@ impl<'tcx> OutlivesConstraintSet<'tcx> {
     /// Computes cycles (SCCs) in the graph of regions. In particular,
     /// find all regions R1, R2 such that R1: R2 and R2: R1 and group
     /// them into an SCC, and find the relationships between SCCs.
-    pub(crate) fn compute_sccs(
+    pub(crate) fn compute_sccs<
+        A: scc::Annotation,
+        AA: scc::Annotations<RegionVid, ConstraintSccIndex, A>,
+    >(
         &self,
         static_region: RegionVid,
-        definitions: &IndexVec<RegionVid, RegionDefinition<'tcx>>,
-    ) -> ConstraintSccs {
-        let constraint_graph = self.graph(definitions.len());
+        num_region_vars: usize,
+        annotations: &mut AA,
+    ) -> scc::Sccs<RegionVid, ConstraintSccIndex> {
+        let constraint_graph = self.graph(num_region_vars);
         let region_graph = &constraint_graph.region_graph(self, static_region);
-        ConstraintSccs::new_with_annotation(&region_graph, |r| {
-            RegionTracker::new(r, &definitions[r])
-        })
-    }
-
-    /// This method handles Universe errors by rewriting the constraint
-    /// graph. For each strongly connected component in the constraint
-    /// graph such that there is a series of constraints
-    ///    A: B: C: ... : X  where
-    /// A's universe is smaller than X's and A is a placeholder,
-    /// add a constraint that A: 'static. This is a safe upper bound
-    /// in the face of borrow checker/trait solver limitations that will
-    /// eventually go away.
-    ///
-    /// For a more precise definition, see the documentation for
-    /// [`RegionTracker::has_incompatible_universes()`].
-    ///
-    /// This edge case used to be handled during constraint propagation
-    /// by iterating over the strongly connected components in the constraint
-    /// graph while maintaining a set of bookkeeping mappings similar
-    /// to what is stored in `RegionTracker` and manually adding 'sttaic as
-    /// needed.
-    ///
-    /// It was rewritten as part of the Polonius project with the goal of moving
-    /// higher-kindedness concerns out of the path of the borrow checker,
-    /// for two reasons:
-    ///
-    /// 1. Implementing Polonius is difficult enough without also
-    ///     handling them.
-    /// 2. The long-term goal is to handle higher-kinded concerns
-    ///     in the trait solver, where they belong. This avoids
-    ///     logic duplication and allows future trait solvers
-    ///     to compute better bounds than for example our
-    ///     "must outlive 'static" here.
-    ///
-    /// This code is a stop-gap measure in preparation for the future trait solver.
-    ///
-    /// Every constraint added by this method is an
-    /// internal `IllegalUniverse` constraint.
-    #[instrument(skip(self, universal_regions, definitions))]
-    pub(crate) fn add_outlives_static(
-        &mut self,
-        universal_regions: &UniversalRegions<'tcx>,
-        definitions: &IndexVec<RegionVid, RegionDefinition<'tcx>>,
-    ) -> ConstraintSccs {
-        let fr_static = universal_regions.fr_static;
-        let sccs = self.compute_sccs(fr_static, definitions);
-
-        // Changed to `true` if we added any constraints to `self` and need to
-        // recompute SCCs.
-        let mut added_constraints = false;
-
-        for scc in sccs.all_sccs() {
-            // No point in adding 'static: 'static!
-            // This micro-optimisation makes somewhat sense
-            // because static outlives *everything*.
-            if scc == sccs.scc(fr_static) {
-                continue;
-            }
-
-            let annotation = sccs.annotation(scc);
-
-            // If this SCC participates in a universe violation,
-            // e.g. if it reaches a region with a universe smaller than
-            // the largest region reached, add a requirement that it must
-            // outlive `'static`.
-            if annotation.has_incompatible_universes() {
-                // Optimisation opportunity: this will add more constraints than
-                // needed for correctness, since an SCC upstream of another with
-                // a universe violation will "infect" its downstream SCCs to also
-                // outlive static.
-                added_constraints = true;
-                let scc_representative_outlives_static = OutlivesConstraint {
-                    sup: annotation.representative,
-                    sub: fr_static,
-                    category: ConstraintCategory::IllegalUniverse,
-                    locations: Locations::All(rustc_span::DUMMY_SP),
-                    span: rustc_span::DUMMY_SP,
-                    variance_info: VarianceDiagInfo::None,
-                    from_closure: false,
-                };
-                self.push(scc_representative_outlives_static);
-            }
-        }
-
-        if added_constraints {
-            // We changed the constraint set and so must recompute SCCs.
-            self.compute_sccs(fr_static, definitions)
-        } else {
-            // If we didn't add any back-edges; no more work needs doing
-            sccs
-        }
+        scc::Sccs::new_with_annotation(&region_graph, annotations)
     }
 }
 

compiler/rustc_borrowck/src/diagnostics/bound_region_errors.rs

@@ -24,7 +24,6 @@ use rustc_traits::{type_op_ascribe_user_type_with_span, type_op_prove_predicate_
 use tracing::{debug, instrument};
 
 use crate::MirBorrowckCtxt;
-use crate::region_infer::values::RegionElement;
 use crate::session_diagnostics::{
     HigherRankedErrorCause, HigherRankedLifetimeError, HigherRankedSubtypeError,
 };
@@ -49,12 +48,13 @@ impl<'tcx> UniverseInfo<'tcx> {
         UniverseInfo::RelateTys { expected, found }
     }
 
+    /// Report an error where an element erroneously made its way into `placeholder`.
     pub(crate) fn report_erroneous_element(
         &self,
         mbcx: &mut MirBorrowckCtxt<'_, '_, 'tcx>,
         placeholder: ty::PlaceholderRegion,
-        error_element: RegionElement,
         cause: ObligationCause<'tcx>,
+        error_element: Option<ty::PlaceholderRegion>,
     ) {
         match *self {
             UniverseInfo::RelateTys { expected, found } => {
@@ -68,7 +68,7 @@ impl<'tcx> UniverseInfo<'tcx> {
                 mbcx.buffer_error(err);
             }
             UniverseInfo::TypeOp(ref type_op_info) => {
-                type_op_info.report_erroneous_element(mbcx, placeholder, error_element, cause);
+                type_op_info.report_erroneous_element(mbcx, placeholder, cause, error_element);
             }
             UniverseInfo::Other => {
                 // FIXME: This error message isn't great, but it doesn't show
@@ -145,52 +145,54 @@ pub(crate) trait TypeOpInfo<'tcx> {
         error_region: Option<ty::Region<'tcx>>,
     ) -> Option<Diag<'infcx>>;
 
-    /// Constraints require that `error_element` appear in the
-    ///  values of `placeholder`, but this cannot be proven to
-    /// hold. Report an error.
+    /// Turn a placeholder region into a Region with its universe adjusted by
+    /// the base universe.
+    fn region_with_adjusted_universe(
+        &self,
+        placeholder: ty::PlaceholderRegion,
+        tcx: TyCtxt<'tcx>,
+    ) -> ty::Region<'tcx> {
+        let Some(adjusted_universe) =
+            placeholder.universe.as_u32().checked_sub(self.base_universe().as_u32())
+        else {
+            unreachable!(
+                "Could not adjust universe {:?} of {placeholder:?} by base universe {:?}",
+                placeholder.universe,
+                self.base_universe()
+            );
+        };
+        ty::Region::new_placeholder(
+            tcx,
+            ty::Placeholder { universe: adjusted_universe.into(), bound: placeholder.bound },
+        )
+    }
+
+    /// Report an error where an erroneous element reaches `placeholder`.
+    /// The erroneous element is either another placeholder that we provide,
+    ///  or we figure out what happened later.
     #[instrument(level = "debug", skip(self, mbcx))]
     fn report_erroneous_element(
         &self,
         mbcx: &mut MirBorrowckCtxt<'_, '_, 'tcx>,
         placeholder: ty::PlaceholderRegion,
-        error_element: RegionElement,
         cause: ObligationCause<'tcx>,
+        error_element: Option<ty::PlaceholderRegion>,
     ) {
         let tcx = mbcx.infcx.tcx;
-        let base_universe = self.base_universe();
-        debug!(?base_universe);
-
-        let Some(adjusted_universe) =
-            placeholder.universe.as_u32().checked_sub(base_universe.as_u32())
-        else {
-            mbcx.buffer_error(self.fallback_error(tcx, cause.span));
-            return;
-        };
 
-        let placeholder_region = ty::Region::new_placeholder(
-            tcx,
-            ty::Placeholder { universe: adjusted_universe.into(), bound: placeholder.bound },
-        );
-
-        let error_region = if let RegionElement::PlaceholderRegion(error_placeholder) =
-            error_element
-        {
-            let adjusted_universe =
-                error_placeholder.universe.as_u32().checked_sub(base_universe.as_u32());
-            adjusted_universe.map(|adjusted| {
-                ty::Region::new_placeholder(
-                    tcx,
-                    ty::Placeholder { universe: adjusted.into(), bound: error_placeholder.bound },
-                )
-            })
-        } else {
-            None
-        };
+        // FIXME: these adjusted universes are not (always) the same ones as we compute
+        // earlier. They probably should be, but the logic downstream is complicated,
+        // and assumes they use whatever this is.
+        //
+        // In fact, this  function throws away a lot of interesting information that would
+        // probably allow bypassing lots of logic downstream for a much simpler flow.
+        let placeholder_region = self.region_with_adjusted_universe(placeholder, tcx);
+        let error_element = error_element.map(|e| self.region_with_adjusted_universe(e, tcx));
 
         debug!(?placeholder_region);
 
         let span = cause.span;
-        let nice_error = self.nice_error(mbcx, cause, placeholder_region, error_region);
+        let nice_error = self.nice_error(mbcx, cause, placeholder_region, error_element);
 
         debug!(?nice_error);
         mbcx.buffer_error(nice_error.unwrap_or_else(|| self.fallback_error(tcx, span)));

compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs

@@ -565,7 +565,7 @@ impl<'tcx> MirBorrowckCtxt<'_, '_, 'tcx> {
         let (blame_constraint, path) = self.regioncx.best_blame_constraint(
             borrow_region,
             NllRegionVariableOrigin::FreeRegion,
-            |r| self.regioncx.provides_universal_region(r, borrow_region, outlived_region),
+            outlived_region,
         );
         let BlameConstraint { category, from_closure, cause, .. } = blame_constraint;
 

compiler/rustc_borrowck/src/diagnostics/opaque_suggestions.rs

@@ -174,10 +174,9 @@ impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for FindOpaqueRegion<'_, 'tcx> {
                 let opaque_region_vid = self.regioncx.to_region_vid(opaque_region);
 
                 // Find a path between the borrow region and our opaque capture.
-                if let Some((path, _)) =
-                    self.regioncx.find_constraint_paths_between_regions(self.borrow_region, |r| {
-                        r == opaque_region_vid
-                    })
+                if let Some((path, _)) = self
+                    .regioncx
+                    .constraint_path_between_regions(self.borrow_region, opaque_region_vid)
                 {
                     for constraint in path {
                         // If we find a call in this path, then check if it defines the opaque.