Rollup merge of #107348 - lcnr:project-solve-new, r=compiler-errors

small refactor to new projection code

extract `eq_term_and_make_canonical_response` into a helper function which also is another guarantee that the expected term does not influence candidate selection for projections.

also change `evaluate_all(vec![single_goal])` to use `evaluate_goal`.

the second commit now also adds a `debug_assert!` to `evaluate_goal`.

Author: GuillaumeGomez

compiler/rustc_trait_selection/src/solve/mod.rs

@@ -161,6 +161,7 @@ impl<'tcx> InferCtxtEvalExt<'tcx> for InferCtxt<'tcx> {
             search_graph: &mut search_graph,
             infcx: self,
             var_values: CanonicalVarValues::dummy(),
+            in_projection_eq_hack: false,
         }
         .evaluate_goal(goal);
 
@@ -174,6 +175,10 @@ struct EvalCtxt<'a, 'tcx> {
     var_values: CanonicalVarValues<'tcx>,
 
     search_graph: &'a mut search_graph::SearchGraph<'tcx>,
+
+    /// This field is used by a debug assertion in [`EvalCtxt::evaluate_goal`],
+    /// see the comment in that method for more details.
+    in_projection_eq_hack: bool,
 }
 
 impl<'a, 'tcx> EvalCtxt<'a, 'tcx> {
@@ -209,7 +214,8 @@ impl<'a, 'tcx> EvalCtxt<'a, 'tcx> {
         loop {
             let (ref infcx, goal, var_values) =
                 tcx.infer_ctxt().build_with_canonical(DUMMY_SP, &canonical_goal);
-            let mut ecx = EvalCtxt { infcx, var_values, search_graph };
+            let mut ecx =
+                EvalCtxt { infcx, var_values, search_graph, in_projection_eq_hack: false };
             let result = ecx.compute_goal(goal);
 
             // FIXME: `Response` should be `Copy`
@@ -239,10 +245,28 @@ impl<'a, 'tcx> EvalCtxt<'a, 'tcx> {
         let canonical_goal = self.infcx.canonicalize_query(goal, &mut orig_values);
         let canonical_response =
             EvalCtxt::evaluate_canonical_goal(self.tcx(), self.search_graph, canonical_goal)?;
-        Ok((
-            !canonical_response.value.var_values.is_identity(),
-            instantiate_canonical_query_response(self.infcx, &orig_values, canonical_response),
-        ))
+
+        let has_changed = !canonical_response.value.var_values.is_identity();
+        let certainty =
+            instantiate_canonical_query_response(self.infcx, &orig_values, canonical_response);
+
+        // Check that rerunning this query with its inference constraints applied
+        // doesn't result in new inference constraints and has the same result.
+        //
+        // If we have projection goals like `<T as Trait>::Assoc == u32` we recursively
+        // call `exists<U> <T as Trait>::Assoc == U` to enable better caching. This goal
+        // could constrain `U` to `u32` which would cause this check to result in a
+        // solver cycle.
+        if cfg!(debug_assertions) && has_changed && !self.in_projection_eq_hack {
+            let mut orig_values = OriginalQueryValues::default();
+            let canonical_goal = self.infcx.canonicalize_query(goal, &mut orig_values);
+            let canonical_response =
+                EvalCtxt::evaluate_canonical_goal(self.tcx(), self.search_graph, canonical_goal)?;
+            assert!(canonical_response.value.var_values.is_identity());
+            assert_eq!(certainty, canonical_response.value.certainty);
+        }
+
+        Ok((has_changed, certainty))
     }
 
     fn compute_goal(&mut self, goal: Goal<'tcx, ty::Predicate<'tcx>>) -> QueryResult<'tcx> {

compiler/rustc_trait_selection/src/solve/project_goals.rs

@@ -45,8 +45,9 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
                 projection_ty: goal.predicate.projection_ty,
                 term: unconstrained_rhs,
             });
-            let (_has_changed, normalize_certainty) =
-                self.evaluate_goal(goal.with(self.tcx(), unconstrained_predicate))?;
+            let (_has_changed, normalize_certainty) = self.in_projection_eq_hack(|this| {
+                this.evaluate_goal(goal.with(this.tcx(), unconstrained_predicate))
+            })?;
 
             let nested_eq_goals =
                 self.infcx.eq(goal.param_env, unconstrained_rhs, predicate.term)?;
@@ -55,6 +56,15 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
         }
     }
 
+    /// This sets a flag used by a debug assert in [`EvalCtxt::evaluate_goal`],
+    /// see the comment in that method for more details.
+    fn in_projection_eq_hack<T>(&mut self, f: impl FnOnce(&mut Self) -> T) -> T {
+        self.in_projection_eq_hack = true;
+        let result = f(self);
+        self.in_projection_eq_hack = false;
+        result
+    }
+
     /// Is the projection predicate is of the form `exists<T> <Ty as Trait>::Assoc = T`.
     ///
     /// This is the case if the `term` is an inference variable in the innermost universe
@@ -122,6 +132,28 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
             && goal.param_env.visit_with(&mut visitor).is_continue()
     }
 
+    /// After normalizing the projection to `normalized_alias` with the given
+    /// `normalization_certainty`, constrain the inference variable `term` to it
+    /// and return a query response.
+    fn eq_term_and_make_canonical_response(
+        &mut self,
+        goal: Goal<'tcx, ProjectionPredicate<'tcx>>,
+        normalization_certainty: Certainty,
+        normalized_alias: impl Into<ty::Term<'tcx>>,
+    ) -> QueryResult<'tcx> {
+        // The term of our goal should be fully unconstrained, so this should never fail.
+        //
+        // It can however be ambiguous when the `normalized_alias` contains a projection.
+        let nested_goals = self
+            .infcx
+            .eq(goal.param_env, goal.predicate.term, normalized_alias.into())
+            .expect("failed to unify with unconstrained term");
+        let rhs_certainty =
+            self.evaluate_all(nested_goals).expect("failed to unify with unconstrained term");
+
+        self.make_canonical_response(normalization_certainty.unify_and(rhs_certainty))
+    }
+
     fn merge_project_candidates(
         &mut self,
         mut candidates: Vec<Candidate<'tcx>>,
@@ -218,7 +250,7 @@ impl<'tcx> assembly::GoalKind<'tcx> for ProjectionPredicate<'tcx> {
                 .map(|pred| goal.with(tcx, pred));
 
             nested_goals.extend(where_clause_bounds);
-            let trait_ref_certainty = ecx.evaluate_all(nested_goals)?;
+            let match_impl_certainty = ecx.evaluate_all(nested_goals)?;
 
             // In case the associated item is hidden due to specialization, we have to
             // return ambiguity this would otherwise be incomplete, resulting in
@@ -230,7 +262,7 @@ impl<'tcx> assembly::GoalKind<'tcx> for ProjectionPredicate<'tcx> {
                 goal.predicate.def_id(),
                 impl_def_id
             )? else {
-                return ecx.make_canonical_response(trait_ref_certainty.unify_and(Certainty::AMBIGUOUS));
+                return ecx.make_canonical_response(match_impl_certainty.unify_and(Certainty::AMBIGUOUS));
             };
 
             if !assoc_def.item.defaultness(tcx).has_value() {
@@ -277,17 +309,7 @@ impl<'tcx> assembly::GoalKind<'tcx> for ProjectionPredicate<'tcx> {
                 ty.map_bound(|ty| ty.into())
             };
 
-            // The term of our goal should be fully unconstrained, so this should never fail.
-            //
-            // It can however be ambiguous when the resolved type is a projection.
-            let nested_goals = ecx
-                .infcx
-                .eq(goal.param_env, goal.predicate.term, term.subst(tcx, substs))
-                .expect("failed to unify with unconstrained term");
-            let rhs_certainty =
-                ecx.evaluate_all(nested_goals).expect("failed to unify with unconstrained term");
-
-            ecx.make_canonical_response(trait_ref_certainty.unify_and(rhs_certainty))
+            ecx.eq_term_and_make_canonical_response(goal, match_impl_certainty, term.subst(tcx, substs))
         })
     }
 
@@ -309,18 +331,11 @@ impl<'tcx> assembly::GoalKind<'tcx> for ProjectionPredicate<'tcx> {
                 )?;
                 let subst_certainty = ecx.evaluate_all(nested_goals)?;
 
-                // The term of our goal should be fully unconstrained, so this should never fail.
-                //
-                // It can however be ambiguous when the resolved type is a projection.
-                let nested_goals = ecx
-                    .infcx
-                    .eq(goal.param_env, goal.predicate.term, assumption_projection_pred.term)
-                    .expect("failed to unify with unconstrained term");
-                let rhs_certainty = ecx
-                    .evaluate_all(nested_goals)
-                    .expect("failed to unify with unconstrained term");
-
-                ecx.make_canonical_response(subst_certainty.unify_and(rhs_certainty))
+                ecx.eq_term_and_make_canonical_response(
+                    goal,
+                    subst_certainty,
+                    assumption_projection_pred.term,
+                )
             })
         } else {
             Err(NoSolution)
@@ -437,14 +452,12 @@ impl<'tcx> assembly::GoalKind<'tcx> for ProjectionPredicate<'tcx> {
                         [ty::GenericArg::from(goal.predicate.self_ty())],
                     ));
 
-                    let mut nested_goals = ecx.infcx.eq(
-                        goal.param_env,
-                        goal.predicate.term.ty().unwrap(),
+                    let is_sized_certainty = ecx.evaluate_goal(goal.with(tcx, sized_predicate))?.1;
+                    return ecx.eq_term_and_make_canonical_response(
+                        goal,
+                        is_sized_certainty,
                         tcx.types.unit,
-                    )?;
-                    nested_goals.push(goal.with(tcx, sized_predicate));
-
-                    return ecx.evaluate_all_and_make_canonical_response(nested_goals);
+                    );
                 }
 
                 ty::Adt(def, substs) if def.is_struct() => {
@@ -456,7 +469,8 @@ impl<'tcx> assembly::GoalKind<'tcx> for ProjectionPredicate<'tcx> {
                                 tcx,
                                 ty::Binder::dummy(goal.predicate.with_self_ty(tcx, self_ty)),
                             );
-                            return ecx.evaluate_all_and_make_canonical_response(vec![new_goal]);
+                            let (_, certainty) = ecx.evaluate_goal(new_goal)?;
+                            return ecx.make_canonical_response(certainty);
                         }
                     }
                 }
@@ -469,7 +483,8 @@ impl<'tcx> assembly::GoalKind<'tcx> for ProjectionPredicate<'tcx> {
                             tcx,
                             ty::Binder::dummy(goal.predicate.with_self_ty(tcx, self_ty)),
                         );
-                        return ecx.evaluate_all_and_make_canonical_response(vec![new_goal]);
+                        let (_, certainty) = ecx.evaluate_goal(new_goal)?;
+                        return ecx.make_canonical_response(certainty);
                     }
                 },
 
@@ -482,9 +497,7 @@ impl<'tcx> assembly::GoalKind<'tcx> for ProjectionPredicate<'tcx> {
                 ),
             };
 
-            let nested_goals =
-                ecx.infcx.eq(goal.param_env, goal.predicate.term.ty().unwrap(), metadata_ty)?;
-            ecx.evaluate_all_and_make_canonical_response(nested_goals)
+            ecx.eq_term_and_make_canonical_response(goal, Certainty::Yes, metadata_ty)
         })
     }
 

compiler/rustc_trait_selection/src/solve/search_graph/mod.rs

@@ -45,6 +45,7 @@ impl<'tcx> SearchGraph<'tcx> {
     /// Tries putting the new goal on the stack, returning an error if it is already cached.
     ///
     /// This correctly updates the provisional cache if there is a cycle.
+    #[instrument(level = "debug", skip(self, tcx), ret)]
     pub(super) fn try_push_stack(
         &mut self,
         tcx: TyCtxt<'tcx>,
@@ -79,8 +80,10 @@ impl<'tcx> SearchGraph<'tcx> {
             Entry::Occupied(entry_index) => {
                 let entry_index = *entry_index.get();
 
-                cache.add_dependency_of_leaf_on(entry_index);
                 let stack_depth = cache.depth(entry_index);
+                debug!("encountered cycle with depth {stack_depth:?}");
+
+                cache.add_dependency_of_leaf_on(entry_index);
 
                 self.stack[stack_depth].has_been_used = true;
                 // NOTE: The goals on the stack aren't the only goals involved in this cycle.
@@ -117,6 +120,7 @@ impl<'tcx> SearchGraph<'tcx> {
     /// updated the provisional cache and we have to recompute the current goal.
     ///
     /// FIXME: Refer to the rustc-dev-guide entry once it exists.
+    #[instrument(level = "debug", skip(self, tcx, actual_goal), ret)]
     pub(super) fn try_finalize_goal(
         &mut self,
         tcx: TyCtxt<'tcx>,