bye bye assemble_candidates_via_self_ty

Author: lcnr

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

@@ -271,64 +271,32 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
         &mut self,
         goal: Goal<'tcx, G>,
     ) -> Vec<Candidate<'tcx>> {
-        debug_assert_eq!(goal, self.resolve_vars_if_possible(goal));
-        if let Some(ambig) = self.assemble_self_ty_infer_ambiguity_response(goal) {
-            return vec![ambig];
-        }
-
-        let mut candidates = self.assemble_candidates_via_self_ty(goal, 0);
-
-        self.assemble_unsize_to_dyn_candidate(goal, &mut candidates);
-
-        self.assemble_blanket_impl_candidates(goal, &mut candidates);
-
-        self.assemble_param_env_candidates(goal, &mut candidates);
-
-        self.assemble_coherence_unknowable_candidates(goal, &mut candidates);
-
-        candidates
-    }
-
-    /// `?0: Trait` is ambiguous, because it may be satisfied via a builtin rule,
-    /// object bound, alias bound, etc. We are unable to determine this until we can at
-    /// least structurally resolve the type one layer.
-    ///
-    /// It would also require us to consider all impls of the trait, which is both pretty
-    /// bad for perf and would also constrain the self type if there is just a single impl.
-    fn assemble_self_ty_infer_ambiguity_response<G: GoalKind<'tcx>>(
-        &mut self,
-        goal: Goal<'tcx, G>,
-    ) -> Option<Candidate<'tcx>> {
-        if goal.predicate.self_ty().is_ty_var() {
-            debug!("adding self_ty_infer_ambiguity_response");
+        let dummy_candidate = |this: &mut EvalCtxt<'_, 'tcx>, certainty| {
             let source = CandidateSource::BuiltinImpl(BuiltinImplSource::Misc);
-            let result = self
-                .evaluate_added_goals_and_make_canonical_response(Certainty::AMBIGUOUS)
-                .unwrap();
-            let mut dummy_probe = self.inspect.new_probe();
+            let result = this.evaluate_added_goals_and_make_canonical_response(certainty).unwrap();
+            let mut dummy_probe = this.inspect.new_probe();
             dummy_probe.probe_kind(ProbeKind::TraitCandidate { source, result: Ok(result) });
-            self.inspect.finish_probe(dummy_probe);
-            Some(Candidate { source, result })
-        } else {
-            None
+            this.inspect.finish_probe(dummy_probe);
+            vec![Candidate { source, result }]
+        };
+
+        let Some(normalized_self_ty) =
+            self.try_normalize_ty(goal.param_env, goal.predicate.self_ty())
+        else {
+            debug!("overflow while evaluating self type");
+            return dummy_candidate(self, Certainty::OVERFLOW);
+        };
+
+        if normalized_self_ty.is_ty_var() {
+            debug!("self type has been normalized to infer");
+            return dummy_candidate(self, Certainty::AMBIGUOUS);
         }
-    }
 
-    /// Assemble candidates which apply to the self type. This only looks at candidate which
-    /// apply to the specific self type and ignores all others.
-    ///
-    /// Returns `None` if the self type is still ambiguous.
-    fn assemble_candidates_via_self_ty<G: GoalKind<'tcx>>(
-        &mut self,
-        goal: Goal<'tcx, G>,
-        num_steps: usize,
-    ) -> Vec<Candidate<'tcx>> {
+        let goal =
+            goal.with(self.tcx(), goal.predicate.with_self_ty(self.tcx(), normalized_self_ty));
         debug_assert_eq!(goal, self.resolve_vars_if_possible(goal));
-        if let Some(ambig) = self.assemble_self_ty_infer_ambiguity_response(goal) {
-            return vec![ambig];
-        }
 
-        let mut candidates = Vec::new();
+        let mut candidates = vec![];
 
         self.assemble_non_blanket_impl_candidates(goal, &mut candidates);
 
@@ -338,61 +306,15 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
 
         self.assemble_object_bound_candidates(goal, &mut candidates);
 
-        self.assemble_candidates_after_normalizing_self_ty(goal, &mut candidates, num_steps);
-        candidates
-    }
+        self.assemble_unsize_to_dyn_candidate(goal, &mut candidates);
 
-    /// If the self type of a goal is an alias we first try to normalize the self type
-    /// and compute the candidates for the normalized self type in case that succeeds.
-    ///
-    /// These candidates are used in addition to the ones with the alias as a self type.
-    /// We do this to simplify both builtin candidates and for better performance.
-    ///
-    /// We generate the builtin candidates on the fly by looking at the self type, e.g.
-    /// add `FnPtr` candidates if the self type is a function pointer. Handling builtin
-    /// candidates while the self type is still an alias seems difficult. This is similar
-    /// to `try_structurally_resolve_type` during hir typeck (FIXME once implemented).
-    ///
-    /// Looking at all impls for some trait goal is prohibitively expensive. We therefore
-    /// only look at implementations with a matching self type. Because of this function,
-    /// we can avoid looking at all existing impls if the self type is an alias.
-    #[instrument(level = "debug", skip_all)]
-    fn assemble_candidates_after_normalizing_self_ty<G: GoalKind<'tcx>>(
-        &mut self,
-        goal: Goal<'tcx, G>,
-        candidates: &mut Vec<Candidate<'tcx>>,
-        num_steps: usize,
-    ) {
-        let tcx = self.tcx();
-        let &ty::Alias(_, alias) = goal.predicate.self_ty().kind() else { return };
-
-        candidates.extend(self.probe(|_| ProbeKind::NormalizedSelfTyAssembly).enter(|ecx| {
-            if tcx.recursion_limit().value_within_limit(num_steps) {
-                let normalized_ty = ecx.next_ty_infer();
-                let normalizes_to_goal =
-                    goal.with(tcx, ty::NormalizesTo { alias, term: normalized_ty.into() });
-                ecx.add_goal(GoalSource::Misc, normalizes_to_goal);
-                if let Err(NoSolution) = ecx.try_evaluate_added_goals() {
-                    debug!("self type normalization failed");
-                    return vec![];
-                }
-                let normalized_ty = ecx.resolve_vars_if_possible(normalized_ty);
-                debug!(?normalized_ty, "self type normalized");
-                // 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));
-                ecx.assemble_candidates_via_self_ty(goal, num_steps + 1)
-            } else {
-                match ecx.evaluate_added_goals_and_make_canonical_response(Certainty::OVERFLOW) {
-                    Ok(result) => vec![Candidate {
-                        source: CandidateSource::BuiltinImpl(BuiltinImplSource::Misc),
-                        result,
-                    }],
-                    Err(NoSolution) => vec![],
-                }
-            }
-        }));
+        self.assemble_blanket_impl_candidates(goal, &mut candidates);
+
+        self.assemble_param_env_candidates(goal, &mut candidates);
+
+        self.assemble_coherence_unknowable_candidates(goal, &mut candidates);
+
+        candidates
     }
 
     #[instrument(level = "debug", skip_all)]

compiler/rustc_trait_selection/src/solve/mod.rs

@@ -288,11 +288,9 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
 
     /// Normalize a type when it is structually matched on.
     ///
-    /// For self types this is generally already handled through
-    /// `assemble_candidates_after_normalizing_self_ty`, so anything happening
-    /// in [`EvalCtxt::assemble_candidates_via_self_ty`] does not have to normalize
-    /// the self type. It is required when structurally matching on any other
-    /// arguments of a trait goal, e.g. when assembling builtin unsize candidates.
+    /// In nearly all cases this function must be used before matching on a type.
+    /// Not doing so is likely to be incomplete and therefore unsound during
+    /// coherence.
     #[instrument(level = "debug", skip(self), ret)]
     fn try_normalize_ty(
         &mut self,

tests/ui/traits/next-solver/alias-bound-unsound.rs

@@ -16,7 +16,7 @@ trait Foo {
 
 impl Foo for () {
     type Item = String where String: Copy;
-    //~^ ERROR overflow evaluating the requirement `<() as Foo>::Item: Copy`
+    //~^ ERROR overflow evaluating the requirement `String: Copy`
 }
 
 fn main() {

tests/ui/traits/next-solver/alias-bound-unsound.stderr

@@ -1,15 +1,17 @@
-error[E0275]: overflow evaluating the requirement `<() as Foo>::Item: Copy`
-  --> $DIR/alias-bound-unsound.rs:18:17
+error[E0275]: overflow evaluating the requirement `String: Copy`
+  --> $DIR/alias-bound-unsound.rs:18:38
    |
 LL |     type Item = String where String: Copy;
-   |                 ^^^^^^
+   |                                      ^^^^
    |
    = help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`alias_bound_unsound`)
-note: required by a bound in `Foo::Item`
-  --> $DIR/alias-bound-unsound.rs:8:16
+note: the requirement `String: Copy` appears on the `impl`'s associated type `Item` but not on the corresponding trait's associated type
+  --> $DIR/alias-bound-unsound.rs:8:10
    |
+LL | trait Foo {
+   |       --- in this trait
 LL |     type Item: Copy
-   |                ^^^^ required by this bound in `Foo::Item`
+   |          ^^^^ this trait's associated type doesn't have the requirement `String: Copy`
 
 error[E0275]: overflow evaluating the requirement `String <: <() as Foo>::Item`
   --> $DIR/alias-bound-unsound.rs:24:31

tests/ui/traits/next-solver/assembly/ambig-projection-self-is-ambig.rs

@@ -0,0 +1,19 @@
+// check-pass
+// compile-flags: -Znext-solver
+
+trait Reader: Default {
+    fn read_u8_array<A>(&self) -> Result<A, ()> {
+        todo!()
+    }
+
+    fn read_u8(&self) -> Result<u8, ()> {
+        let a: [u8; 1] = self.read_u8_array::<_>()?;
+        // This results in a nested `<Result<?0, ()> as Try>::Residual: Sized` goal.
+        // The self type normalizes to `?0`. We previously did not force that to be
+        // ambiguous but instead incompletely applied the `Self: Sized` candidate
+        // from the `ParamEnv`, resulting in a type error.
+        Ok(a[0])
+    }
+}
+
+fn main() {}

tests/ui/traits/next-solver/dont-type_of-tait-in-defining-scope.is_send.stderr

@@ -0,0 +1,16 @@
+error[E0283]: type annotations needed: cannot satisfy `Foo: Send`
+  --> $DIR/dont-type_of-tait-in-defining-scope.rs:15:18
+   |
+LL |     needs_send::<Foo>();
+   |                  ^^^
+   |
+   = note: cannot satisfy `Foo: Send`
+note: required by a bound in `needs_send`
+  --> $DIR/dont-type_of-tait-in-defining-scope.rs:12:18
+   |
+LL | fn needs_send<T: Send>() {}
+   |                  ^^^^ required by this bound in `needs_send`
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0283`.

tests/ui/traits/next-solver/dont-type_of-tait-in-defining-scope.not_send.stderr

@@ -1,12 +1,12 @@
 error[E0283]: type annotations needed: cannot satisfy `Foo: Send`
-  --> $DIR/dont-type_of-tait-in-defining-scope.rs:16:18
+  --> $DIR/dont-type_of-tait-in-defining-scope.rs:15:18
    |
 LL |     needs_send::<Foo>();
    |                  ^^^
    |
    = note: cannot satisfy `Foo: Send`
 note: required by a bound in `needs_send`
-  --> $DIR/dont-type_of-tait-in-defining-scope.rs:13:18
+  --> $DIR/dont-type_of-tait-in-defining-scope.rs:12:18
    |
 LL | fn needs_send<T: Send>() {}
    |                  ^^^^ required by this bound in `needs_send`

tests/ui/traits/next-solver/dont-type_of-tait-in-defining-scope.rs

@@ -1,6 +1,5 @@
 // revisions: is_send not_send
 // compile-flags: -Znext-solver
-//[is_send] check-pass
 
 #![feature(type_alias_impl_trait)]
 
@@ -14,7 +13,7 @@ fn needs_send<T: Send>() {}
 
 fn test(_: Foo) {
     needs_send::<Foo>();
-    //[not_send]~^ ERROR type annotations needed: cannot satisfy `Foo: Send`
+    //~^ ERROR type annotations needed: cannot satisfy `Foo: Send`
 }
 
 fn defines(_: Foo) {

tests/ui/traits/next-solver/overflow/recursive-self-normalization-2.rs

@@ -17,6 +17,7 @@ fn test<T: Foo1<Assoc1 = <T as Foo2>::Assoc2> + Foo2<Assoc2 = <T as Foo1>::Assoc
     //~| ERROR overflow evaluating the requirement `<T as Foo1>::Assoc1 == _`
     //~| ERROR overflow evaluating the requirement `<T as Foo1>::Assoc1 == _`
     //~| ERROR overflow evaluating the requirement `<T as Foo1>::Assoc1 == _`
+    //~| ERROR overflow evaluating the requirement `<T as Foo1>::Assoc1: Sized`
     //~| ERROR overflow evaluating the requirement `<T as Foo1>::Assoc1: Bar`
 }
 

tests/ui/traits/next-solver/overflow/recursive-self-normalization-2.stderr

@@ -19,6 +19,23 @@ note: required by a bound in `needs_bar`
 LL | fn needs_bar<S: Bar>() {}
    |                 ^^^ required by this bound in `needs_bar`
 
+error[E0275]: overflow evaluating the requirement `<T as Foo1>::Assoc1: Sized`
+  --> $DIR/recursive-self-normalization-2.rs:15:17
+   |
+LL |     needs_bar::<T::Assoc1>();
+   |                 ^^^^^^^^^
+   |
+   = help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`recursive_self_normalization_2`)
+note: required by a bound in `needs_bar`
+  --> $DIR/recursive-self-normalization-2.rs:12:14
+   |
+LL | fn needs_bar<S: Bar>() {}
+   |              ^ required by this bound in `needs_bar`
+help: consider relaxing the implicit `Sized` restriction
+   |
+LL | fn needs_bar<S: Bar + ?Sized>() {}
+   |                     ++++++++
+
 error[E0275]: overflow evaluating the requirement `<T as Foo1>::Assoc1 == _`
   --> $DIR/recursive-self-normalization-2.rs:15:5
    |
@@ -45,6 +62,6 @@ LL |     needs_bar::<T::Assoc1>();
    = help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`recursive_self_normalization_2`)
    = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
 
-error: aborting due to 5 previous errors
+error: aborting due to 6 previous errors
 
 For more information about this error, try `rustc --explain E0275`.