Auto merge of #123720 - amandasystems:dyn-enable-refactor, r=<try>

Rewrite handling of universe-leaking placeholder regions into outlives constraints

This commit prepares for Polonius by moving handling of leak check/universe errors out of the inference step by rewriting any universe error into an outlives-static constraint.

This variant is a work in progress but seems to pass most tests.

Note that a few debug assertions no longer hold; a few extra eyes on those changes are appreciated!

Author: bors

compiler/rustc_borrowck/src/constraints/mod.rs

@@ -1,12 +1,16 @@
+use rustc_data_structures::fx::FxHashSet;
 use rustc_data_structures::graph::scc::Sccs;
 use rustc_index::{IndexSlice, IndexVec};
+use rustc_infer::infer::NllRegionVariableOrigin;
 use rustc_middle::mir::ConstraintCategory;
 use rustc_middle::ty::{RegionVid, VarianceDiagInfo};
 use rustc_span::Span;
 use std::fmt;
 use std::ops::Index;
 
+use crate::region_infer::RegionDefinition;
 use crate::type_check::Locations;
+use crate::universal_regions::UniversalRegions;
 
 pub(crate) mod graph;
 
@@ -62,6 +66,65 @@ impl<'tcx> OutlivesConstraintSet<'tcx> {
     ) -> &IndexSlice<OutlivesConstraintIndex, OutlivesConstraint<'tcx>> {
         &self.outlives
     }
+    pub(crate) fn placeholders_to_static(
+        &mut self,
+        universal_regions: &UniversalRegions<'tcx>,
+        definitions: &IndexVec<RegionVid, RegionDefinition<'tcx>>,
+    ) {
+        let universe = |rvid: RegionVid| definitions[rvid].universe;
+
+        let is_placeholder = |rvid: RegionVid| {
+            matches!(definitions[rvid].origin, NllRegionVariableOrigin::Placeholder(_))
+        };
+
+        let outlives_static = |rvid: RegionVid| OutlivesConstraint {
+            sup: rvid,
+            sub: universal_regions.fr_static, // All the following values are made up ex nihil
+            category: ConstraintCategory::IllegalUniverse,
+            locations: Locations::All(rustc_span::DUMMY_SP),
+            span: rustc_span::DUMMY_SP,
+            variance_info: VarianceDiagInfo::None,
+            from_closure: false,
+        };
+
+        let outlived_regions = |longer: RegionVid| {
+            self.outlives().iter().filter_map(move |OutlivesConstraint { sub, sup, .. }| {
+                (*sup == longer).then_some(sub)
+            })
+        };
+
+        // Start a search to find reachable regions from here that have incompatible universes
+        let should_be_static:Vec<_> = definitions.indices().filter_map(|rvid| {
+            let mut queue: Vec<RegionVid> = outlived_regions(rvid).map(|&r| r).collect();
+            let mut seen: FxHashSet<_> = queue.iter().map(|&r| r).collect();
+            seen.insert(rvid);
+            debug!(?rvid);
+            debug!("rvid definition: {:?}", definitions[rvid]);
+            while let Some(outlived) = queue.pop() {
+                if is_placeholder(outlived) && universe(outlived) > universe(rvid) {
+                    debug!(
+                        "{rvid:?}: {outlived:?} => {rvid:?}: 'static, universe leak ({:?} > {:?})",
+                        universe(outlived),
+                        universe(rvid)
+                    );
+                    return Some(rvid);
+                } else {
+                    for &r in outlived_regions(outlived) {
+                        if seen.contains(&r) {
+                            continue;
+                        }
+                        seen.insert(r);
+                        queue.push(r);
+                    }
+                }
+            }
+            None
+        }).collect();
+
+        for rvid in should_be_static {
+            self.push(outlives_static(rvid));
+        }
+    }
 }
 
 impl<'tcx> Index<OutlivesConstraintIndex> for OutlivesConstraintSet<'tcx> {

compiler/rustc_borrowck/src/diagnostics/region_errors.rs

@@ -66,7 +66,8 @@ impl<'tcx> ConstraintDescription for ConstraintCategory<'tcx> {
             ConstraintCategory::Predicate(_)
             | ConstraintCategory::Boring
             | ConstraintCategory::BoringNoLocation
-            | ConstraintCategory::Internal => "",
+            | ConstraintCategory::Internal
+            | ConstraintCategory::IllegalUniverse => "",
         }
     }
 }

compiler/rustc_borrowck/src/region_infer/mod.rs

@@ -71,7 +71,7 @@ pub struct RegionInferenceContext<'tcx> {
     /// The SCC computed from `constraints` and the constraint
     /// graph. We have an edge from SCC A to SCC B if `A: B`. Used to
     /// compute the values of each region.
-    constraint_sccs: Rc<Sccs<RegionVid, ConstraintSccIndex>>,
+    constraint_sccs: Sccs<RegionVid, ConstraintSccIndex>,
 
     /// Reverse of the SCC constraint graph --  i.e., an edge `A -> B` exists if
     /// `B: A`. This is used to compute the universal regions that are required
@@ -251,7 +251,7 @@ pub enum ExtraConstraintInfo {
 #[instrument(skip(infcx, sccs), level = "debug")]
 fn sccs_info<'cx, 'tcx>(
     infcx: &'cx BorrowckInferCtxt<'cx, 'tcx>,
-    sccs: Rc<Sccs<RegionVid, ConstraintSccIndex>>,
+    sccs: &Sccs<RegionVid, ConstraintSccIndex>,
 ) {
     use crate::renumber::RegionCtxt;
 
@@ -322,12 +322,12 @@ impl<'tcx> RegionInferenceContext<'tcx> {
     /// The `outlives_constraints` and `type_tests` are an initial set
     /// of constraints produced by the MIR type check.
     pub(crate) fn new<'cx>(
-        _infcx: &BorrowckInferCtxt<'cx, 'tcx>,
+        infcx: &BorrowckInferCtxt<'cx, 'tcx>,
         var_infos: VarInfos,
         universal_regions: Rc<UniversalRegions<'tcx>>,
         placeholder_indices: Rc<PlaceholderIndices>,
         universal_region_relations: Frozen<UniversalRegionRelations<'tcx>>,
-        outlives_constraints: OutlivesConstraintSet<'tcx>,
+        mut outlives_constraints: OutlivesConstraintSet<'tcx>,
         member_constraints_in: MemberConstraintSet<'tcx, RegionVid>,
         universe_causes: FxIndexMap<ty::UniverseIndex, UniverseInfo<'tcx>>,
         type_tests: Vec<TypeTest<'tcx>>,
@@ -345,13 +345,15 @@ impl<'tcx> RegionInferenceContext<'tcx> {
             .map(|info| RegionDefinition::new(info.universe, info.origin))
             .collect();
 
+        outlives_constraints.placeholders_to_static(&universal_regions, &definitions);
+
         let constraints = Frozen::freeze(outlives_constraints);
         let constraint_graph = Frozen::freeze(constraints.graph(definitions.len()));
         let fr_static = universal_regions.fr_static;
-        let constraint_sccs = Rc::new(constraints.compute_sccs(&constraint_graph, fr_static));
+        let constraint_sccs = constraints.compute_sccs(&constraint_graph, fr_static);
 
         if cfg!(debug_assertions) {
-            sccs_info(_infcx, constraint_sccs.clone());
+            sccs_info(infcx, &constraint_sccs);
         }
 
         let mut scc_values =
@@ -535,7 +537,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         // This iterator has unstable order but we collect it all into an IndexVec
         for (external_name, variable) in self.universal_regions.named_universal_regions() {
             debug!(
-                "init_universal_regions: region {:?} has external name {:?}",
+                "init_free_and_bound_regions: region {:?} has external name {:?}",
                 variable, external_name
             );
             self.definitions[variable].external_name = Some(external_name);
@@ -557,21 +559,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
                 }
 
                 NllRegionVariableOrigin::Placeholder(placeholder) => {
-                    // Each placeholder region is only visible from
-                    // its universe `ui` and its extensions. So we
-                    // can't just add it into `scc` unless the
-                    // universe of the scc can name this region.
-                    let scc_universe = self.scc_universes[scc];
-                    if scc_universe.can_name(placeholder.universe) {
-                        self.scc_values.add_element(scc, placeholder);
-                    } else {
-                        debug!(
-                            "init_free_and_bound_regions: placeholder {:?} is \
-                             not compatible with universe {:?} of its SCC {:?}",
-                            placeholder, scc_universe, scc,
-                        );
-                        self.add_incompatible_universe(scc);
-                    }
+                    self.scc_values.add_element(scc, placeholder);
                 }
 
                 NllRegionVariableOrigin::Existential { .. } => {
@@ -739,8 +727,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         // SCC. For each SCC, we visit its successors and compute
         // their values, then we union all those values to get our
         // own.
-        let constraint_sccs = self.constraint_sccs.clone();
-        for scc in constraint_sccs.all_sccs() {
+        for scc in self.constraint_sccs.all_sccs() {
             self.compute_value_for_scc(scc);
         }
 
@@ -755,23 +742,10 @@ impl<'tcx> RegionInferenceContext<'tcx> {
     /// (which is assured by iterating over SCCs in dependency order).
     #[instrument(skip(self), level = "debug")]
     fn compute_value_for_scc(&mut self, scc_a: ConstraintSccIndex) {
-        let constraint_sccs = self.constraint_sccs.clone();
-
         // Walk each SCC `B` such that `A: B`...
-        for &scc_b in constraint_sccs.successors(scc_a) {
+        for &scc_b in self.constraint_sccs.successors(scc_a) {
             debug!(?scc_b);
-
-            // ...and add elements from `B` into `A`. One complication
-            // arises because of universes: If `B` contains something
-            // that `A` cannot name, then `A` can only contain `B` if
-            // it outlives static.
-            if self.universe_compatible(scc_b, scc_a) {
-                // `A` can name everything that is in `B`, so just
-                // merge the bits.
-                self.scc_values.add_region(scc_a, scc_b);
-            } else {
-                self.add_incompatible_universe(scc_a);
-            }
+            self.scc_values.add_region(scc_a, scc_b);
         }
 
         // Now take member constraints into account.
@@ -827,12 +801,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         if self.scc_universes[scc] != ty::UniverseIndex::ROOT {
             return;
         }
-        debug_assert!(
-            self.scc_values.placeholders_contained_in(scc).next().is_none(),
-            "scc {:?} in a member constraint has placeholder value: {:?}",
-            scc,
-            self.scc_values.region_value_str(scc),
-        );
 
         // The existing value for `scc` is a lower-bound. This will
         // consist of some set `{P} + {LB}` of points `{P}` and
@@ -917,21 +885,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         self.scc_values.placeholders_contained_in(scc_b).all(|p| universe_a.can_name(p.universe))
     }
 
-    /// Extend `scc` so that it can outlive some placeholder region
-    /// from a universe it can't name; at present, the only way for
-    /// this to be true is if `scc` outlives `'static`. This is
-    /// actually stricter than necessary: ideally, we'd support bounds
-    /// like `for<'a: 'b>` that might then allow us to approximate
-    /// `'a` with `'b` and not `'static`. But it will have to do for
-    /// now.
-    fn add_incompatible_universe(&mut self, scc: ConstraintSccIndex) {
-        debug!("add_incompatible_universe(scc={:?})", scc);
-
-        let fr_static = self.universal_regions.fr_static;
-        self.scc_values.add_all_points(scc);
-        self.scc_values.add_element(scc, fr_static);
-    }
-
     /// Once regions have been propagated, this method is used to see
     /// whether the "type tests" produced by typeck were satisfied;
     /// type tests encode type-outlives relationships like `T:
@@ -1563,7 +1516,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         // Because this free region must be in the ROOT universe, we
         // know it cannot contain any bound universes.
         assert!(self.scc_universes[longer_fr_scc] == ty::UniverseIndex::ROOT);
-        debug_assert!(self.scc_values.placeholders_contained_in(longer_fr_scc).next().is_none());
 
         // Only check all of the relations for the main representative of each
         // SCC, otherwise just check that we outlive said representative. This
@@ -1772,20 +1724,15 @@ impl<'tcx> RegionInferenceContext<'tcx> {
     ///   that cannot be named by `fr1`; in that case, we will require
     ///   that `fr1: 'static` because it is the only way to `fr1: r` to
     ///   be satisfied. (See `add_incompatible_universe`.)
+    #[instrument(skip(self), ret)]
     pub(crate) fn provides_universal_region(
         &self,
         r: RegionVid,
         fr1: RegionVid,
         fr2: RegionVid,
     ) -> bool {
-        debug!("provides_universal_region(r={:?}, fr1={:?}, fr2={:?})", r, fr1, fr2);
-        let result = {
-            r == fr2 || {
-                fr2 == self.universal_regions.fr_static && self.cannot_name_placeholder(fr1, r)
-            }
-        };
-        debug!("provides_universal_region: result = {:?}", result);
-        result
+        let fr2_is_static = fr2 == self.universal_regions.fr_static;
+        r == fr2 || (fr2_is_static && self.cannot_name_placeholder(fr1, r))
     }
 
     /// If `r2` represents a placeholder region, then this returns
@@ -1900,6 +1847,10 @@ impl<'tcx> RegionInferenceContext<'tcx> {
 
             // This loop can be hot.
             for constraint in outgoing_edges_from_graph {
+                if matches!(constraint.category, ConstraintCategory::IllegalUniverse) {
+                    debug!("Ignoring illegal universe constraint: {constraint:?}");
+                    continue;
+                }
                 handle_constraint(constraint);
             }
 
@@ -2249,7 +2200,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
 
     /// Access to the SCC constraint graph.
     pub(crate) fn constraint_sccs(&self) -> &Sccs<RegionVid, ConstraintSccIndex> {
-        self.constraint_sccs.as_ref()
+        &self.constraint_sccs
     }
 
     /// Access to the region graph, built from the outlives constraints.

compiler/rustc_middle/src/mir/query.rs

@@ -273,6 +273,8 @@ pub enum ConstraintCategory<'tcx> {
 
     /// A constraint that doesn't correspond to anything the user sees.
     Internal,
+    /// An internal constraint derived from an illegal universe relation.
+    IllegalUniverse,
 }
 
 #[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Hash)]