Preliminary attempt at removing placeholders out of the borrow checker.

This version is maximally naive, and currently unsound.

Handle more placeholder errors.

This update 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 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.

Use annotions on the outlives-static constraints to
help the search for who is to blame.

Invalid placeholder relation constraints are redirects

This commit makes the search for blamable outlives constraints
treat an added `x: 'static` edge as a redirect to figure out
why it reached an invalid placeholder.

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.

This version mostly works!

I've switched to encoding our new outlives-static constraints
with two extra parameters: the source and drain of a path in the
original constraint graph that caused a placeholder issue.

This handles all of the soundness issues, leaving 16 failing
diagnostics.

Remove some stuff we don't need or can't use anymore

Figure out better names for opaque types!

Fix tidy error

Fix most of the cases of missing "implied 'static" warnings

Fix documentation that build-failed

Author: amandasystems

compiler/rustc_borrowck/src/constraints/graph.rs

@@ -106,15 +106,17 @@ impl<D: ConstraintGraphDirection> ConstraintGraph<D> {
     }
 
     /// Given a region `R`, iterate over all constraints `R: R1`.
+    /// if `static_region` is `None`, do not yield implicit
+    /// `'static -> a` edges.
     pub(crate) fn outgoing_edges<'a, 'tcx>(
         &'a self,
         region_sup: RegionVid,
         constraints: &'a OutlivesConstraintSet<'tcx>,
-        static_region: RegionVid,
+        static_region: Option<RegionVid>,
     ) -> Edges<'a, 'tcx, D> {
         //if this is the `'static` region and the graph's direction is normal,
         //then setup the Edges iterator to return all regions #53178
-        if region_sup == static_region && D::is_normal() {
+        if Some(region_sup) == static_region && D::is_normal() {
             Edges {
                 graph: self,
                 constraints,
@@ -135,7 +137,7 @@ pub(crate) struct Edges<'a, 'tcx, D: ConstraintGraphDirection> {
     constraints: &'a OutlivesConstraintSet<'tcx>,
     pointer: Option<OutlivesConstraintIndex>,
     next_static_idx: Option<usize>,
-    static_region: RegionVid,
+    static_region: Option<RegionVid>,
 }
 
 impl<'a, 'tcx, D: ConstraintGraphDirection> Iterator for Edges<'a, 'tcx, D> {
@@ -153,8 +155,12 @@ impl<'a, 'tcx, D: ConstraintGraphDirection> Iterator for Edges<'a, 'tcx, D> {
                 Some(next_static_idx + 1)
             };
 
+            let Some(static_region) = self.static_region else {
+                return None;
+            };
+
             Some(OutlivesConstraint {
-                sup: self.static_region,
+                sup: static_region,
                 sub: next_static_idx.into(),
                 locations: Locations::All(DUMMY_SP),
                 span: DUMMY_SP,
@@ -194,7 +200,11 @@ impl<'a, 'tcx, D: ConstraintGraphDirection> RegionGraph<'a, 'tcx, D> {
     /// there exists a constraint `R: R1`.
     pub(crate) fn outgoing_regions(&self, region_sup: RegionVid) -> Successors<'a, 'tcx, D> {
         Successors {
-            edges: self.constraint_graph.outgoing_edges(region_sup, self.set, self.static_region),
+            edges: self.constraint_graph.outgoing_edges(
+                region_sup,
+                self.set,
+                Some(self.static_region),
+            ),
         }
     }
 }

compiler/rustc_borrowck/src/constraints/mod.rs

@@ -2,6 +2,7 @@ use std::fmt;
 use std::ops::Index;
 
 use rustc_index::{IndexSlice, IndexVec};
+use rustc_infer::infer::NllRegionVariableOrigin;
 use rustc_middle::mir::ConstraintCategory;
 use rustc_middle::ty::{RegionVid, TyCtxt, VarianceDiagInfo};
 use rustc_span::Span;
@@ -69,6 +70,27 @@ impl<'tcx> OutlivesConstraintSet<'tcx> {
         })
     }
 
+    /// There is a placeholder violation; add a requirement
+    /// that some SCC outlive static and explain which region
+    /// reaching which other region caused that.
+    fn add_placeholder_violation_constraint(
+        &mut self,
+        outlives_static: RegionVid,
+        blame_from: RegionVid,
+        blame_to: RegionVid,
+        fr_static: RegionVid,
+    ) {
+        self.push(OutlivesConstraint {
+            sup: outlives_static,
+            sub: fr_static,
+            category: ConstraintCategory::IllegalPlaceholder(blame_from, blame_to),
+            locations: Locations::All(rustc_span::DUMMY_SP),
+            span: rustc_span::DUMMY_SP,
+            variance_info: VarianceDiagInfo::None,
+            from_closure: false,
+        });
+    }
+
     /// 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
@@ -79,7 +101,7 @@ impl<'tcx> OutlivesConstraintSet<'tcx> {
     /// eventually go away.
     ///
     /// For a more precise definition, see the documentation for
-    /// [`RegionTracker::has_incompatible_universes()`].
+    /// [`RegionTracker`] and its methods!.
     ///
     /// This edge case used to be handled during constraint propagation
     /// by iterating over the strongly connected components in the constraint
@@ -117,36 +139,67 @@ impl<'tcx> OutlivesConstraintSet<'tcx> {
         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,
+            // 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
+            // outlive `'static`. Here we get to know which reachable region
+            // caused the violation.
+            if let Some(to) = annotation.universe_violation() {
+                // Optimisation opportunity: this will potentially 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.
+                // outlive static. However, some of those may be useful for error
+                // reporting.
                 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);
+                self.add_placeholder_violation_constraint(
+                    annotation.representative,
+                    annotation.representative,
+                    to,
+                    fr_static,
+                );
+            }
+        }
+
+        // The second kind of violation: a placeholder reaching another placeholder.
+        // OPTIMIZATION: This one is even more optimisable since it adds constraints for every
+        // placeholder in an SCC.
+        for rvid in definitions.iter_enumerated().filter_map(|(rvid, definition)| {
+            if matches!(definition.origin, NllRegionVariableOrigin::Placeholder { .. }) {
+                Some(rvid)
+            } else {
+                None
+            }
+        }) {
+            let scc = sccs.scc(rvid);
+            let annotation = sccs.annotation(scc);
+
+            // Unwrap safety: since this is our SCC it must contain us, which is
+            // at worst min AND max, but it has at least one or there is a bug.
+            let min = annotation.min_reachable_placeholder.unwrap();
+            let max = annotation.max_reachable_placeholder.unwrap();
+
+            // Good path: Nothing to see here, at least no other placeholders!
+            if min == max {
+                continue;
             }
+
+            // Bad path: figure out who we illegally reached.
+            // Note that this will prefer the representative if it is a
+            // placeholder, since the representative has the smallest index!
+            let other_placeholder = if min != rvid { min } else { max };
+
+            debug!(
+                "Placeholder {rvid:?} of SCC {scc:?} reaches other placeholder {other_placeholder:?}"
+            );
+            added_constraints = true;
+            self.add_placeholder_violation_constraint(
+                annotation.representative,
+                rvid,
+                other_placeholder,
+                fr_static,
+            );
         }
 
         if added_constraints {

compiler/rustc_borrowck/src/diagnostics/bound_region_errors.rs

@@ -161,24 +161,12 @@ pub(crate) trait TypeOpInfo<'tcx> {
             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
-            };
-
         debug!(?placeholder_region);
 
+        // FIXME: this is obviously weird; this whole argument now does nothing and maybe
+        // it should?
         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, None);
 
         debug!(?nice_error);
 

compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs

@@ -491,7 +491,7 @@ impl<'tcx> MirBorrowckCtxt<'_, '_, 'tcx> {
         let (blame_constraint, extra_info) = 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/region_errors.rs

@@ -61,7 +61,7 @@ impl<'tcx> ConstraintDescription for ConstraintCategory<'tcx> {
             | ConstraintCategory::Boring
             | ConstraintCategory::BoringNoLocation
             | ConstraintCategory::Internal
-            | ConstraintCategory::IllegalUniverse => "",
+            | ConstraintCategory::IllegalPlaceholder(..) => "",
         }
     }
 }
@@ -206,52 +206,35 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
         &self,
         diag: &mut Diag<'_>,
         lower_bound: RegionVid,
-    ) {
+    ) -> Option<()> {
         let mut suggestions = vec![];
         let hir = self.infcx.tcx.hir();
 
-        // find generic associated types in the given region 'lower_bound'
-        let gat_id_and_generics = self
-            .regioncx
-            .placeholders_contained_in(lower_bound)
-            .map(|placeholder| {
-                if let Some(id) = placeholder.bound.kind.get_id()
-                    && let Some(placeholder_id) = id.as_local()
-                    && let gat_hir_id = self.infcx.tcx.local_def_id_to_hir_id(placeholder_id)
-                    && let Some(generics_impl) = self
-                        .infcx
-                        .tcx
-                        .parent_hir_node(self.infcx.tcx.parent_hir_id(gat_hir_id))
-                        .generics()
-                {
-                    Some((gat_hir_id, generics_impl))
-                } else {
-                    None
-                }
-            })
-            .collect::<Vec<_>>();
-        debug!(?gat_id_and_generics);
-
         // find higher-ranked trait bounds bounded to the generic associated types
+        let scc = self.regioncx.constraint_sccs().scc(lower_bound);
+        let placeholder: ty::PlaceholderRegion = self.regioncx.placeholder_representative(scc)?;
+        let placeholder_id = placeholder.bound.kind.get_id()?.as_local()?;
+        let gat_hir_id = self.infcx.tcx.local_def_id_to_hir_id(placeholder_id);
+        let generics_impl =
+            self.infcx.tcx.parent_hir_node(self.infcx.tcx.parent_hir_id(gat_hir_id)).generics()?;
+
         let mut hrtb_bounds = vec![];
-        gat_id_and_generics.iter().flatten().for_each(|(gat_hir_id, generics)| {
-            for pred in generics.predicates {
-                let BoundPredicate(WhereBoundPredicate { bound_generic_params, bounds, .. }) = pred
-                else {
-                    continue;
-                };
-                if bound_generic_params
-                    .iter()
-                    .rfind(|bgp| self.infcx.tcx.local_def_id_to_hir_id(bgp.def_id) == *gat_hir_id)
-                    .is_some()
-                {
-                    for bound in *bounds {
-                        hrtb_bounds.push(bound);
-                    }
+
+        for pred in generics_impl.predicates {
+            let BoundPredicate(WhereBoundPredicate { bound_generic_params, bounds, .. }) = pred
+            else {
+                continue;
+            };
+            if bound_generic_params
+                .iter()
+                .rfind(|bgp| self.infcx.tcx.local_def_id_to_hir_id(bgp.def_id) == gat_hir_id)
+                .is_some()
+            {
+                for bound in *bounds {
+                    hrtb_bounds.push(bound);
                 }
             }
-        });
-        debug!(?hrtb_bounds);
+        }
 
         hrtb_bounds.iter().for_each(|bound| {
             let Trait(PolyTraitRef { trait_ref, span: trait_span, .. }) = bound else {
@@ -300,6 +283,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                 Applicability::MaybeIncorrect,
             );
         }
+        Some(())
     }
 
     /// Produces nice borrowck error diagnostics for all the errors collected in `nll_errors`.
@@ -446,9 +430,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
         debug!("report_region_error(fr={:?}, outlived_fr={:?})", fr, outlived_fr);
 
         let (blame_constraint, extra_info) =
-            self.regioncx.best_blame_constraint(fr, fr_origin, |r| {
-                self.regioncx.provides_universal_region(r, fr, outlived_fr)
-            });
+            self.regioncx.best_blame_constraint(fr, fr_origin, outlived_fr);
         let BlameConstraint { category, cause, variance_info, .. } = blame_constraint;
 
         debug!("report_region_error: category={:?} {:?} {:?}", category, cause, variance_info);

compiler/rustc_borrowck/src/nll.rs

@@ -122,15 +122,13 @@ pub(crate) fn compute_regions<'a, 'tcx>(
     // base constraints generated by the type-check.
     let var_origins = infcx.get_region_var_origins();
     let MirTypeckRegionConstraints {
-        placeholder_indices,
-        placeholder_index_to_region: _,
         liveness_constraints,
         mut outlives_constraints,
         mut member_constraints,
         universe_causes,
         type_tests,
+        ..
     } = constraints;
-    let placeholder_indices = Rc::new(placeholder_indices);
 
     // If requested, emit legacy polonius facts.
     polonius::emit_facts(
@@ -158,7 +156,6 @@ pub(crate) fn compute_regions<'a, 'tcx>(
         infcx,
         var_origins,
         universal_regions,
-        placeholder_indices,
         universal_region_relations,
         outlives_constraints,
         member_constraints,

compiler/rustc_borrowck/src/region_infer/graphviz.rs

@@ -12,6 +12,9 @@ use rustc_middle::ty::UniverseIndex;
 use super::*;
 
 fn render_outlives_constraint(constraint: &OutlivesConstraint<'_>) -> String {
+    if let ConstraintCategory::IllegalPlaceholder(from, to) = constraint.category {
+        return format!("b/c {from:?}: {to:?}");
+    }
     match constraint.locations {
         Locations::All(_) => "All(...)".to_string(),
         Locations::Single(loc) => format!("{loc:?}"),