Rollup merge of rust-lang#108896 - BoxyUwU:new_solver_add_goal_fn, r=lcnr

new solver: make all goal evaluation able to be automatically rerun

It is generally wrong to call `evaluate_goal` multiple times or `evaluate_goal` and `evaluate_all` for the same `QueryResult` without correctly handling rerunning the goals when inference makes progress. Not doing so will result in the assertion in `evaluate_goal` firing because rerunning the goal will lead to a more accurate `QueryResult`.

Currently there are lots of places that get this wrong and generally it is complex and error prone to handle correctly everywhere. This PR introduces a way to add goals to the `EvalCtxt` and then run all the added goals in a loop so that `evaluate_goal`/`evaluate_all` is not necessary to call manually.

There are a few complications for making everything work "right":
1. the `normalizes-to` hack that replaces the rhs with an unconstrained infer var requires special casing in the new `try_evaluate_added_goals` function similar to how `evaluate_goal`'s assertion special cases that hack.

2. `assemble_candidates_after_normalizing_self_ty`'s normalization step needs to be reran for each candidate otherwise the found candidates will potentially get a more accurate `QueryResult` when rerunning the projection/trait goal which can effect the `QueryResult` of the projection/trait goal.
This is implemented via `EvalCtxt::probe`'s closure's `EvalCtxt` inheriting the added goals of the `EvalCtxt` that `probe` is called on, allowing us to add goals in a probe, and then enter a nested probe for each candidate and evaluate added goals which include the normalization step's goals.

I made `make_canonical_response` evaluate added goals so that it will be hard to mess up the impl of the solver by forgetting to evaluate added goals. Right now the only way to mess this up would be to call `response_no_constraints` (which from the name is obviously weird).

The visibility of `evaluate_goal` means that it can be called from various `compute_x_goal` or candidate assembly functions, this is generally wrong and we should never call `evaluate_goal` manually, instead we should be calling `add_goal`/`add_goals`. This is solved by moving `evaluate_goal` `evaluate_canonical_goal` and `compute_goal` into `eval_ctxt`'s module and making them private so they cannot be called from elsewhere, forcing people to call `add_goal/s` and `evaluate_added_goals_and_make_canonical_resposne`/`try_evaluate_added_goals`

---

Other changes:
- removed the `&& false` that was introduced to the assertion in `evaluate_goal` in rust-lang#108839
- remove a `!self.did_overflow()` requirement in `search_graph.is_empty()` which causes goals that overflow to ICE
- made `EvalCtxt::eq` take `&mut self` and add all the nested goals via `add_goals` instead of returning them as 99% of call sites just immediately called `EvalCtxt::add_goals` manually.

r? `@lcnr`

Author: Dylan-DPC

compiler/rustc_trait_selection/src/solve/assembly.rs

@@ -224,7 +224,9 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
         if goal.predicate.self_ty().is_ty_var() {
             return vec![Candidate {
                 source: CandidateSource::BuiltinImpl,
-                result: self.make_canonical_response(Certainty::AMBIGUOUS).unwrap(),
+                result: self
+                    .evaluate_added_goals_and_make_canonical_response(Certainty::AMBIGUOUS)
+                    .unwrap(),
             }];
         }
 
@@ -261,37 +263,26 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
         let &ty::Alias(ty::Projection, projection_ty) = goal.predicate.self_ty().kind() else {
             return
         };
-        self.probe(|this| {
-            let normalized_ty = this.next_ty_infer();
+
+        self.probe(|ecx| {
+            let normalized_ty = ecx.next_ty_infer();
             let normalizes_to_goal = goal.with(
                 tcx,
                 ty::Binder::dummy(ty::ProjectionPredicate {
                     projection_ty,
                     term: normalized_ty.into(),
                 }),
             );
-            let normalization_certainty = match this.evaluate_goal(normalizes_to_goal) {
-                Ok((_, certainty)) => certainty,
-                Err(NoSolution) => return,
-            };
-            let normalized_ty = this.resolve_vars_if_possible(normalized_ty);
-
-            // NOTE: Alternatively we could call `evaluate_goal` here and only have a `Normalized` candidate.
-            // This doesn't work as long as we use `CandidateSource` in winnowing.
-            let goal = goal.with(tcx, goal.predicate.with_self_ty(tcx, normalized_ty));
-            let normalized_candidates = this.assemble_and_evaluate_candidates(goal);
-            for mut normalized_candidate in normalized_candidates {
-                normalized_candidate.result =
-                    normalized_candidate.result.unchecked_map(|mut response| {
-                        // FIXME: This currently hides overflow in the normalization step of the self type
-                        // which is probably wrong. Maybe `unify_and` should actually keep overflow as
-                        // we treat it as non-fatal anyways.
-                        response.certainty = response.certainty.unify_and(normalization_certainty);
-                        response
-                    });
-                candidates.push(normalized_candidate);
+            ecx.add_goal(normalizes_to_goal);
+            if let Ok(_) = ecx.try_evaluate_added_goals() {
+                let normalized_ty = ecx.resolve_vars_if_possible(normalized_ty);
+
+                // NOTE: Alternatively we could call `evaluate_goal` here and only have a `Normalized` candidate.
+                // This doesn't work as long as we use `CandidateSource` in winnowing.
+                let goal = goal.with(tcx, goal.predicate.with_self_ty(tcx, normalized_ty));
+                candidates.extend(ecx.assemble_and_evaluate_candidates(goal));
             }
-        })
+        });
     }
 
     fn assemble_impl_candidates<G: GoalKind<'tcx>>(
@@ -516,7 +507,7 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
                 } else {
                     Certainty::AMBIGUOUS
                 };
-                return self.make_canonical_response(certainty);
+                return self.evaluate_added_goals_and_make_canonical_response(certainty);
             }
         }
 
@@ -538,14 +529,16 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
         }
     }
 
-    fn discard_reservation_impl(&self, mut candidate: Candidate<'tcx>) -> Candidate<'tcx> {
+    fn discard_reservation_impl(&mut self, mut candidate: Candidate<'tcx>) -> Candidate<'tcx> {
         if let CandidateSource::Impl(def_id) = candidate.source {
             if let ty::ImplPolarity::Reservation = self.tcx().impl_polarity(def_id) {
                 debug!("Selected reservation impl");
                 // We assemble all candidates inside of a probe so by
                 // making a new canonical response here our result will
                 // have no constraints.
-                candidate.result = self.make_canonical_response(Certainty::AMBIGUOUS).unwrap();
+                candidate.result = self
+                    .evaluate_added_goals_and_make_canonical_response(Certainty::AMBIGUOUS)
+                    .unwrap();
             }
         }
 

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

@@ -48,7 +48,13 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
     /// - `external_constraints`: additional constraints which aren't expressable
     ///   using simple unification of inference variables.
     #[instrument(level = "debug", skip(self))]
-    pub(super) fn make_canonical_response(&self, certainty: Certainty) -> QueryResult<'tcx> {
+    pub(super) fn evaluate_added_goals_and_make_canonical_response(
+        &mut self,
+        certainty: Certainty,
+    ) -> QueryResult<'tcx> {
+        let goals_certainty = self.try_evaluate_added_goals()?;
+        let certainty = certainty.unify_and(goals_certainty);
+
         let external_constraints = self.compute_external_query_constraints()?;
 
         let response = Response { var_values: self.var_values, external_constraints, certainty };
@@ -209,7 +215,7 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
             // FIXME: To deal with #105787 I also expect us to emit nested obligations here at
             // some point. We can figure out how to deal with this once we actually have
             // an ICE.
-            let nested_goals = self.eq(param_env, orig, response)?;
+            let nested_goals = self.eq_and_get_goals(param_env, orig, response)?;
             assert!(nested_goals.is_empty(), "{nested_goals:?}");
         }