Auto merge of #140737 - amandasystems:revised-constraint-search, r=<try>

Region inference: Use outlives-static constraints in constraint search

Author: rust-bors[bot]

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

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

compiler/rustc_borrowck/src/diagnostics/region_errors.rs

@@ -62,7 +62,7 @@ impl<'tcx> ConstraintDescription for ConstraintCategory<'tcx> {
             | ConstraintCategory::Boring
             | ConstraintCategory::BoringNoLocation
             | ConstraintCategory::Internal
-            | ConstraintCategory::IllegalUniverse => "",
+            | ConstraintCategory::OutlivesUnnameablePlaceholder(..) => "",
         }
     }
 }
@@ -410,11 +410,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);
@@ -470,9 +474,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,6 +1,7 @@
 //! Logic for lowering higher-kinded outlives constraints
 //! (with placeholders and universes) and turn them into regular
 //! outlives constraints.
+use core::cmp;
 
 use rustc_data_structures::frozen::Frozen;
 use rustc_data_structures::fx::FxIndexMap;
@@ -10,7 +11,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;
@@ -64,84 +65,147 @@ 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: cmp::min(min_pl, min_placeholder),
+                max_placeholder: cmp::max(max_pl, max_placeholder),
+                max_universe: cmp::max(max_u, 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.merge_scc(other.representative),
+            ..self.merge_reached(other)
+        }
     }
 
-    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: cmp::min(
+                self.max_nameable_universe,
+                other.max_nameable_universe,
+            ),
+            reachable_placeholders: self.reachable_placeholders.merge(other.reachable_placeholders),
+            representative: self.representative,
+        }
     }
 }
 
@@ -314,28 +378,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:?}"),
+        }
     }
 }