Rollup merge of rust-lang#139762 - compiler-errors:non-env, r=lcnr

Don't assemble non-env/bound candidates if projection is rigid

Putting this up for an initial review, it's still missing comments, clean-up, and possibly a tweak to deal with ambiguities in the `BestObligation` folder.

This PR fixes rust-lang/trait-system-refactor-initiative#173. Specifically, we're creating an unnecessary query cycle in normalization by assembling an *impl candidate* even if we know later on during `merge_candidates` that we'll be filtering out that impl candidate.

This PR adjusts the `merge_candidates` to assemble *only* env/bound candidates if we have `TraitGoalProvenVia::ParamEnv | TraitGoalProvenVia::AliasBound`.

I'll leave some thoughts/comments in the code.

r? lcnr

Author: ChrisDenton

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

@@ -288,6 +288,21 @@ where
     ) -> Vec<Candidate<I>>;
 }
 
+/// Allows callers of `assemble_and_evaluate_candidates` to choose whether to limit
+/// candidate assembly to param-env and alias-bound candidates.
+///
+/// On top of being a micro-optimization, as it avoids doing unnecessary work when
+/// a param-env trait bound candidate shadows impls for normalization, this is also
+/// required to prevent query cycles due to RPITIT inference. See the issue at:
+/// <https://github.com/rust-lang/trait-system-refactor-initiative/issues/173>.
+pub(super) enum AssembleCandidatesFrom {
+    All,
+    /// Only assemble candidates from the environment and alias bounds, ignoring
+    /// user-written and built-in impls. We only expect `ParamEnv` and `AliasBound`
+    /// candidates to be assembled.
+    EnvAndBounds,
+}
+
 impl<D, I> EvalCtxt<'_, D>
 where
     D: SolverDelegate<Interner = I>,
@@ -296,6 +311,7 @@ where
     pub(super) fn assemble_and_evaluate_candidates<G: GoalKind<D>>(
         &mut self,
         goal: Goal<I, G>,
+        assemble_from: AssembleCandidatesFrom,
     ) -> Vec<Candidate<I>> {
         let Ok(normalized_self_ty) =
             self.structurally_normalize_ty(goal.param_env, goal.predicate.self_ty())
@@ -322,16 +338,18 @@ where
             }
         }
 
-        self.assemble_impl_candidates(goal, &mut candidates);
-
-        self.assemble_builtin_impl_candidates(goal, &mut candidates);
-
         self.assemble_alias_bound_candidates(goal, &mut candidates);
-
-        self.assemble_object_bound_candidates(goal, &mut candidates);
-
         self.assemble_param_env_candidates(goal, &mut candidates);
 
+        match assemble_from {
+            AssembleCandidatesFrom::All => {
+                self.assemble_impl_candidates(goal, &mut candidates);
+                self.assemble_builtin_impl_candidates(goal, &mut candidates);
+                self.assemble_object_bound_candidates(goal, &mut candidates);
+            }
+            AssembleCandidatesFrom::EnvAndBounds => {}
+        }
+
         candidates
     }
 
@@ -754,6 +772,9 @@ where
         })
     }
 
+    /// Assemble and merge candidates for goals which are related to an underlying trait
+    /// goal. Right now, this is normalizes-to and host effect goals.
+    ///
     /// We sadly can't simply take all possible candidates for normalization goals
     /// and check whether they result in the same constraints. We want to make sure
     /// that trying to normalize an alias doesn't result in constraints which aren't
@@ -782,47 +803,44 @@ where
     ///
     /// See trait-system-refactor-initiative#124 for more details.
     #[instrument(level = "debug", skip(self, inject_normalize_to_rigid_candidate), ret)]
-    pub(super) fn merge_candidates(
+    pub(super) fn assemble_and_merge_candidates<G: GoalKind<D>>(
         &mut self,
         proven_via: Option<TraitGoalProvenVia>,
-        candidates: Vec<Candidate<I>>,
+        goal: Goal<I, G>,
         inject_normalize_to_rigid_candidate: impl FnOnce(&mut EvalCtxt<'_, D>) -> QueryResult<I>,
     ) -> QueryResult<I> {
         let Some(proven_via) = proven_via else {
             // We don't care about overflow. If proving the trait goal overflowed, then
             // it's enough to report an overflow error for that, we don't also have to
             // overflow during normalization.
-            return Ok(self.make_ambiguous_response_no_constraints(MaybeCause::Ambiguity));
+            //
+            // We use `forced_ambiguity` here over `make_ambiguous_response_no_constraints`
+            // because the former will also record a built-in candidate in the inspector.
+            return self.forced_ambiguity(MaybeCause::Ambiguity).map(|cand| cand.result);
         };
 
         match proven_via {
             TraitGoalProvenVia::ParamEnv | TraitGoalProvenVia::AliasBound => {
-                let mut considered_candidates = Vec::new();
-                considered_candidates.extend(
-                    candidates
-                        .iter()
-                        .filter(|c| matches!(c.source, CandidateSource::ParamEnv(_)))
-                        .map(|c| c.result),
-                );
-
                 // Even when a trait bound has been proven using a where-bound, we
                 // still need to consider alias-bounds for normalization, see
-                // tests/ui/next-solver/alias-bound-shadowed-by-env.rs.
-                //
+                // `tests/ui/next-solver/alias-bound-shadowed-by-env.rs`.
+                let candidates_from_env_and_bounds: Vec<_> = self
+                    .assemble_and_evaluate_candidates(goal, AssembleCandidatesFrom::EnvAndBounds);
+
                 // We still need to prefer where-bounds over alias-bounds however.
-                // See tests/ui/winnowing/norm-where-bound-gt-alias-bound.rs.
-                //
-                // FIXME(const_trait_impl): should this behavior also be used by
-                // constness checking. Doing so is *at least theoretically* breaking,
-                // see github.com/rust-lang/rust/issues/133044#issuecomment-2500709754
-                if considered_candidates.is_empty() {
-                    considered_candidates.extend(
-                        candidates
-                            .iter()
-                            .filter(|c| matches!(c.source, CandidateSource::AliasBound))
-                            .map(|c| c.result),
-                    );
-                }
+                // See `tests/ui/winnowing/norm-where-bound-gt-alias-bound.rs`.
+                let mut considered_candidates: Vec<_> = if candidates_from_env_and_bounds
+                    .iter()
+                    .any(|c| matches!(c.source, CandidateSource::ParamEnv(_)))
+                {
+                    candidates_from_env_and_bounds
+                        .into_iter()
+                        .filter(|c| matches!(c.source, CandidateSource::ParamEnv(_)))
+                        .map(|c| c.result)
+                        .collect()
+                } else {
+                    candidates_from_env_and_bounds.into_iter().map(|c| c.result).collect()
+                };
 
                 // If the trait goal has been proven by using the environment, we want to treat
                 // aliases as rigid if there are no applicable projection bounds in the environment.
@@ -839,6 +857,9 @@ where
                 }
             }
             TraitGoalProvenVia::Misc => {
+                let candidates =
+                    self.assemble_and_evaluate_candidates(goal, AssembleCandidatesFrom::All);
+
                 // Prefer "orphaned" param-env normalization predicates, which are used
                 // (for example, and ideally only) when proving item bounds for an impl.
                 let candidates_from_env: Vec<_> = candidates

compiler/rustc_next_trait_solver/src/solve/effect_goals.rs

@@ -399,12 +399,11 @@ where
         &mut self,
         goal: Goal<I, ty::HostEffectPredicate<I>>,
     ) -> QueryResult<I> {
-        let candidates = self.assemble_and_evaluate_candidates(goal);
         let (_, proven_via) = self.probe(|_| ProbeKind::ShadowedEnvProbing).enter(|ecx| {
             let trait_goal: Goal<I, ty::TraitPredicate<I>> =
                 goal.with(ecx.cx(), goal.predicate.trait_ref);
             ecx.compute_trait_goal(trait_goal)
         })?;
-        self.merge_candidates(proven_via, candidates, |_ecx| Err(NoSolution))
+        self.assemble_and_merge_candidates(proven_via, goal, |_ecx| Err(NoSolution))
     }
 }

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

@@ -32,14 +32,13 @@ where
         let cx = self.cx();
         match goal.predicate.alias.kind(cx) {
             ty::AliasTermKind::ProjectionTy | ty::AliasTermKind::ProjectionConst => {
-                let candidates = self.assemble_and_evaluate_candidates(goal);
                 let trait_ref = goal.predicate.alias.trait_ref(cx);
                 let (_, proven_via) =
                     self.probe(|_| ProbeKind::ShadowedEnvProbing).enter(|ecx| {
                         let trait_goal: Goal<I, ty::TraitPredicate<I>> = goal.with(cx, trait_ref);
                         ecx.compute_trait_goal(trait_goal)
                     })?;
-                self.merge_candidates(proven_via, candidates, |ecx| {
+                self.assemble_and_merge_candidates(proven_via, goal, |ecx| {
                     ecx.probe(|&result| ProbeKind::RigidAlias { result }).enter(|this| {
                         this.structurally_instantiate_normalizes_to_term(
                             goal,

compiler/rustc_next_trait_solver/src/solve/trait_goals.rs

@@ -13,7 +13,7 @@ use tracing::{instrument, trace};
 
 use crate::delegate::SolverDelegate;
 use crate::solve::assembly::structural_traits::{self, AsyncCallableRelevantTypes};
-use crate::solve::assembly::{self, Candidate};
+use crate::solve::assembly::{self, AssembleCandidatesFrom, Candidate};
 use crate::solve::inspect::ProbeKind;
 use crate::solve::{
     BuiltinImplSource, CandidateSource, Certainty, EvalCtxt, Goal, GoalSource, MaybeCause,
@@ -1365,7 +1365,7 @@ where
         &mut self,
         goal: Goal<I, TraitPredicate<I>>,
     ) -> Result<(CanonicalResponse<I>, Option<TraitGoalProvenVia>), NoSolution> {
-        let candidates = self.assemble_and_evaluate_candidates(goal);
+        let candidates = self.assemble_and_evaluate_candidates(goal, AssembleCandidatesFrom::All);
         self.merge_trait_candidates(goal, candidates)
     }
 }

tests/ui/impl-unused-tps.stderr

@@ -7,23 +7,6 @@ LL | impl<T> Foo<T> for [isize; 0] {
 LL | impl<T, U> Foo<T> for U {
    | ^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `[isize; 0]`
 
-error[E0207]: the type parameter `U` is not constrained by the impl trait, self type, or predicates
-  --> $DIR/impl-unused-tps.rs:32:9
-   |
-LL | impl<T, U> Bar for T {
-   |         ^ unconstrained type parameter
-
-error[E0119]: conflicting implementations of trait `Bar`
-  --> $DIR/impl-unused-tps.rs:40:1
-   |
-LL |   impl<T, U> Bar for T {
-   |   -------------------- first implementation here
-...
-LL | / impl<T, U> Bar for T
-LL | | where
-LL | |     T: Bar<Out = U>,
-   | |____________________^ conflicting implementation
-
 error[E0119]: conflicting implementations of trait `Foo<[isize; 0]>` for type `[isize; 0]`
   --> $DIR/impl-unused-tps.rs:49:1
    |
@@ -52,6 +35,12 @@ error[E0207]: the type parameter `U` is not constrained by the impl trait, self
 LL | impl<T, U> Foo<T> for [isize; 1] {
    |         ^ unconstrained type parameter
 
+error[E0207]: the type parameter `U` is not constrained by the impl trait, self type, or predicates
+  --> $DIR/impl-unused-tps.rs:32:9
+   |
+LL | impl<T, U> Bar for T {
+   |         ^ unconstrained type parameter
+
 error[E0207]: the type parameter `U` is not constrained by the impl trait, self type, or predicates
   --> $DIR/impl-unused-tps.rs:40:9
    |
@@ -70,6 +59,17 @@ error[E0207]: the type parameter `V` is not constrained by the impl trait, self
 LL | impl<T, U, V> Foo<T> for T
    |            ^ unconstrained type parameter
 
+error[E0119]: conflicting implementations of trait `Bar`
+  --> $DIR/impl-unused-tps.rs:40:1
+   |
+LL |   impl<T, U> Bar for T {
+   |   -------------------- first implementation here
+...
+LL | / impl<T, U> Bar for T
+LL | | where
+LL | |     T: Bar<Out = U>,
+   | |____________________^ conflicting implementation
+
 error: aborting due to 9 previous errors
 
 Some errors have detailed explanations: E0119, E0207.

tests/ui/traits/next-solver/rpitit-cycle-due-to-rigid.rs

@@ -0,0 +1,32 @@
+//@ compile-flags: -Znext-solver
+//@ check-pass
+//@ edition: 2024
+
+// Ensure we don't end up in a query cycle due to trying to assemble an impl candidate
+// for an RPITIT normalizes-to goal, even though that impl candidate would *necessarily*
+// be made rigid by a where clause. This query cycle is thus avoidable by not assembling
+// that impl candidate which we *know* we are going to throw away anyways.
+
+use std::future::Future;
+
+pub trait ReactiveFunction: Send {
+    type Output;
+
+    fn invoke(self) -> Self::Output;
+}
+
+trait AttributeValue {
+    fn resolve(self) -> impl Future<Output = ()> + Send;
+}
+
+impl<F, V> AttributeValue for F
+where
+    F: ReactiveFunction<Output = V>,
+    V: AttributeValue,
+{
+    async fn resolve(self) {
+        self.invoke().resolve().await
+    }
+}
+
+fn main() {}