Invalid placeholder relation constraints are redirects

amandasystems · amandasystems · commit 6ab45ce111b9 · 2024-09-12T15:52:03.000+02:00
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.
diff --git a/compiler/rustc_borrowck/src/constraints/graph.rs b/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<'s, 'tcx, D: ConstraintGraphDirection> {
     constraints: &'s OutlivesConstraintSet<'tcx>,
     pointer: Option<OutlivesConstraintIndex>,
     next_static_idx: Option<usize>,
-    static_region: RegionVid,
+    static_region: Option<RegionVid>,
 }
 
 impl<'s, 'tcx, D: ConstraintGraphDirection> Iterator for Edges<'s, 'tcx, D> {
@@ -153,8 +155,12 @@ impl<'s, 'tcx, D: ConstraintGraphDirection> Iterator for Edges<'s, '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<'s, 'tcx, D: ConstraintGraphDirection> RegionGraph<'s, 'tcx, D> {
     /// there exists a constraint `R: R1`.
     pub(crate) fn outgoing_regions(&self, region_sup: RegionVid) -> Successors<'s, '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),
+            ),
         }
     }
 }
diff --git a/compiler/rustc_borrowck/src/constraints/mod.rs b/compiler/rustc_borrowck/src/constraints/mod.rs
@@ -131,6 +131,10 @@ impl<'tcx> OutlivesConstraintSet<'tcx> {
             // reaches another placeholder, add a requirement that it must
             // outlive `'static`.
             if let Some(offending_region) = annotation.placeholder_violation(&sccs) {
+                assert!(
+                    annotation.representative != offending_region,
+                    "Attemtping to blame a constraint for itself!"
+                );
                 // 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
diff --git a/compiler/rustc_borrowck/src/region_infer/graphviz.rs b/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(p) = constraint.category {
+        return format!("b/c {p:?}");
+    }
     match constraint.locations {
         Locations::All(_) => "All(...)".to_string(),
         Locations::Single(loc) => format!("{loc:?}"),
diff --git a/compiler/rustc_borrowck/src/region_infer/mod.rs b/compiler/rustc_borrowck/src/region_infer/mod.rs
@@ -1777,18 +1777,20 @@ impl<'tcx> RegionInferenceContext<'tcx> {
     }
 
     /// Walks the graph of constraints (where `'a: 'b` is considered
-    /// an edge `'a -> 'b`) to find all paths from `from_region` to
-    /// `to_region`. The paths are accumulated into the vector
-    /// `results`. The paths are stored as a series of
-    /// `ConstraintIndex` values -- in other words, a list of *edges*.
-    ///
+    /// an edge `'a -> 'b`) to find a path from `from_region` to
+    /// the first region `R` for which the predicate function
+    /// `target_test` returns `true`.
     /// Returns: a series of constraints as well as the region `R`
     /// that passed the target test.
+    /// If `include_static_outlives_all` is `true`, then the synthetic
+    /// outlives constraints `'static -> a` for every region `a` are
+    /// considered in the search, otherwise they are ignored.
     #[instrument(skip(self, target_test), ret)]
-    pub(crate) fn find_constraint_paths_between_regions(
+    pub(crate) fn find_constraint_path_to(
         &self,
         from_region: RegionVid,
         target_test: impl Fn(RegionVid) -> bool,
+        include_static_outlives_all: bool,
     ) -> Option<(Vec<OutlivesConstraint<'tcx>>, RegionVid)> {
         let mut context = IndexVec::from_elem(Trace::NotVisited, &self.definitions);
         context[from_region] = Trace::StartRegion;
@@ -1801,7 +1803,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
 
         while let Some(r) = deque.pop_front() {
             debug!(
-                "find_constraint_paths_between_regions: from_region={:?} r={:?} value={}",
+                "find_constraint_path_to: from_region={:?} r={:?} value={}",
                 from_region,
                 r,
                 self.region_value_str(r),
@@ -1837,7 +1839,11 @@ impl<'tcx> RegionInferenceContext<'tcx> {
 
             // A constraint like `'r: 'x` can come from our constraint
             // graph.
-            let fr_static = self.universal_regions.fr_static;
+            let fr_static = if include_static_outlives_all {
+                Some(self.universal_regions.fr_static)
+            } else {
+                None
+            };
             let outgoing_edges_from_graph =
                 self.constraint_graph.outgoing_edges(r, &self.constraints, fr_static);
 
@@ -1883,11 +1889,13 @@ impl<'tcx> RegionInferenceContext<'tcx> {
     pub(crate) fn find_sub_region_live_at(&self, fr1: RegionVid, location: Location) -> RegionVid {
         trace!(scc = ?self.constraint_sccs.scc(fr1));
         trace!(universe = ?self.region_universe(fr1));
-        self.find_constraint_paths_between_regions(fr1, |r| {
+        self.find_constraint_path_to(fr1, |r| {
             // First look for some `r` such that `fr1: r` and `r` is live at `location`
             trace!(?r, liveness_constraints=?self.liveness_constraints.pretty_print_live_points(r));
             self.liveness_constraints.is_live_at(r, location)
-        })
+        },
+        true
+    )
         .map(|(_path, r)| r)
         .unwrap()
     }
@@ -1919,6 +1927,66 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         self.universal_regions.as_ref()
     }
 
+    /// Find a path of outlives constraints from `from` to `to`,
+    /// taking placeholder blame constraints into account, e.g.
+    /// if there is a relationship where `r1` reaches `r2` and
+    /// r2 has a larger universe or if r1 and r2 both come from
+    /// placeholder regions.
+    ///
+    /// Returns the path and the target region, which may or may
+    /// not be the original `to`. It panics if there is no such
+    /// path.
+    fn path_to_modulo_placeholders(
+        &self,
+        from: RegionVid,
+        to: RegionVid,
+    ) -> (Vec<OutlivesConstraint<'tcx>>, RegionVid) {
+        let path = self.find_constraint_path_to(from, |r| r == to, true).unwrap().0;
+
+        // If we are looking for a path to 'static, and we are passing
+        // through a constraint synthesised from an illegal placeholder
+        // relation, redirect the search to the placeholder to blame.
+        if self.is_static(to) {
+            for constraint in path.iter() {
+                let ConstraintCategory::IllegalPlaceholder(culprit_r) = constraint.category else {
+                    continue;
+                };
+
+                debug!("{culprit_r:?} is the reason {from:?}: 'static!");
+                // FIXME: think: this may be for transitive reasons and
+                // we may have to do this arbitrarily many times. Or may we?
+                return self.find_constraint_path_to(from, |r| r == culprit_r, false).unwrap();
+            }
+        }
+        // No funny business; just return the path!
+        (path, to)
+    }
+
+    /// Find interesting spans from bound placeholders' predicates
+    /// from a constraint path.
+    fn find_bound_region_predicate_span(
+        &self,
+        path: &[OutlivesConstraint<'_>],
+    ) -> Vec<ExtraConstraintInfo> {
+        for constraint in path.iter() {
+            let outlived = constraint.sub;
+            let Some(origin) = self.var_infos.get(outlived) else {
+                continue;
+            };
+            let RegionVariableOrigin::Nll(NllRegionVariableOrigin::Placeholder(p)) = origin.origin
+            else {
+                continue;
+            };
+            debug!(?constraint, ?p);
+            let ConstraintCategory::Predicate(span) = constraint.category else {
+                continue;
+            };
+            // We only want to point to one
+            return vec![ExtraConstraintInfo::PlaceholderFromPredicate(span)];
+        }
+        vec![]
+    }
+
     /// Tries to find the best constraint to blame for the fact that
     /// `to_region: from_region`.
     /// This works by following the constraint graph,
@@ -1932,34 +2000,10 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         from_region_origin: NllRegionVariableOrigin,
         to_region: RegionVid,
     ) -> (BlameConstraint<'tcx>, Vec<ExtraConstraintInfo>) {
-        let result = self.best_blame_constraint_(from_region, from_region_origin, to_region);
-
-        // We are trying to blame an outlives-static constraint added
-        // by an issue with placeholder regions. We figure out why the placeholder
-        // region issue happened instead.
-        if let ConstraintCategory::IllegalPlaceholder(offending_r) = result.0.category {
-            debug!("best_blame_constraint: placeholder issue caused by {offending_r:?}");
-
-            if to_region == offending_r {
-                // We do not want an infinite loop.
-                return result;
-            }
-            return self.best_blame_constraint(from_region, from_region_origin, offending_r);
-        }
+        assert!(from_region != to_region, "Trying to blame a region for itself!");
 
-        result
-    }
+        let (path, new_to_region) = self.path_to_modulo_placeholders(from_region, to_region);
 
-    #[instrument(level = "debug", skip(self))]
-    pub(crate) fn best_blame_constraint_(
-        &self,
-        from_region: RegionVid,
-        from_region_origin: NllRegionVariableOrigin,
-        to_region: RegionVid,
-    ) -> (BlameConstraint<'tcx>, Vec<ExtraConstraintInfo>) {
-        // Find all paths
-        let (path, target_region) =
-            self.find_constraint_paths_between_regions(from_region, |r| r == to_region).unwrap();
         debug!(
             "path={:#?}",
             path.iter()
@@ -1972,24 +2016,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
                 .collect::<Vec<_>>()
         );
 
-        let mut extra_info = vec![];
-        for constraint in path.iter() {
-            let outlived = constraint.sub;
-            let Some(origin) = self.var_infos.get(outlived) else {
-                continue;
-            };
-            let RegionVariableOrigin::Nll(NllRegionVariableOrigin::Placeholder(p)) = origin.origin
-            else {
-                continue;
-            };
-            debug!(?constraint, ?p);
-            let ConstraintCategory::Predicate(span) = constraint.category else {
-                continue;
-            };
-            extra_info.push(ExtraConstraintInfo::PlaceholderFromPredicate(span));
-            // We only want to point to one
-            break;
-        }
+        let extra_info = self.find_bound_region_predicate_span(&path);
 
         // We try to avoid reporting a `ConstraintCategory::Predicate` as our best constraint.
         // Instead, we use it to produce an improved `ObligationCauseCode`.
@@ -2040,7 +2067,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         // most likely to be the point where the value escapes -- but
         // we still want to screen for an "interesting" point to
         // highlight (e.g., a call site or something).
-        let target_scc = self.constraint_sccs.scc(target_region);
+        let target_scc = self.constraint_sccs.scc(new_to_region);
         let mut range = 0..path.len();
 
         // As noted above, when reporting an error, there is typically a chain of constraints
@@ -2237,6 +2264,11 @@ impl<'tcx> RegionInferenceContext<'tcx> {
     fn scc_representative(&self, scc: ConstraintSccIndex) -> RegionVid {
         self.constraint_sccs.annotation(scc).representative
     }
+
+    /// Returns true if `r` is `'static`.
+    fn is_static(&self, r: RegionVid) -> bool {
+        r == self.universal_regions.fr_static
+    }
 }
 
 impl<'tcx> RegionDefinition<'tcx> {
