Produce slightly fewer placeholder constraints

This seems to hit the sweet spot between enabling debugging
and being efficient.

Somewhat cleaner representation that separates concerns

Avoid handling placeholders for MIR bodies without them

Determine once and for all if there are higher-kinded concerns and nope out

This also changes how annotations for SCCs work.

This version finally does away with all kinds of universe handling

It does so in a preprocessing step that now also removes higher-ranked
concerns from type tests.

Fix handling of opaque regions

Author: amandasystems

compiler/rustc_borrowck/src/constraints/mod.rs

@@ -1,14 +1,16 @@
 use std::fmt;
 use std::ops::Index;
 
+use rustc_data_structures::fx::FxHashSet;
+use rustc_data_structures::graph::scc;
 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;
 use tracing::{debug, instrument};
 
-use crate::region_infer::{ConstraintSccs, RegionDefinition, RegionTracker};
+use crate::region_infer::{RegionDefinition, RegionTracker, SccAnnotations};
 use crate::type_check::Locations;
 use crate::universal_regions::UniversalRegions;
 
@@ -58,16 +60,18 @@ impl<'tcx> OutlivesConstraintSet<'tcx> {
     /// Computes cycles (SCCs) in the graph of regions. In particular,
     /// find all regions R1, R2 such that R1: R2 and R2: R1 and group
     /// them into an SCC, and find the relationships between SCCs.
-    pub(crate) fn compute_sccs(
+    pub(crate) fn compute_sccs<
+        A: scc::Annotation,
+        AA: scc::Annotations<RegionVid, ConstraintSccIndex, A>,
+    >(
         &self,
         static_region: RegionVid,
-        definitions: &IndexVec<RegionVid, RegionDefinition<'tcx>>,
-    ) -> ConstraintSccs {
-        let constraint_graph = self.graph(definitions.len());
+        num_region_vars: usize,
+        annotations: &mut AA,
+    ) -> scc::Sccs<RegionVid, ConstraintSccIndex> {
+        let constraint_graph = self.graph(num_region_vars);
         let region_graph = &constraint_graph.region_graph(self, static_region);
-        ConstraintSccs::new_with_annotation(&region_graph, |r| {
-            RegionTracker::new(r, &definitions[r])
-        })
+        scc::Sccs::new_with_annotation(&region_graph, annotations)
     }
 
     /// There is a placeholder violation; add a requirement
@@ -126,88 +130,91 @@ impl<'tcx> OutlivesConstraintSet<'tcx> {
     /// Every constraint added by this method is an
     /// internal `IllegalUniverse` constraint.
     #[instrument(skip(self, universal_regions, definitions))]
-    pub(crate) fn add_outlives_static(
+    pub(crate) fn add_outlives_static<'d>(
         &mut self,
         universal_regions: &UniversalRegions<'tcx>,
-        definitions: &IndexVec<RegionVid, RegionDefinition<'tcx>>,
-    ) -> ConstraintSccs {
+        definitions: &'d IndexVec<RegionVid, RegionDefinition<'tcx>>,
+    ) -> (scc::Sccs<RegionVid, ConstraintSccIndex>, IndexVec<ConstraintSccIndex, RegionTracker>)
+    {
         let fr_static = universal_regions.fr_static;
-        let sccs = self.compute_sccs(fr_static, definitions);
+        let mut annotations = SccAnnotations::init(definitions);
+        let sccs = self.compute_sccs(fr_static, definitions.len(), &mut annotations);
 
-        // Changed to `true` if we added any constraints to `self` and need to
-        // recompute SCCs.
-        let mut added_constraints = false;
+        // Is this SCC already outliving static directly or transitively?
+        let mut outlives_static = FxHashSet::default();
 
         for scc in sccs.all_sccs() {
-            let annotation = sccs.annotation(scc);
+            let annotation: RegionTracker = annotations.scc_to_annotation[scc];
+            if scc == sccs.scc(fr_static) {
+                // No use adding 'static: 'static.
+                continue;
+            }
 
             // 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`. 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. However, some of those may be useful for error
-                // reporting.
-                added_constraints = true;
+                outlives_static.insert(scc);
                 self.add_placeholder_violation_constraint(
-                    annotation.representative,
-                    annotation.representative,
+                    annotation.representative_rvid(),
+                    annotation.representative_rvid(),
                     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);
+        // Note: it's possible to sort this iterator by SCC and get dependency order,
+        // which makes it easy to only add only one constraint per future cycle.
+        // However, this worsens diagnostics and requires iterating over
+        // all successors to determine if we outlive static transitively,
+        // a cost you pay even if you have no placeholders.
+        let placeholders_and_sccs =
+            definitions.iter_enumerated().filter_map(|(rvid, definition)| {
+                if matches!(definition.origin, NllRegionVariableOrigin::Placeholder { .. }) {
+                    Some((sccs.scc(rvid), rvid))
+                } else {
+                    None
+                }
+            });
 
-            // 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();
+        // The second kind of violation: a placeholder reaching another placeholder.
+        for (scc, rvid) in placeholders_and_sccs {
+            let annotation = annotations.scc_to_annotation[scc];
 
-            // Good path: Nothing to see here, at least no other placeholders!
-            if min == max {
+            if sccs.scc(fr_static) == scc || outlives_static.contains(&scc) {
+                debug!("{:?} already outlives (or is) static", annotation.representative_rvid());
                 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 let Some(other_placeholder) = annotation.reaches_other_placeholder(rvid) {
+                debug!(
+                    "Placeholder {rvid:?} of SCC {scc:?} reaches other placeholder {other_placeholder:?}"
+                );
+                outlives_static.insert(scc);
+                self.add_placeholder_violation_constraint(
+                    annotation.representative_rvid(),
+                    rvid,
+                    other_placeholder,
+                    fr_static,
+                );
+            };
         }
 
-        if added_constraints {
+        if !outlives_static.is_empty() {
+            debug!("The following SCCs had :'static constraints added: {:?}", outlives_static);
+            let mut annotations = SccAnnotations::init(definitions);
+
             // We changed the constraint set and so must recompute SCCs.
-            self.compute_sccs(fr_static, definitions)
+            (
+                self.compute_sccs(fr_static, definitions.len(), &mut annotations),
+                annotations.scc_to_annotation,
+            )
         } else {
             // If we didn't add any back-edges; no more work needs doing
-            sccs
+            (sccs, annotations.scc_to_annotation)
         }
     }
 }

compiler/rustc_borrowck/src/diagnostics/opaque_suggestions.rs

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

@@ -212,7 +212,9 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
 
         // 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 =
@@ -221,7 +223,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
         let mut hrtb_bounds = vec![];
 
         for pred in generics_impl.predicates {
-            let BoundPredicate(WhereBoundPredicate { bound_generic_params, bounds, .. }) = pred
+            let BoundPredicate(WhereBoundPredicate { bound_generic_params, bounds, .. }): &rustc_hir::WherePredicate<'tcx> = pred
             else {
                 continue;
             };
@@ -329,6 +331,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                             span: type_test_span,
                         });
 
+                        debug!("This bound caused outlives static: {:?}", type_test.derived_from);
+
                         // Add notes and suggestions for the case of 'static lifetime
                         // implied but not specified when a generic associated types
                         // are from higher-ranked trait bounds

compiler/rustc_borrowck/src/nll.rs

@@ -7,15 +7,18 @@ use std::{env, io};
 
 use polonius_engine::{Algorithm, Output};
 use rustc_data_structures::fx::FxIndexMap;
+use rustc_data_structures::graph::scc::Sccs;
 use rustc_hir::def_id::LocalDefId;
-use rustc_index::IndexSlice;
+use rustc_index::{IndexSlice, IndexVec};
+use rustc_infer::infer::NllRegionVariableOrigin;
+use rustc_infer::infer::region_constraints::RegionVariableInfo;
 use rustc_middle::mir::pretty::{PrettyPrintMirOptions, dump_mir_with_options};
 use rustc_middle::mir::{
     Body, ClosureOutlivesSubject, ClosureRegionRequirements, PassWhere, Promoted, create_dump_file,
     dump_enabled, dump_mir,
 };
 use rustc_middle::ty::print::with_no_trimmed_paths;
-use rustc_middle::ty::{self, OpaqueHiddenType, TyCtxt};
+use rustc_middle::ty::{self, OpaqueHiddenType, RegionVid, TyCtxt};
 use rustc_mir_dataflow::ResultsCursor;
 use rustc_mir_dataflow::impls::MaybeInitializedPlaces;
 use rustc_mir_dataflow::move_paths::MoveData;
@@ -25,11 +28,14 @@ use rustc_span::symbol::sym;
 use tracing::{debug, instrument};
 
 use crate::borrow_set::BorrowSet;
+use crate::constraints::{ConstraintSccIndex, OutlivesConstraintSet};
 use crate::consumers::ConsumerOptions;
 use crate::diagnostics::RegionErrors;
 use crate::facts::{AllFacts, AllFactsExt, RustcFacts};
 use crate::location::LocationTable;
-use crate::region_infer::RegionInferenceContext;
+use crate::region_infer::{
+    RegionDefinition, RegionInferenceContext, Representative, SccAnnotations, TypeTest,
+};
 use crate::type_check::{self, MirTypeckRegionConstraints, MirTypeckResults};
 use crate::universal_regions::UniversalRegions;
 use crate::{BorrowckInferCtxt, polonius, renumber};
@@ -152,6 +158,15 @@ pub(crate) fn compute_regions<'a, 'tcx>(
         infcx.set_tainted_by_errors(guar);
     }
 
+    let (type_tests, sccs, region_definitions, scc_representatives) =
+        rewrite_higher_kinded_outlives_as_constraints(
+            &mut outlives_constraints,
+            &var_origins,
+            &universal_regions,
+            type_tests,
+            infcx.tcx,
+        );
+
     let mut regioncx = RegionInferenceContext::new(
         infcx,
         var_origins,
@@ -163,6 +178,9 @@ pub(crate) fn compute_regions<'a, 'tcx>(
         type_tests,
         liveness_constraints,
         elements,
+        sccs,
+        region_definitions,
+        scc_representatives,
     );
 
     // If requested: dump NLL facts, and run legacy polonius analysis.
@@ -208,6 +226,78 @@ pub(crate) fn compute_regions<'a, 'tcx>(
     }
 }
 
+fn rewrite_higher_kinded_outlives_as_constraints<'tcx>(
+    outlives_constraints: &mut OutlivesConstraintSet<'tcx>,
+    var_origins: &IndexVec<RegionVid, RegionVariableInfo>,
+    universal_regions: &UniversalRegions<'tcx>,
+    type_tests: Vec<TypeTest<'tcx>>,
+    tcx: TyCtxt<'tcx>,
+) -> (
+    Vec<TypeTest<'tcx>>,
+    Sccs<ty::RegionVid, ConstraintSccIndex>,
+    IndexVec<ty::RegionVid, RegionDefinition<'tcx>>,
+    IndexVec<ConstraintSccIndex, Representative>,
+) {
+    // Create a RegionDefinition for each inference variable. This happens here because
+    // it allows us to sneak in a cheap check for placeholders. Otherwise, its proper home
+    // is in `RegionInferenceContext::new()`, probably.
+    let (definitions, has_placeholders) = {
+        let mut definitions = IndexVec::with_capacity(var_origins.len());
+        let mut has_placeholders = false;
+
+        for info in var_origins.iter() {
+            let definition = RegionDefinition::new(info);
+            has_placeholders |=
+                matches!(definition.origin, NllRegionVariableOrigin::Placeholder(_));
+            definitions.push(definition);
+        }
+
+        // Add external names from universal regions in fun function definitions.
+        for (external_name, variable) in universal_regions.named_universal_regions() {
+            debug!("region {:?} has external name {:?}", variable, external_name);
+            definitions[variable].external_name = Some(external_name);
+        }
+        (definitions, has_placeholders)
+    };
+
+    if !has_placeholders {
+        debug!("No placeholder regions found; skipping rewriting logic!");
+        let mut annotations = SccAnnotations::init(&definitions);
+        let sccs = outlives_constraints.compute_sccs(
+            universal_regions.fr_static,
+            definitions.len(),
+            &mut annotations,
+        );
+        let annotations = annotations.scc_to_annotation;
+        return (type_tests, sccs, definitions, annotations);
+    }
+
+    debug!("Placeholders present; activating placeholder handling logic!");
+    let (sccs, scc_annotations) =
+        outlives_constraints.add_outlives_static(&universal_regions, &definitions);
+
+    // Rewrite universe-violating type tests into outlives 'static while we remember
+    // which universes go where.
+    let type_tests = type_tests
+        .into_iter()
+        .map(|type_test| {
+            type_test.rewrite_higher_kinded_constraints(
+                &sccs,
+                &scc_annotations,
+                universal_regions,
+                tcx,
+            )
+        })
+        .collect();
+
+    let representatives = scc_annotations
+        .into_iter()
+        .map(|rich_annotation| rich_annotation.into_representative())
+        .collect();
+
+    (type_tests, sccs, definitions, representatives)
+}
+
 /// `-Zdump-mir=nll` dumps MIR annotated with NLL specific information:
 /// - free regions
 /// - inferred region values

compiler/rustc_borrowck/src/region_infer/dump_mir.rs

@@ -42,10 +42,9 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         for region in self.regions() {
             writeln!(
                 out,
-                "| {r:rw$?} | {ui:4?} | {v}",
+                "| {r:rw$?} | {v}",
                 r = region,
                 rw = REGION_WIDTH,
-                ui = self.region_universe(region),
                 v = self.region_value_str(region),
             )?;
         }