Auto merge of rust-lang#119744 - lcnr:assemble-only-rigid, r=compiler-errors

only assemble alias bound candidates for rigid aliases

fixes rust-lang/trait-system-refactor-initiative#77

This also causes `<Wrapper<?0> as Trait>::Unwrap: Trait` to always be ambig, as we now normalize the self type before checking whether it is an inference variable.

I cannot think of an approach to the underlying issues here which does not require the "may-define means must-define" restriction for opaque types. Going to go ahead with this and added this restriction to the tracking issue for the new solver to make sure we don't stabilize it without getting types + lang signoff here.

r? `@compiler-errors`

Author: bors

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

@@ -253,82 +253,39 @@ pub(super) trait GoalKind<'tcx>:
         ecx: &mut EvalCtxt<'_, 'tcx>,
         goal: Goal<'tcx, Self>,
     ) -> Vec<(CanonicalResponse<'tcx>, BuiltinImplSource)>;
-
-    /// Consider the `Unsize` candidate corresponding to coercing a sized type
-    /// into a `dyn Trait`.
-    ///
-    /// This is computed separately from the rest of the `Unsize` candidates
-    /// since it is only done once per self type, and not once per
-    /// *normalization step* (in `assemble_candidates_via_self_ty`).
-    fn consider_unsize_to_dyn_candidate(
-        ecx: &mut EvalCtxt<'_, 'tcx>,
-        goal: Goal<'tcx, Self>,
-    ) -> QueryResult<'tcx>;
 }
 
 impl<'tcx> EvalCtxt<'_, 'tcx> {
     pub(super) fn assemble_and_evaluate_candidates<G: GoalKind<'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 +295,13 @@ 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_blanket_impl_candidates(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_param_env_candidates(goal, &mut candidates);
+
+        self.assemble_coherence_unknowable_candidates(goal, &mut candidates);
+
+        candidates
     }
 
     #[instrument(level = "debug", skip_all)]
@@ -500,24 +409,6 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
         }
     }
 
-    #[instrument(level = "debug", skip_all)]
-    fn assemble_unsize_to_dyn_candidate<G: GoalKind<'tcx>>(
-        &mut self,
-        goal: Goal<'tcx, G>,
-        candidates: &mut Vec<Candidate<'tcx>>,
-    ) {
-        let tcx = self.tcx();
-        if tcx.lang_items().unsize_trait() == Some(goal.predicate.trait_def_id(tcx)) {
-            match G::consider_unsize_to_dyn_candidate(self, goal) {
-                Ok(result) => candidates.push(Candidate {
-                    source: CandidateSource::BuiltinImpl(BuiltinImplSource::Misc),
-                    result,
-                }),
-                Err(NoSolution) => (),
-            }
-        }
-    }
-
     #[instrument(level = "debug", skip_all)]
     fn assemble_blanket_impl_candidates<G: GoalKind<'tcx>>(
         &mut self,

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,

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

@@ -603,13 +603,6 @@ impl<'tcx> assembly::GoalKind<'tcx> for NormalizesTo<'tcx> {
         )
     }
 
-    fn consider_unsize_to_dyn_candidate(
-        _ecx: &mut EvalCtxt<'_, 'tcx>,
-        goal: Goal<'tcx, Self>,
-    ) -> QueryResult<'tcx> {
-        bug!("`Unsize` does not have an associated type: {:?}", goal)
-    }
-
     fn consider_structural_builtin_unsize_candidates(
         _ecx: &mut EvalCtxt<'_, 'tcx>,
         goal: Goal<'tcx, Self>,

compiler/rustc_trait_selection/src/solve/trait_goals.rs

@@ -490,53 +490,6 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> {
         ecx.evaluate_added_goals_and_make_canonical_response(certainty)
     }
 
-    fn consider_unsize_to_dyn_candidate(
-        ecx: &mut EvalCtxt<'_, 'tcx>,
-        goal: Goal<'tcx, Self>,
-    ) -> QueryResult<'tcx> {
-        ecx.probe(|_| ProbeKind::UnsizeAssembly).enter(|ecx| {
-            let a_ty = goal.predicate.self_ty();
-            // We need to normalize the b_ty since it's destructured as a `dyn Trait`.
-            let Some(b_ty) =
-                ecx.try_normalize_ty(goal.param_env, goal.predicate.trait_ref.args.type_at(1))
-            else {
-                return ecx.evaluate_added_goals_and_make_canonical_response(Certainty::OVERFLOW);
-            };
-
-            let ty::Dynamic(b_data, b_region, ty::Dyn) = *b_ty.kind() else {
-                return Err(NoSolution);
-            };
-
-            let tcx = ecx.tcx();
-
-            // Can only unsize to an object-safe trait.
-            if b_data.principal_def_id().is_some_and(|def_id| !tcx.check_is_object_safe(def_id)) {
-                return Err(NoSolution);
-            }
-
-            // Check that the type implements all of the predicates of the trait object.
-            // (i.e. the principal, all of the associated types match, and any auto traits)
-            ecx.add_goals(
-                GoalSource::ImplWhereBound,
-                b_data.iter().map(|pred| goal.with(tcx, pred.with_self_ty(tcx, a_ty))),
-            );
-
-            // The type must be `Sized` to be unsized.
-            if let Some(sized_def_id) = tcx.lang_items().sized_trait() {
-                ecx.add_goal(
-                    GoalSource::ImplWhereBound,
-                    goal.with(tcx, ty::TraitRef::new(tcx, sized_def_id, [a_ty])),
-                );
-            } else {
-                return Err(NoSolution);
-            }
-
-            // The type must outlive the lifetime of the `dyn` we're unsizing into.
-            ecx.add_goal(GoalSource::Misc, goal.with(tcx, ty::OutlivesPredicate(a_ty, b_region)));
-            ecx.evaluate_added_goals_and_make_canonical_response(Certainty::Yes)
-        })
-    }
-
     /// ```ignore (builtin impl example)
     /// trait Trait {
     ///     fn foo(&self);
@@ -588,8 +541,11 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> {
                     goal, a_data, a_region, b_data, b_region,
                 ),
 
-                // `T` -> `dyn Trait` unsizing is handled separately in `consider_unsize_to_dyn_candidate`
-                (_, &ty::Dynamic(..)) => vec![],
+                // `T` -> `dyn Trait` unsizing.
+                (_, &ty::Dynamic(b_region, b_data, ty::Dyn)) => result_to_single(
+                    ecx.consider_builtin_unsize_to_dyn_candidate(goal, b_region, b_data),
+                    BuiltinImplSource::Misc,
+                ),
 
                 // `[T; N]` -> `[T]` unsizing
                 (&ty::Array(a_elem_ty, ..), &ty::Slice(b_elem_ty)) => result_to_single(
@@ -691,6 +647,42 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
         responses
     }
 
+    fn consider_builtin_unsize_to_dyn_candidate(
+        &mut self,
+        goal: Goal<'tcx, (Ty<'tcx>, Ty<'tcx>)>,
+        b_data: &'tcx ty::List<ty::PolyExistentialPredicate<'tcx>>,
+        b_region: ty::Region<'tcx>,
+    ) -> QueryResult<'tcx> {
+        let tcx = self.tcx();
+        let Goal { predicate: (a_ty, _), .. } = goal;
+
+        // Can only unsize to an object-safe trait.
+        if b_data.principal_def_id().is_some_and(|def_id| !tcx.check_is_object_safe(def_id)) {
+            return Err(NoSolution);
+        }
+
+        // Check that the type implements all of the predicates of the trait object.
+        // (i.e. the principal, all of the associated types match, and any auto traits)
+        self.add_goals(
+            GoalSource::ImplWhereBound,
+            b_data.iter().map(|pred| goal.with(tcx, pred.with_self_ty(tcx, a_ty))),
+        );
+
+        // The type must be `Sized` to be unsized.
+        if let Some(sized_def_id) = tcx.lang_items().sized_trait() {
+            self.add_goal(
+                GoalSource::ImplWhereBound,
+                goal.with(tcx, ty::TraitRef::new(tcx, sized_def_id, [a_ty])),
+            );
+        } else {
+            return Err(NoSolution);
+        }
+
+        // The type must outlive the lifetime of the `dyn` we're unsizing into.
+        self.add_goal(GoalSource::Misc, goal.with(tcx, ty::OutlivesPredicate(a_ty, b_region)));
+        self.evaluate_added_goals_and_make_canonical_response(Certainty::Yes)
+    }
+
     fn consider_builtin_upcast_to_principal(
         &mut self,
         goal: Goal<'tcx, (Ty<'tcx>, Ty<'tcx>)>,

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() {}