Auto merge of #140737 - amandasystems:revised-constraint-search, r=lcnr

Region inference: Use outlives-static constraints in constraint search

Revise the extra `r: 'static` constraints added upon universe issues to add an explanation, and use that explanation during constraint blame search. This greatly simplifies the region inference logic, which now does not need to reverse-engineer the event that caused a region to outlive `'static`.

This cosmetically changes the output of two UI tests. I blessed them i separate commits with separate motivations, but that can of course be squashed as desired. We probably want that.

The PR was extracted out of #130227 and consists of one-third of its functional payload.

r? lcnr

Author: bors

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_types.rs

@@ -243,10 +243,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_path_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.

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::OutlivesUnnameablePlaceholder(..) => "",
         }
     }
 }
@@ -369,11 +369,15 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                     let error_vid = self.regioncx.region_from_element(longer_fr, &error_element);
 
                     // Find the code to blame for the fact that `longer_fr` outlives `error_fr`.
-                    let (_, cause) = self.regioncx.find_outlives_blame_span(
-                        longer_fr,
-                        NllRegionVariableOrigin::Placeholder(placeholder),
-                        error_vid,
-                    );
+                    let cause = self
+                        .regioncx
+                        .best_blame_constraint(
+                            longer_fr,
+                            NllRegionVariableOrigin::Placeholder(placeholder),
+                            error_vid,
+                        )
+                        .0
+                        .cause;
 
                     let universe = placeholder.universe;
                     let universe_info = self.regioncx.universe_info(universe);
@@ -429,9 +433,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
     ) {
         debug!("report_region_error(fr={:?}, outlived_fr={:?})", fr, outlived_fr);
 
-        let (blame_constraint, path) = self.regioncx.best_blame_constraint(fr, fr_origin, |r| {
-            self.regioncx.provides_universal_region(r, fr, outlived_fr)
-        });
+        let (blame_constraint, path) =
+            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/handle_placeholders.rs

@@ -1,7 +1,6 @@
 //! Logic for lowering higher-kinded outlives constraints
 //! (with placeholders and universes) and turn them into regular
 //! outlives constraints.
-
 use rustc_data_structures::frozen::Frozen;
 use rustc_data_structures::fx::FxIndexMap;
 use rustc_data_structures::graph::scc;
@@ -10,7 +9,7 @@ use rustc_index::IndexVec;
 use rustc_infer::infer::RegionVariableOrigin;
 use rustc_middle::mir::ConstraintCategory;
 use rustc_middle::ty::{RegionVid, UniverseIndex};
-use tracing::debug;
+use tracing::{debug, trace};
 
 use crate::constraints::{ConstraintSccIndex, OutlivesConstraintSet};
 use crate::consumers::OutlivesConstraint;
@@ -62,84 +61,145 @@ impl scc::Annotations<RegionVid> for SccAnnotations<'_, '_, RegionTracker> {
     type SccIdx = ConstraintSccIndex;
 }
 
+#[derive(Copy, Debug, Clone, PartialEq, Eq)]
+enum PlaceholderReachability {
+    /// This SCC reaches no placeholders.
+    NoPlaceholders,
+    /// This SCC reaches at least one placeholder.
+    Placeholders {
+        /// The largest-universed placeholder we can reach
+        max_universe: (UniverseIndex, RegionVid),
+
+        /// The placeholder with the smallest ID
+        min_placeholder: RegionVid,
+
+        /// The placeholder with the largest ID
+        max_placeholder: RegionVid,
+    },
+}
+
+impl PlaceholderReachability {
+    /// Merge the reachable placeholders of two graph components.
+    fn merge(self, other: PlaceholderReachability) -> PlaceholderReachability {
+        use PlaceholderReachability::*;
+        match (self, other) {
+            (NoPlaceholders, NoPlaceholders) => NoPlaceholders,
+            (NoPlaceholders, p @ Placeholders { .. })
+            | (p @ Placeholders { .. }, NoPlaceholders) => p,
+            (
+                Placeholders {
+                    min_placeholder: min_pl,
+                    max_placeholder: max_pl,
+                    max_universe: max_u,
+                },
+                Placeholders { min_placeholder, max_placeholder, max_universe },
+            ) => Placeholders {
+                min_placeholder: min_pl.min(min_placeholder),
+                max_placeholder: max_pl.max(max_placeholder),
+                max_universe: max_u.max(max_universe),
+            },
+        }
+    }
+
+    fn max_universe(&self) -> Option<(UniverseIndex, RegionVid)> {
+        match self {
+            Self::NoPlaceholders => None,
+            Self::Placeholders { max_universe, .. } => Some(*max_universe),
+        }
+    }
+
+    /// If we have reached placeholders, determine if they can
+    /// be named from this universe.
+    fn can_be_named_by(&self, from: UniverseIndex) -> bool {
+        self.max_universe()
+            .is_none_or(|(max_placeholder_universe, _)| from.can_name(max_placeholder_universe))
+    }
+}
+
 /// An annotation for region graph SCCs that tracks
 /// the values of its elements. This annotates a single SCC.
 #[derive(Copy, Debug, Clone)]
 pub(crate) struct RegionTracker {
-    /// The largest universe of a placeholder reached from this SCC.
-    /// This includes placeholders within this SCC.
-    max_placeholder_universe_reached: UniverseIndex,
+    reachable_placeholders: PlaceholderReachability,
 
     /// The largest universe nameable from this SCC.
     /// It is the smallest nameable universes of all
-    /// existential regions reachable from it.
-    max_nameable_universe: UniverseIndex,
+    /// existential regions reachable from it. Small Rvids are preferred.
+    max_nameable_universe: (UniverseIndex, RegionVid),
 
     /// The representative Region Variable Id for this SCC.
     pub(crate) representative: Representative,
 }
 
 impl RegionTracker {
     pub(crate) fn new(rvid: RegionVid, definition: &RegionDefinition<'_>) -> Self {
-        let placeholder_universe =
+        let reachable_placeholders =
             if matches!(definition.origin, NllRegionVariableOrigin::Placeholder(_)) {
-                definition.universe
+                PlaceholderReachability::Placeholders {
+                    max_universe: (definition.universe, rvid),
+                    min_placeholder: rvid,
+                    max_placeholder: rvid,
+                }
             } else {
-                UniverseIndex::ROOT
+                PlaceholderReachability::NoPlaceholders
             };
 
         Self {
-            max_placeholder_universe_reached: placeholder_universe,
-            max_nameable_universe: definition.universe,
+            reachable_placeholders,
+            max_nameable_universe: (definition.universe, rvid),
             representative: Representative::new(rvid, definition),
         }
     }
 
     /// The largest universe this SCC can name. It's the smallest
-    /// largest nameable uninverse of any reachable region.
+    /// largest nameable universe of any reachable region, or
+    /// `max_nameable(r) = min (max_nameable(r') for r' reachable from r)`
     pub(crate) fn max_nameable_universe(self) -> UniverseIndex {
-        self.max_nameable_universe
+        self.max_nameable_universe.0
     }
 
     pub(crate) fn max_placeholder_universe_reached(self) -> UniverseIndex {
-        self.max_placeholder_universe_reached
-    }
-
-    fn merge_min_max_seen(&mut self, other: &Self) {
-        self.max_placeholder_universe_reached = std::cmp::max(
-            self.max_placeholder_universe_reached,
-            other.max_placeholder_universe_reached,
-        );
-
-        self.max_nameable_universe =
-            std::cmp::min(self.max_nameable_universe, other.max_nameable_universe);
-    }
-
-    /// Returns `true` if during the annotated SCC reaches a placeholder
-    /// with a universe larger than the smallest nameable universe of any
-    /// reachable existential region.
-    pub(crate) fn has_incompatible_universes(&self) -> bool {
-        self.max_nameable_universe().cannot_name(self.max_placeholder_universe_reached)
+        if let Some((universe, _)) = self.reachable_placeholders.max_universe() {
+            universe
+        } else {
+            UniverseIndex::ROOT
+        }
     }
 
     /// Determine if the tracked universes of the two SCCs are compatible.
     pub(crate) fn universe_compatible_with(&self, other: Self) -> bool {
+        // HACK: We first check whether we can name the highest existential universe
+        // of `other`. This only exists to avoid errors in case that scc already
+        // depends on a placeholder it cannot name itself.
         self.max_nameable_universe().can_name(other.max_nameable_universe())
-            || self.max_nameable_universe().can_name(other.max_placeholder_universe_reached)
+            || other.reachable_placeholders.can_be_named_by(self.max_nameable_universe())
+    }
+
+    /// If this SCC reaches a placeholder it can't name, return it.
+    fn unnameable_placeholder(&self) -> Option<(UniverseIndex, RegionVid)> {
+        self.reachable_placeholders.max_universe().filter(|&(placeholder_universe, _)| {
+            !self.max_nameable_universe().can_name(placeholder_universe)
+        })
     }
 }
 
 impl scc::Annotation for RegionTracker {
-    fn merge_scc(mut self, other: Self) -> Self {
-        self.representative = self.representative.merge_scc(other.representative);
-        self.merge_min_max_seen(&other);
-        self
+    fn merge_scc(self, other: Self) -> Self {
+        trace!("{:?} << {:?}", self.representative, other.representative);
+
+        Self {
+            representative: self.representative.min(other.representative),
+            max_nameable_universe: self.max_nameable_universe.min(other.max_nameable_universe),
+            reachable_placeholders: self.reachable_placeholders.merge(other.reachable_placeholders),
+        }
     }
 
-    fn merge_reached(mut self, other: Self) -> Self {
-        // No update to in-component values, only add seen values.
-        self.merge_min_max_seen(&other);
-        self
+    fn merge_reached(self, other: Self) -> Self {
+        Self {
+            max_nameable_universe: self.max_nameable_universe.min(other.max_nameable_universe),
+            reachable_placeholders: self.reachable_placeholders.merge(other.reachable_placeholders),
+            representative: self.representative,
+        }
     }
 }
 
@@ -310,28 +370,52 @@ fn rewrite_placeholder_outlives<'tcx>(
 
         let annotation = annotations[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.
-            let scc_representative_outlives_static = OutlivesConstraint {
-                sup: annotation.representative.rvid(),
-                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,
-            };
-            outlives_constraints.push(scc_representative_outlives_static);
-            added_constraints = true;
-            debug!("Added {:?}: 'static!", annotation.representative.rvid());
-        }
+        let Some((max_u, max_u_rvid)) = annotation.unnameable_placeholder() else {
+            continue;
+        };
+
+        debug!(
+            "Placeholder universe {max_u:?} is too large for its SCC, represented by {:?}",
+            annotation.representative
+        );
+
+        // We only add one `r: 'static` constraint per SCC, where `r` is the SCC representative.
+        // That constraint is annotated with some placeholder `unnameable` where
+        // `unnameable` is unnameable from `r` and there is a path in the constraint graph
+        // between them.
+        //
+        // There is one exception; if some other region in this SCC can't name `'r`, then
+        // we pick the region with the smallest universe in the SCC, so that a path can
+        // always start in `'r` to find a motivation that isn't cyclic.
+        let blame_to = if annotation.representative.rvid() == max_u_rvid {
+            // Assertion: the region that lowered our universe is an existential one and we are a placeholder!
+
+            // The SCC's representative is not nameable from some region
+            // that ends up in the SCC.
+            let small_universed_rvid = annotation.max_nameable_universe.1;
+            debug!(
+                "{small_universed_rvid:?} lowered our universe to {:?}",
+                annotation.max_nameable_universe()
+            );
+            small_universed_rvid
+        } else {
+            // `max_u_rvid` is not nameable by the SCC's representative.
+            max_u_rvid
+        };
+
+        // FIXME: if we can extract a useful blame span here, future error
+        // reporting and constraint search can be simplified.
+
+        added_constraints = true;
+        outlives_constraints.push(OutlivesConstraint {
+            sup: annotation.representative.rvid(),
+            sub: fr_static,
+            category: ConstraintCategory::OutlivesUnnameablePlaceholder(blame_to),
+            locations: Locations::All(rustc_span::DUMMY_SP),
+            span: rustc_span::DUMMY_SP,
+            variance_info: VarianceDiagInfo::None,
+            from_closure: false,
+        });
     }
     added_constraints
 }

compiler/rustc_borrowck/src/region_infer/graphviz.rs

@@ -12,9 +12,13 @@ use rustc_middle::ty::UniverseIndex;
 use super::*;
 
 fn render_outlives_constraint(constraint: &OutlivesConstraint<'_>) -> String {
-    match constraint.locations {
-        Locations::All(_) => "All(...)".to_string(),
-        Locations::Single(loc) => format!("{loc:?}"),
+    if let ConstraintCategory::OutlivesUnnameablePlaceholder(unnameable) = constraint.category {
+        format!("{unnameable:?} unnameable")
+    } else {
+        match constraint.locations {
+            Locations::All(_) => "All(...)".to_string(),
+            Locations::Single(loc) => format!("{loc:?}"),
+        }
     }
 }