Unrolled build for #161423 in rollup 162487

Rollup merge of #161423 - Dnreikronos:trait_selection/preserve_type_op_constraints, r=BoxyUwU

trait_selection: Keep type-op region constraints in borrowck

I ran into this while working on #158588. `borrowck_env_fail` still had a FIXME because the function body wasn't reporting the outlives error. The next solver creates the region constraint, but the canonical type-op path doesn't put it in `QueryResponse`. Borrowck never sees it, so the type op looks fine and the error is lost.

With `-Zassumptions-on-binders`, these type ops now run locally on borrowck's `InferCtxt`. The fast path still runs first. That leaves the constraint in the same inference context borrowck reads later.

I prefer this over adding more data to the old canonical response. The next solver already caches its work, and teaching the old query path about these constraints felt like extra machinery for something we can avoid. Running locally is pretty boring, but I think that's a good thing here. The old FIXME is now the regression test.

Author: bors

compiler/rustc_borrowck/src/handle_placeholders.rs

@@ -246,8 +246,16 @@ pub(crate) fn compute_sccs_applying_placeholder_outlives_constraints<'tcx>(
         mut outlives_constraints,
         universe_causes,
         type_tests,
+        solver_constraints,
     } = constraints;
 
+    // These have already been destructured into `outlives_constraints` at the
+    // end of MIR type checking.
+    assert!(
+        solver_constraints.is_true(),
+        "solver region constraints not lowered to NLL = {solver_constraints:#?}",
+    );
+
     let fr_static = universal_regions.fr_static;
     let compute_sccs =
         |constraints: &OutlivesConstraintSet<'tcx>,

compiler/rustc_borrowck/src/type_check/constraint_conversion.rs

@@ -67,7 +67,8 @@ impl<'a, 'tcx> ConstraintConversion<'a, 'tcx> {
 
     #[instrument(skip(self), level = "debug")]
     pub(super) fn convert_all(&mut self, query_constraints: &QueryRegionConstraints<'tcx>) {
-        let QueryRegionConstraints { constraints, assumptions } = query_constraints;
+        let QueryRegionConstraints { constraints, assumptions, solver_constraints } =
+            query_constraints;
         let assumptions =
             elaborate::elaborate_outlives_assumptions(self.infcx.tcx, assumptions.iter().copied());
 
@@ -76,6 +77,9 @@ impl<'a, 'tcx> ConstraintConversion<'a, 'tcx> {
                 self.convert(predicate, category, &assumptions);
             });
         }
+
+        self.constraints
+            .register_solver_constraint(solver_constraints.clone().with_spans(self.span));
     }
 
     /// Given an instance of the closure type, this method instantiates the "extra" requirements

compiler/rustc_borrowck/src/type_check/mod.rs

@@ -17,6 +17,7 @@ use rustc_infer::infer::outlives::env::RegionBoundPairs;
 use rustc_infer::infer::region_constraints::RegionConstraintData;
 use rustc_infer::infer::{
     BoundRegionConversionTime, InferCtxt, NllRegionVariableOrigin, RegionVariableOrigin,
+    SolverRegionConstraint,
 };
 use rustc_infer::traits::{Obligation, ObligationCause, PredicateObligations};
 use rustc_middle::bug;
@@ -113,6 +114,7 @@ pub(crate) fn type_check<'tcx>(
         outlives_constraints: OutlivesConstraintSet::default(),
         type_tests: Vec::default(),
         universe_causes: FxIndexMap::default(),
+        solver_constraints: SolverRegionConstraint::new_true(),
     };
 
     let CreateResult {
@@ -134,6 +136,13 @@ pub(crate) fn type_check<'tcx>(
             pre_assumptions.is_empty(),
             "there should be no incoming region assumptions = {pre_assumptions:#?}",
         );
+        // Solver region constraints from computing the implied bounds went through
+        // `ConstraintConversion` and are already stored in `constraints`.
+        let pre_solver_constraints = infcx.take_solver_region_constraints();
+        assert!(
+            pre_solver_constraints.is_true(),
+            "there should be no incoming solver region constraints = {pre_solver_constraints:#?}",
+        );
     }
 
     debug!(?normalized_inputs_and_output);
@@ -174,6 +183,10 @@ pub(crate) fn type_check<'tcx>(
     let polonius_context = typeck.polonius_context;
 
     if infcx.tcx.assumptions_on_binders() {
+        let solver_constraints = mem::replace(
+            &mut typeck.constraints.solver_constraints,
+            SolverRegionConstraint::new_true(),
+        );
         let mut converter = constraint_conversion::ConstraintConversion::new(
             typeck.infcx,
             typeck.universal_regions,
@@ -185,6 +198,7 @@ pub(crate) fn type_check<'tcx>(
             typeck.constraints,
         );
         typeck.infcx.destructure_solver_region_constraints_for_borrowck(
+            solver_constraints,
             &mut converter,
             typeck.known_type_outlives_obligations,
             universal_region_relations.outlives.clone(),
@@ -293,9 +307,25 @@ pub(crate) struct MirTypeckRegionConstraints<'tcx> {
     pub(crate) universe_causes: FxIndexMap<ty::UniverseIndex, UniverseInfo<'tcx>>,
 
     pub(crate) type_tests: Vec<TypeTest<'tcx>>,
+
+    /// The region constraints emitted by the next solver under
+    /// `-Zassumptions-on-binders`. Unlike the constraints above these are not yet
+    /// lowered to NLL, we destructure them into `outlives_constraints` at the end
+    /// of MIR type checking.
+    pub(crate) solver_constraints: SolverRegionConstraint<'tcx>,
 }
 
 impl<'tcx> MirTypeckRegionConstraints<'tcx> {
+    /// Adds `constraint` to the constraints we've accumulated so far.
+    pub(crate) fn register_solver_constraint(&mut self, constraint: SolverRegionConstraint<'tcx>) {
+        // FIXME(-Zassumptions-on-binders): This is pretty bad for perf, we rebuild the
+        // entire constraint every time instead of updating it incrementally.
+        self.solver_constraints = SolverRegionConstraint::build_and(
+            constraint,
+            mem::replace(&mut self.solver_constraints, SolverRegionConstraint::new_true()),
+        );
+    }
+
     /// Creates a `Region` for a given `PlaceholderRegion`, or returns the
     /// region that corresponds to a previously created one.
     pub(crate) fn placeholder_region(

compiler/rustc_infer/src/infer/canonical/query_response.rs

@@ -146,11 +146,13 @@ impl<'tcx> InferCtxt<'tcx> {
         let region_obligations = self.take_registered_region_obligations();
         let region_assumptions = self.take_registered_region_assumptions();
         debug!(?region_obligations);
+        let solver_constraints = self.take_solver_region_constraints();
         let region_constraints = self.with_region_constraints(|region_constraints| {
             make_query_region_constraints(
                 region_obligations,
                 region_constraints,
                 region_assumptions,
+                solver_constraints,
             )
         });
         debug!(?region_constraints);
@@ -195,9 +197,10 @@ impl<'tcx> InferCtxt<'tcx> {
         let InferOk { value: result_args, obligations } =
             self.query_response_instantiation(cause, param_env, original_values, query_response)?;
 
-        for QueryRegionConstraint { constraint, visible_for_leak_check: vis, .. } in
-            &query_response.value.region_constraints.constraints
-        {
+        let QueryRegionConstraints { constraints, assumptions, solver_constraints } =
+            &query_response.value.region_constraints;
+
+        for QueryRegionConstraint { constraint, visible_for_leak_check: vis, .. } in constraints {
             let constraint = instantiate_value(self.tcx, &result_args, *constraint);
             match constraint {
                 ty::RegionConstraint::Outlives(clause) => {
@@ -209,11 +212,15 @@ impl<'tcx> InferCtxt<'tcx> {
             }
         }
 
-        for assumption in &query_response.value.region_constraints.assumptions {
+        for assumption in assumptions {
             let assumption = instantiate_value(self.tcx, &result_args, *assumption);
             self.register_region_assumption(assumption);
         }
 
+        let solver_constraints =
+            instantiate_value(self.tcx, &result_args, solver_constraints.clone());
+        self.register_solver_region_constraint(solver_constraints.with_spans(cause.span));
+
         let user_result: R =
             query_response.instantiate_projected(self.tcx, &result_args, |q_r| q_r.value.clone());
 
@@ -325,27 +332,31 @@ impl<'tcx> InferCtxt<'tcx> {
             }
         }
 
+        let QueryRegionConstraints { constraints, assumptions, solver_constraints } =
+            &query_response.value.region_constraints;
+
         // ...also include the other query region constraints from the query.
-        output_query_region_constraints.constraints.extend(
-            query_response.value.region_constraints.constraints.iter().filter_map(|&r_c| {
-                let r_c = instantiate_value(self.tcx, &result_args, r_c);
+        output_query_region_constraints.constraints.extend(constraints.iter().filter_map(|&r_c| {
+            let r_c = instantiate_value(self.tcx, &result_args, r_c);
 
-                // Screen out `'a: 'a` or `'a == 'a` cases.
-                if r_c.constraint.is_trivial() { None } else { Some(r_c) }
-            }),
-        );
+            // Screen out `'a: 'a` or `'a == 'a` cases.
+            if r_c.constraint.is_trivial() { None } else { Some(r_c) }
+        }));
 
         // FIXME(higher_ranked_auto): Optimize this to instantiate all assumptions
         // at once, rather than calling `instantiate_value` repeatedly which may
         // create more universes.
-        output_query_region_constraints.assumptions.extend(
-            query_response
-                .value
-                .region_constraints
-                .assumptions
-                .iter()
-                .map(|&r_c| instantiate_value(self.tcx, &result_args, r_c)),
-        );
+        output_query_region_constraints
+            .assumptions
+            .extend(assumptions.iter().map(|&r_c| instantiate_value(self.tcx, &result_args, r_c)));
+
+        let solver_constraints =
+            instantiate_value(self.tcx, &result_args, solver_constraints.clone());
+        output_query_region_constraints.solver_constraints =
+            ty::region_constraint::RegionConstraint::build_and(
+                std::mem::take(&mut output_query_region_constraints.solver_constraints),
+                solver_constraints,
+            );
 
         let user_result: R =
             query_response.instantiate_projected(self.tcx, &result_args, |q_r| q_r.value.clone());
@@ -619,6 +630,7 @@ pub fn make_query_region_constraints<'tcx>(
     outlives_obligations: Vec<TypeOutlivesConstraint<'tcx>>,
     region_constraints: &RegionConstraintData<'tcx>,
     assumptions: Vec<ty::ArgOutlivesClause<'tcx>>,
+    solver_constraints: ty::region_constraint::RegionConstraint<TyCtxt<'tcx>>,
 ) -> QueryRegionConstraints<'tcx> {
     let RegionConstraintData { constraints, verifys } = region_constraints;
 
@@ -663,5 +675,5 @@ pub fn make_query_region_constraints<'tcx>(
         ))
         .collect();
 
-    QueryRegionConstraints { constraints, assumptions }
+    QueryRegionConstraints { constraints, assumptions, solver_constraints }
 }

compiler/rustc_infer/src/infer/outlives/obligations.rs

@@ -238,11 +238,16 @@ impl<'tcx> InferCtxt<'tcx> {
             outlives_env.known_type_outlives().into_iter().cloned().collect(),
             outlives_env.free_region_map().relation.clone(),
         );
-        self.destructure_solver_region_constraints(assumptions, self);
+        let constraint = self.inner.borrow().solver_region_constraint_storage.get_constraint();
+        self.destructure_solver_region_constraints(constraint, assumptions, self);
     }
 
+    /// Unlike regionck, borrowck doesn't keep these constraints in the `InferCtxt`.
+    /// It stores them in `MirTypeckRegionConstraints` alongside its other region
+    /// constraints, so it hands us the constraint to destructure.
     pub fn destructure_solver_region_constraints_for_borrowck(
         &self,
+        constraint: SolverRegionConstraint<'tcx>,
         // this is always ConstraintConversion but lol
         conversion: impl TypeOutlivesDelegate<'tcx>,
         known_type_outlives: &[PolyTypeOutlivesClause<'tcx>],
@@ -252,19 +257,19 @@ impl<'tcx> InferCtxt<'tcx> {
             known_type_outlives.into_iter().cloned().collect(),
             region_outlives.maybe_map(|r| Some(Region::new_var(self.tcx, r))).unwrap(),
         );
-        self.destructure_solver_region_constraints(assumptions, conversion);
+        self.destructure_solver_region_constraints(constraint, assumptions, conversion);
     }
 
     #[instrument(level = "debug", skip(self, conversion))]
     pub fn destructure_solver_region_constraints(
         &self,
+        constraint: SolverRegionConstraint<'tcx>,
         assumptions: rustc_type_ir::region_constraint::Assumptions<TyCtxt<'tcx>>,
         mut conversion: impl TypeOutlivesDelegate<'tcx>,
     ) {
         assert!(self.tcx.assumptions_on_binders());
         assert!(self.next_trait_solver());
 
-        let constraint = self.inner.borrow().solver_region_constraint_storage.get_constraint();
         debug!(?constraint);
         let constraint = region_constraint::destructure_type_outlives_constraints_in_root(
             self,

compiler/rustc_infer/src/infer/solver_region_constraints.rs

@@ -1,9 +1,11 @@
 use rustc_middle::ty::TyCtxt;
 use rustc_span::Span;
+use rustc_type_ir::region_constraint::RegionConstraint;
 use tracing::instrument;
 
-pub type SolverRegionConstraint<'tcx> =
-    rustc_type_ir::region_constraint::RegionConstraint<TyCtxt<'tcx>, Span>;
+use super::InferCtxt;
+
+pub type SolverRegionConstraint<'tcx> = RegionConstraint<TyCtxt<'tcx>, Span>;
 
 #[derive(Clone, Debug)]
 pub(crate) struct SolverRegionConstraintStorage<'tcx>(SolverRegionConstraint<'tcx>);
@@ -17,11 +19,24 @@ impl<'tcx> SolverRegionConstraintStorage<'tcx> {
         self.0.clone()
     }
 
+    pub(crate) fn take(&mut self) -> SolverRegionConstraint<'tcx> {
+        core::mem::replace(&mut self.0, SolverRegionConstraint::new_true())
+    }
+
     #[instrument(level = "debug", skip(self))]
     pub(crate) fn overwrite(&mut self, constraint: SolverRegionConstraint<'tcx>) {
         self.0 = constraint;
     }
 }
 
+impl<'tcx> InferCtxt<'tcx> {
+    /// Trait queries just want to pass back the solver region constraints "as is",
+    /// mirroring `take_registered_region_obligations`.
+    pub fn take_solver_region_constraints(&self) -> RegionConstraint<TyCtxt<'tcx>> {
+        assert!(!self.in_snapshot(), "cannot take solver region constraints in a snapshot");
+        self.inner.borrow_mut().solver_region_constraint_storage.take().without_spans()
+    }
+}
+
 #[cfg(test)]
 mod tests;

compiler/rustc_infer/src/infer/solver_region_constraints/tests.rs

@@ -1,7 +1,29 @@
+use rustc_middle::infer::canonical::QueryRegionConstraints;
 use rustc_middle::ty::TyCtxt;
 use rustc_span::{BytePos, Span};
 use rustc_type_ir::region_constraint::{And, LeafRegionConstraint, Or};
 
+use super::{SolverRegionConstraint, SolverRegionConstraintStorage};
+
+#[test]
+fn true_constraint_keeps_query_response_empty() {
+    // Mirrors `register_solver_region_constraint`, which registers unconditionally:
+    // anding a trivially true constraint into an empty store has to leave the store
+    // trivially true, as the resulting query response would otherwise no longer be
+    // empty. This relies on `And`/`Or` being kept in canonical form.
+    let mut storage = SolverRegionConstraintStorage::<'static>::new();
+    storage.overwrite(SolverRegionConstraint::build_and(
+        SolverRegionConstraint::new_true(),
+        storage.get_constraint(),
+    ));
+
+    let constraints = QueryRegionConstraints {
+        solver_constraints: storage.get_constraint().without_spans(),
+        ..Default::default()
+    };
+    assert!(constraints.is_empty());
+}
+
 #[test]
 fn canonicalization_preserves_only_one_ambiguity() {
     let first = Span::with_root_ctxt(BytePos(1), BytePos(2));

compiler/rustc_middle/src/infer/canonical.rs

@@ -76,13 +76,21 @@ pub struct QueryResponse<'tcx, R> {
     pub value: R,
 }
 
-#[derive(Clone, Debug, Default, PartialEq, Eq, Hash)]
+#[derive(Clone, Debug, Default, PartialEq, Hash)]
 #[derive(StableHash, TypeFoldable, TypeVisitable)]
 pub struct QueryRegionConstraints<'tcx> {
     pub constraints: Vec<QueryRegionConstraint<'tcx>>,
     pub assumptions: Vec<ty::ArgOutlivesClause<'tcx>>,
+    /// Region constraints emitted by the next solver under
+    /// `-Zassumptions-on-binders`.
+    ///
+    /// These stay unspanned while passing through a canonical query. The type-op
+    /// caller attaches its origin span when consuming the response.
+    pub solver_constraints: ir::region_constraint::RegionConstraint<TyCtxt<'tcx>>,
 }
 
+impl Eq for QueryRegionConstraints<'_> {}
+
 impl QueryRegionConstraints<'_> {
     /// Represents an empty (trivially true) set of region constraints.
     ///
@@ -91,8 +99,23 @@ impl QueryRegionConstraints<'_> {
     /// discharge a requirement from another query, which is a potential problem if we did throw
     /// away these assumptions because there were no constraints.
     pub fn is_empty(&self) -> bool {
-        let QueryRegionConstraints { constraints, assumptions } = self;
-        constraints.is_empty() && assumptions.is_empty()
+        let QueryRegionConstraints { constraints, assumptions, solver_constraints } = self;
+        constraints.is_empty() && assumptions.is_empty() && solver_constraints.is_true()
+    }
+
+    pub fn extend(&mut self, other: &Self) {
+        let QueryRegionConstraints { constraints, assumptions, solver_constraints } = self;
+        let QueryRegionConstraints {
+            constraints: other_constraints,
+            assumptions: other_assumptions,
+            solver_constraints: other_solver_constraints,
+        } = other;
+        constraints.extend(other_constraints.iter().cloned());
+        assumptions.extend(other_assumptions.iter().cloned());
+        *solver_constraints = ir::region_constraint::RegionConstraint::build_and(
+            std::mem::take(solver_constraints),
+            other_solver_constraints.clone(),
+        );
     }
 }
 

compiler/rustc_trait_selection/src/solve/delegate.rs

@@ -374,6 +374,11 @@ impl<'tcx> rustc_next_trait_solver::delegate::SolverDelegate for SolverDelegate<
                 region_obligations,
                 region_constraints,
                 region_assumptions,
+                // We're only called with `-Zassumptions-on-binders` disabled, in which
+                // case the solver never emits new-style region constraints. With it
+                // enabled the solver instead returns `ExternalRegionConstraints::NextGen`,
+                // reading the constraint straight out of the `InferCtxt`.
+                Default::default(),
             )
         });