do not prefer trivially true candidates

Author: lcnr

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

@@ -1335,7 +1335,7 @@ where
                     candidates.retain(|c| matches!(c.source, CandidateSource::ParamEnv(_)));
                 }
 
-                if let Some((response, _)) = self.try_merge_candidates(&candidates) {
+                if let Some(response) = self.try_merge_candidates(&candidates) {
                     Ok(response)
                 } else {
                     self.flounder(&candidates).map_err(Into::into)
@@ -1357,7 +1357,7 @@ where
                 // means we can just ignore inference constraints and don't have to special-case
                 // constraining the normalized-to `term`.
                 self.filter_specialized_impls(AllowInferenceConstraints::Yes, &mut candidates);
-                if let Some((response, _)) = self.try_merge_candidates(&candidates) {
+                if let Some(response) = self.try_merge_candidates(&candidates) {
                     Ok(response)
                 } else {
                     self.flounder(&candidates).map_err(Into::into)

compiler/rustc_next_trait_solver/src/solve/mod.rs

@@ -86,23 +86,18 @@ fn equal_response_modulo_region_constraints<I: Interner>(
     b: &CanonicalResponse<I>,
 ) -> bool {
     let CanonicalResponse {
-        max_universe: a_max_universe,
-        var_kinds: a_var_kinds,
-        value:
-            Response {
-                var_values: a_var_values,
-                certainty: a_certainty,
-                external_constraints: a_external_constraints,
-            },
+        max_universe,
+        var_kinds,
+        value: Response { var_values, certainty, external_constraints },
     } = a;
 
     let ExternalConstraintsData { region_constraints: _, opaque_types, normalization_nested_goals } =
-        &**a_external_constraints;
+        &**external_constraints;
 
-    a_max_universe == &b.max_universe
-        && a_var_kinds == &b.var_kinds
-        && a_var_values == &b.value.var_values
-        && a_certainty == &b.value.certainty
+    max_universe == &b.max_universe
+        && var_kinds == &b.var_kinds
+        && var_values == &b.value.var_values
+        && certainty == &b.value.certainty
         && opaque_types == &b.value.external_constraints.opaque_types
         && normalization_nested_goals == &b.value.external_constraints.normalization_nested_goals
 }
@@ -319,12 +314,6 @@ where
     }
 }
 
-#[derive(Debug)]
-enum MergeCandidateInfo {
-    AlwaysApplicable(usize),
-    EqualResponse,
-}
-
 impl<D, I> EvalCtxt<'_, D>
 where
     D: SolverDelegate<Interner = I>,
@@ -337,48 +326,33 @@ where
     fn try_merge_candidates(
         &mut self,
         candidates: &[Candidate<I>],
-    ) -> Option<(CanonicalResponse<I>, MergeCandidateInfo)> {
-        if candidates.is_empty() {
+    ) -> Option<CanonicalResponse<I>> {
+        let Some((first, rest)) = candidates.split_first() else {
             return None;
-        }
-
-        let always_applicable = candidates.iter().enumerate().find(|(_, candidate)| {
-            candidate.result.value.certainty == Certainty::Yes
-                && has_no_inference_or_external_constraints(candidate.result)
-        });
+        };
 
-        if let Some((i, candidate)) = always_applicable {
-            return Some((candidate.result, MergeCandidateInfo::AlwaysApplicable(i)));
+        if rest.iter().any(|candidate| {
+            !equal_response_modulo_region_constraints(&first.result, &candidate.result)
+        }) {
+            return None;
         }
 
-        let one: CanonicalResponse<I> = candidates[0].result;
-
-        if candidates[1..]
-            .iter()
-            .all(|candidate| equal_response_modulo_region_constraints(&one, &candidate.result))
-        {
-            let region_constraints = &one.value.external_constraints.region_constraints;
-            if candidates[1..].iter().all(|candidate| {
-                &candidate.result.value.external_constraints.region_constraints
-                    == region_constraints
-            }) {
-                return Some((one, MergeCandidateInfo::EqualResponse));
-            }
-
-            // If candidates differ only in region constraints, their merged region
-            // constraints are an `Or` of their respective constraints. If one of them
-            // has no region constraints, the `Or` constraint evaluates to `true`.
-            //
-            // This is a special case of `-Zassumptions-on-binders` and should be
-            // replaced eventually.
-            if let Some(candidate) = candidates.iter().find(|candidate| {
-                candidate.result.value.external_constraints.region_constraints.is_empty()
-            }) {
-                return Some((candidate.result, MergeCandidateInfo::EqualResponse));
-            }
+        let region_constraints = &first.result.value.external_constraints.region_constraints;
+        if candidates[1..].iter().all(|candidate| {
+            &candidate.result.value.external_constraints.region_constraints == region_constraints
+        }) {
+            return Some(first.result);
         }
 
-        None
+        // If candidates differ only in region constraints, their merged region
+        // constraints are an OR of their respective constraints. We don't support
+        // OR constraints yet, so the only way we can merge differing candidates
+        // if one of them has no region constraints as then the OR constraint
+        // is trivially true.
+        candidates
+            .iter()
+            .find(|c| c.result.value.external_constraints.region_constraints.is_empty())
+            .map(|c| c.result)
     }
 
     fn bail_with_ambiguity(&mut self, candidates: &[Candidate<I>]) -> CanonicalResponse<I> {

compiler/rustc_next_trait_solver/src/solve/trait_goals.rs

@@ -24,8 +24,7 @@ use crate::solve::assembly::{
 use crate::solve::inspect::ProbeKind;
 use crate::solve::{
     BuiltinImplSource, CandidateSource, Certainty, EvalCtxt, Goal, GoalSource, MaybeCause,
-    MergeCandidateInfo, NoSolution, ParamEnvSource, StalledOnCoroutines,
-    has_only_region_constraints,
+    NoSolution, ParamEnvSource, StalledOnCoroutines, has_only_region_constraints,
 };
 
 impl<D, I> assembly::GoalKind<D> for TraitClause<I>
@@ -1578,7 +1577,7 @@ where
             .all(|candidate| matches!(candidate.source, CandidateSource::BuiltinImpl(_)));
 
         if is_marker || all_builtin {
-            self.try_merge_candidates(candidates).map(|(response, _)| response)
+            self.try_merge_candidates(candidates)
         } else if candidates.len() > 1 {
             None
         } else {
@@ -1591,7 +1590,7 @@ where
         candidates: &[Candidate<I>],
         proven_via: TraitGoalProvenVia,
     ) -> (CanonicalResponse<I>, Option<TraitGoalProvenVia>) {
-        if let Some((response, _)) = self.try_merge_candidates(candidates) {
+        if let Some(response) = self.try_merge_candidates(candidates) {
             (response, Some(proven_via))
         } else {
             (self.bail_with_ambiguity(candidates), None)
@@ -1656,6 +1655,7 @@ where
             let alias_bounds: Vec<_> = candidates
                 .extract_if(.., |c| matches!(c.source, CandidateSource::AliasBound(..)))
                 .collect();
+            candidates.into_iter().for_each(|c| self.ignore_candidate_head_usages(c.head_usages));
             return Ok(self.merge_candidates_or_bail_with_ambiguity(
                 &alias_bounds,
                 TraitGoalProvenVia::AliasBound,
@@ -1671,38 +1671,10 @@ where
             let where_bounds: Vec<_> = candidates
                 .extract_if(.., |c| matches!(c.source, CandidateSource::ParamEnv(_)))
                 .collect();
-            let Some((response, info)) = self.try_merge_candidates(&where_bounds) else {
+            candidates.into_iter().for_each(|c| self.ignore_candidate_head_usages(c.head_usages));
+            let Some(response) = self.try_merge_candidates(&where_bounds) else {
                 return Ok((self.bail_with_ambiguity(&where_bounds), None));
             };
-            match info {
-                // If there's an always applicable candidate, the result of all
-                // other candidates does not matter. This means we can ignore
-                // them when checking whether we've reached a fixpoint.
-                //
-                // We always prefer the first always applicable candidate, even if a
-                // later candidate is also always applicable and would result in fewer
-                // reruns. We could slightly improve this by e.g. searching for another
-                // always applicable candidate which doesn't depend on any cycle heads.
-                //
-                // NOTE: This is optimization is observable in case there is an always
-                // applicable global candidate and another non-global candidate which only
-                // applies because of a provisional result. I can't even think of a test
-                // case where this would occur and even then, this would not be unsound.
-                // Supporting this makes the code more involved, so I am just going to
-                // ignore this for now.
-                MergeCandidateInfo::AlwaysApplicable(i) => {
-                    for (j, c) in where_bounds.into_iter().enumerate() {
-                        if i != j {
-                            self.ignore_candidate_head_usages(c.head_usages)
-                        }
-                    }
-                    // If a where-bound does not apply, we don't actually get a
-                    // candidate for it. We manually track the head usages
-                    // of all failed `ParamEnv` candidates instead.
-                    self.ignore_candidate_head_usages(failed_candidate_info.param_env_head_usages);
-                }
-                MergeCandidateInfo::EqualResponse => {}
-            }
             return Ok((response, Some(TraitGoalProvenVia::ParamEnv)));
         }
 
@@ -1711,6 +1683,7 @@ where
             let alias_bounds: Vec<_> = candidates
                 .extract_if(.., |c| matches!(c.source, CandidateSource::AliasBound(_)))
                 .collect();
+            candidates.into_iter().for_each(|c| self.ignore_candidate_head_usages(c.head_usages));
             return Ok(self.merge_candidates_or_bail_with_ambiguity(
                 &alias_bounds,
                 TraitGoalProvenVia::AliasBound,