diff --git a/compiler/rustc_next_trait_solver/src/solve/assembly/mod.rs b/compiler/rustc_next_trait_solver/src/solve/assembly/mod.rs index ae538441499e0..d5ce32f30ed17 100644 --- a/compiler/rustc_next_trait_solver/src/solve/assembly/mod.rs +++ b/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) diff --git a/compiler/rustc_next_trait_solver/src/solve/mod.rs b/compiler/rustc_next_trait_solver/src/solve/mod.rs index 099c755d78427..dfcb83dda6896 100644 --- a/compiler/rustc_next_trait_solver/src/solve/mod.rs +++ b/compiler/rustc_next_trait_solver/src/solve/mod.rs @@ -86,23 +86,18 @@ fn equal_response_modulo_region_constraints( b: &CanonicalResponse, ) -> 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 EvalCtxt<'_, D> where D: SolverDelegate, @@ -337,48 +326,33 @@ where fn try_merge_candidates( &mut self, candidates: &[Candidate], - ) -> Option<(CanonicalResponse, MergeCandidateInfo)> { - if candidates.is_empty() { + ) -> Option> { + 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 = 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]) -> CanonicalResponse { diff --git a/compiler/rustc_next_trait_solver/src/solve/trait_goals.rs b/compiler/rustc_next_trait_solver/src/solve/trait_goals.rs index 19a09e99c7d64..294b2cbca02bd 100644 --- a/compiler/rustc_next_trait_solver/src/solve/trait_goals.rs +++ b/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 assembly::GoalKind for TraitClause @@ -1575,7 +1574,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 { @@ -1588,7 +1587,7 @@ where candidates: &[Candidate], proven_via: TraitGoalProvenVia, ) -> (CanonicalResponse, Option) { - 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) @@ -1653,6 +1652,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, @@ -1668,38 +1668,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))); } @@ -1708,6 +1680,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, diff --git a/tests/ui/traits/next-solver/cycles/inductive-cycle-but-ok.rs b/tests/ui/traits/next-solver/cycles/inductive-cycle-but-ok.rs deleted file mode 100644 index edcf2e5472b49..0000000000000 --- a/tests/ui/traits/next-solver/cycles/inductive-cycle-but-ok.rs +++ /dev/null @@ -1,44 +0,0 @@ -//@ compile-flags: -Znext-solver -//@ check-pass -#![feature(trivial_bounds, marker_trait_attr)] -#![allow(trivial_bounds)] - -// This previously triggered a bug in the provisional cache. -// -// This has the proof tree -// - `Root: Trait` proven via impl -// - `MultipleCandidates: Trait` -// - candidate: overflow-impl -// - `Root: Trait` (inductive cycle ~> OVERFLOW) -// - candidate: trivial-impl ~> YES -// - merge respones ~> YES -// - `MultipleCandidates: Trait` (in provisional cache ~> OVERFLOW) -// -// We previously incorrectly treated the `MultipleCandidates: Trait` as -// overflow because it was in the cache and reached via an inductive cycle. -// It should be `YES`. - -struct Root; -struct MultipleCandidates; - -#[marker] -trait Trait {} -impl Trait for Root -where - MultipleCandidates: Trait, - MultipleCandidates: Trait, -{} - -// overflow-impl -impl Trait for MultipleCandidates -where - Root: Trait, -{} -// trivial-impl -impl Trait for MultipleCandidates {} - -fn impls_trait() {} - -fn main() { - impls_trait::(); -} diff --git a/tests/ui/traits/next-solver/cycles/inductive-cycle-discarded-coinductive-constraints.rs b/tests/ui/traits/next-solver/cycles/inductive-cycle-discarded-coinductive-constraints.rs deleted file mode 100644 index 527ca812efb8b..0000000000000 --- a/tests/ui/traits/next-solver/cycles/inductive-cycle-discarded-coinductive-constraints.rs +++ /dev/null @@ -1,36 +0,0 @@ -//@ check-pass -//@ compile-flags: -Znext-solver -#![feature(rustc_attrs, marker_trait_attr)] -#[rustc_coinductive] -trait Trait {} - -impl Trait for (T, U) -where - (U, T): Trait, - (T, U): Inductive, - (): ConstrainToU32, -{} - -trait ConstrainToU32 {} -impl ConstrainToU32 for () {} - -// We only prefer the candidate without an inductive cycle -// once the inductive cycle has the same constraints as the -// other goal. -#[marker] -trait Inductive {} -impl Inductive for (T, U) -where - (T, U): Trait, -{} - -impl Inductive for (u32, u32) {} - -fn impls_trait() -where - (T, U): Trait, -{} - -fn main() { - impls_trait::<_, _>(); -}