From 5f548890b8f446c7681ab02be8ae62afda2b9c9f Mon Sep 17 00:00:00 2001
From: lcnr <rust@lcnr.de>
Date: Thu, 14 Nov 2024 18:50:32 +0100
Subject: [PATCH 1/2] add tests
---
.../param-env-eager-norm-dedup.rs | 23 +++++++++++++++++++
.../winnowing/global-non-global-env-1.rs | 18 +++++++++++++++
.../winnowing/global-non-global-env-2.rs | 18 +++++++++++++++
.../winnowing/global-non-global-env-2.stderr | 17 ++++++++++++++
.../winnowing/global-non-global-env-3.rs | 20 ++++++++++++++++
.../winnowing/global-non-global-env-4.rs | 19 +++++++++++++++
.../winnowing/global-non-global-env-4.stderr | 17 ++++++++++++++
7 files changed, 132 insertions(+)
create mode 100644 tests/ui/const-generics/min_const_generics/param-env-eager-norm-dedup.rs
create mode 100644 tests/ui/traits/winnowing/global-non-global-env-1.rs
create mode 100644 tests/ui/traits/winnowing/global-non-global-env-2.rs
create mode 100644 tests/ui/traits/winnowing/global-non-global-env-2.stderr
create mode 100644 tests/ui/traits/winnowing/global-non-global-env-3.rs
create mode 100644 tests/ui/traits/winnowing/global-non-global-env-4.rs
create mode 100644 tests/ui/traits/winnowing/global-non-global-env-4.stderr
diff --git a/tests/ui/const-generics/min_const_generics/param-env-eager-norm-dedup.rs b/tests/ui/const-generics/min_const_generics/param-env-eager-norm-dedup.rs
new file mode 100644
index 0000000000000..9600b3875ba48
--- /dev/null
+++ b/tests/ui/const-generics/min_const_generics/param-env-eager-norm-dedup.rs
@@ -0,0 +1,23 @@
+//@ check-pass
+
+// This caused a regression in a crater run in #132325.
+//
+// The underlying issue is a really subtle implementation detail.
+//
+// When building the `param_env` for `Trait` we start out with its
+// explicit predicates `Self: Trait` and `Self: for<'a> Super<'a, { 1 + 1 }>`.
+//
+// When normalizing the environment we also elaborate. This implicitly
+// deduplicates its returned predicates. We currently first eagerly
+// normalize constants in the unnormalized param env to avoid issues
+// caused by our lack of deferred alias equality.
+//
+// So we actually elaborate `Self: Trait` and `Self: for<'a> Super<'a, 2>`,
+// resulting in a third `Self: for<'a> Super<'a, { 1 + 1 }>` predicate which
+// then gets normalized to `Self: for<'a> Super<'a, 2>` at which point we
+// do not deduplicate however. By failing to handle equal where-bounds in
+// candidate selection, this caused ambiguity when checking that `Trait` is
+// well-formed.
+trait Super<'a, const N: usize> {}
+trait Trait: for<'a> Super<'a, { 1 + 1 }> {}
+fn main() {}
diff --git a/tests/ui/traits/winnowing/global-non-global-env-1.rs b/tests/ui/traits/winnowing/global-non-global-env-1.rs
new file mode 100644
index 0000000000000..d232d32dddffd
--- /dev/null
+++ b/tests/ui/traits/winnowing/global-non-global-env-1.rs
@@ -0,0 +1,18 @@
+//@ check-pass
+
+// A regression test for an edge case of candidate selection
+// in the old trait solver, see #132325 for more details.
+
+trait Trait<T> {}
+impl<T> Trait<T> for () {}
+
+fn impls_trait<T: Trait<U>, U>(_: T) -> U { todo!() }
+fn foo<T>() -> u32
+where
+ (): Trait<u32>,
+ (): Trait<T>,
+{
+ impls_trait(())
+}
+
+fn main() {}
diff --git a/tests/ui/traits/winnowing/global-non-global-env-2.rs b/tests/ui/traits/winnowing/global-non-global-env-2.rs
new file mode 100644
index 0000000000000..1647606934660
--- /dev/null
+++ b/tests/ui/traits/winnowing/global-non-global-env-2.rs
@@ -0,0 +1,18 @@
+// A regression test for an edge case of candidate selection
+// in the old trait solver, see #132325 for more details. Unlike
+// the first test, this one has two impl candidates.
+
+trait Trait<T> {}
+impl Trait<u32> for () {}
+impl Trait<u64> for () {}
+
+fn impls_trait<T: Trait<U>, U>(_: T) -> U { todo!() }
+fn foo<T>() -> u32
+where
+ (): Trait<u32>,
+ (): Trait<T>,
+{
+ impls_trait(()) //~ ERROR mismatched types
+}
+
+fn main() {}
diff --git a/tests/ui/traits/winnowing/global-non-global-env-2.stderr b/tests/ui/traits/winnowing/global-non-global-env-2.stderr
new file mode 100644
index 0000000000000..dee01c72e4820
--- /dev/null
+++ b/tests/ui/traits/winnowing/global-non-global-env-2.stderr
@@ -0,0 +1,17 @@
+error[E0308]: mismatched types
+ --> $DIR/global-non-global-env-2.rs:15:5
+ |
+LL | fn foo<T>() -> u32
+ | - --- expected `u32` because of return type
+ | |
+ | found this type parameter
+...
+LL | impls_trait(())
+ | ^^^^^^^^^^^^^^^ expected `u32`, found type parameter `T`
+ |
+ = note: expected type `u32`
+ found type parameter `T`
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0308`.
diff --git a/tests/ui/traits/winnowing/global-non-global-env-3.rs b/tests/ui/traits/winnowing/global-non-global-env-3.rs
new file mode 100644
index 0000000000000..008d07e41446a
--- /dev/null
+++ b/tests/ui/traits/winnowing/global-non-global-env-3.rs
@@ -0,0 +1,20 @@
+//@ check-pass
+
+// A regression test for an edge case of candidate selection
+// in the old trait solver, see #132325 for more details. Unlike
+// the second test, the where-bounds are in a different order.
+
+trait Trait<T> {}
+impl Trait<u32> for () {}
+impl Trait<u64> for () {}
+
+fn impls_trait<T: Trait<U>, U>(_: T) -> U { todo!() }
+fn foo<T>() -> u32
+where
+ (): Trait<T>,
+ (): Trait<u32>,
+{
+ impls_trait(())
+}
+
+fn main() {}
diff --git a/tests/ui/traits/winnowing/global-non-global-env-4.rs b/tests/ui/traits/winnowing/global-non-global-env-4.rs
new file mode 100644
index 0000000000000..1f35cf20f83dc
--- /dev/null
+++ b/tests/ui/traits/winnowing/global-non-global-env-4.rs
@@ -0,0 +1,19 @@
+// A regression test for an edge case of candidate selection
+// in the old trait solver, see #132325 for more details. Unlike
+// the third test, this one has 3 impl candidates.
+
+trait Trait<T> {}
+impl Trait<u32> for () {}
+impl Trait<u64> for () {}
+impl Trait<u128> for () {}
+
+fn impls_trait<T: Trait<U>, U>(_: T) -> U { todo!() }
+fn foo<T>() -> u32
+where
+ (): Trait<T>,
+ (): Trait<u32>,
+{
+ impls_trait(()) //~ ERROR mismatched types
+}
+
+fn main() {}
diff --git a/tests/ui/traits/winnowing/global-non-global-env-4.stderr b/tests/ui/traits/winnowing/global-non-global-env-4.stderr
new file mode 100644
index 0000000000000..6449f3feb08cf
--- /dev/null
+++ b/tests/ui/traits/winnowing/global-non-global-env-4.stderr
@@ -0,0 +1,17 @@
+error[E0308]: mismatched types
+ --> $DIR/global-non-global-env-4.rs:16:5
+ |
+LL | fn foo<T>() -> u32
+ | - --- expected `u32` because of return type
+ | |
+ | found this type parameter
+...
+LL | impls_trait(())
+ | ^^^^^^^^^^^^^^^ expected `u32`, found type parameter `T`
+ |
+ = note: expected type `u32`
+ found type parameter `T`
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0308`.
From 3350b9faadd8e9c3cf87a9c8de266ea252a67c5c Mon Sep 17 00:00:00 2001
From: lcnr <rust@lcnr.de>
Date: Tue, 29 Oct 2024 16:10:13 +0100
Subject: [PATCH 2/2] consistently handle global where-bounds
---
compiler/rustc_trait_selection/src/lib.rs | 1 +
.../src/traits/select/mod.rs | 546 ++++++++----------
.../winnowing/global-non-global-env-2.rs | 4 +-
.../winnowing/global-non-global-env-2.stderr | 17 -
.../winnowing/global-non-global-env-4.rs | 4 +-
.../winnowing/global-non-global-env-4.stderr | 17 -
6 files changed, 248 insertions(+), 341 deletions(-)
delete mode 100644 tests/ui/traits/winnowing/global-non-global-env-2.stderr
delete mode 100644 tests/ui/traits/winnowing/global-non-global-env-4.stderr
diff --git a/compiler/rustc_trait_selection/src/lib.rs b/compiler/rustc_trait_selection/src/lib.rs
index 11d72106b221f..1af383e9200d0 100644
--- a/compiler/rustc_trait_selection/src/lib.rs
+++ b/compiler/rustc_trait_selection/src/lib.rs
@@ -23,6 +23,7 @@
#![feature(extract_if)]
#![feature(if_let_guard)]
#![feature(iter_intersperse)]
+#![feature(iterator_try_reduce)]
#![feature(let_chains)]
#![feature(never_type)]
#![feature(rustdoc_internals)]
diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs
index 25fe43e3a0e6f..32a2d5a6d93a6 100644
--- a/compiler/rustc_trait_selection/src/traits/select/mod.rs
+++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs
@@ -445,7 +445,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
// Winnow, but record the exact outcome of evaluation, which
// is needed for specialization. Propagate overflow if it occurs.
- let mut candidates = candidates
+ let candidates = candidates
.into_iter()
.map(|c| match self.evaluate_candidate(stack, &c) {
Ok(eval) if eval.may_apply() => {
@@ -458,40 +458,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
.flat_map(Result::transpose)
.collect::<Result<Vec<_>, _>>()?;
- debug!(?stack, ?candidates, "winnowed to {} candidates", candidates.len());
-
- let has_non_region_infer = stack.obligation.predicate.has_non_region_infer();
-
- // If there are STILL multiple candidates, we can further
- // reduce the list by dropping duplicates -- including
- // resolving specializations.
- if candidates.len() > 1 {
- let mut i = 0;
- while i < candidates.len() {
- let should_drop_i = (0..candidates.len()).filter(|&j| i != j).any(|j| {
- self.candidate_should_be_dropped_in_favor_of(
- &candidates[i],
- &candidates[j],
- has_non_region_infer,
- ) == DropVictim::Yes
- });
- if should_drop_i {
- debug!(candidate = ?candidates[i], "Dropping candidate #{}/{}", i, candidates.len());
- candidates.swap_remove(i);
- } else {
- debug!(candidate = ?candidates[i], "Retaining candidate #{}/{}", i, candidates.len());
- i += 1;
-
- // If there are *STILL* multiple candidates, give up
- // and report ambiguity.
- if i > 1 {
- debug!("multiple matches, ambig");
- return Ok(None);
- }
- }
- }
- }
-
+ debug!(?stack, ?candidates, "{} potentially applicable candidates", candidates.len());
// If there are *NO* candidates, then there are no impls --
// that we know of, anyway. Note that in the case where there
// are unbound type variables within the obligation, it might
@@ -508,13 +475,18 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
// to have emitted at least one.
if stack.obligation.predicate.references_error() {
debug!(?stack.obligation.predicate, "found error type in predicate, treating as ambiguous");
- return Ok(None);
+ Ok(None)
+ } else {
+ Err(Unimplemented)
+ }
+ } else {
+ let has_non_region_infer = stack.obligation.predicate.has_non_region_infer();
+ if let Some(candidate) = self.winnow_candidates(has_non_region_infer, candidates) {
+ self.filter_reservation_impls(candidate)
+ } else {
+ Ok(None)
}
- return Err(Unimplemented);
}
-
- // Just one candidate left.
- self.filter_reservation_impls(candidates.pop().unwrap().candidate)
}
///////////////////////////////////////////////////////////////////////////
@@ -1803,18 +1775,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
}
}
-#[derive(Debug, Copy, Clone, PartialEq, Eq)]
-enum DropVictim {
- Yes,
- No,
-}
-
-impl DropVictim {
- fn drop_if(should_drop: bool) -> DropVictim {
- if should_drop { DropVictim::Yes } else { DropVictim::No }
- }
-}
-
/// ## Winnowing
///
/// Winnowing is the process of attempting to resolve ambiguity by
@@ -1822,131 +1782,149 @@ impl DropVictim {
/// type variables and then we also attempt to evaluate recursive
/// bounds to see if they are satisfied.
impl<'tcx> SelectionContext<'_, 'tcx> {
- /// Returns `DropVictim::Yes` if `victim` should be dropped in favor of
- /// `other`. Generally speaking we will drop duplicate
- /// candidates and prefer where-clause candidates.
+ /// If there are multiple ways to prove a trait goal, we make some
+ /// *fairly arbitrary* choices about which candidate is actually used.
///
- /// See the comment for "SelectionCandidate" for more details.
- #[instrument(level = "debug", skip(self))]
- fn candidate_should_be_dropped_in_favor_of(
+ /// For more details, look at the implementation of this method :)
+ #[instrument(level = "debug", skip(self), ret)]
+ fn winnow_candidates(
&mut self,
- victim: &EvaluatedCandidate<'tcx>,
- other: &EvaluatedCandidate<'tcx>,
has_non_region_infer: bool,
- ) -> DropVictim {
- if victim.candidate == other.candidate {
- return DropVictim::Yes;
+ mut candidates: Vec<EvaluatedCandidate<'tcx>>,
+ ) -> Option<SelectionCandidate<'tcx>> {
+ if candidates.len() == 1 {
+ return Some(candidates.pop().unwrap().candidate);
+ }
+
+ // We prefer trivial builtin candidates, i.e. builtin impls without any nested
+ // requirements, over all others. This is a fix for #53123 and prevents winnowing
+ // from accidentally extending the lifetime of a variable.
+ let mut trivial_builtin = candidates
+ .iter()
+ .filter(|c| matches!(c.candidate, BuiltinCandidate { has_nested: false }));
+ if let Some(_trivial) = trivial_builtin.next() {
+ // There should only ever be a single trivial builtin candidate
+ // as they would otherwise overlap.
+ debug_assert_eq!(trivial_builtin.next(), None);
+ return Some(BuiltinCandidate { has_nested: false });
}
- // Check if a bound would previously have been removed when normalizing
- // the param_env so that it can be given the lowest priority. See
- // #50825 for the motivation for this.
- let is_global =
- |cand: ty::PolyTraitPredicate<'tcx>| cand.is_global() && !cand.has_bound_vars();
+ // Before we consider where-bounds, we have to deduplicate them here and also
+ // drop where-bounds in case the same where-bound exists without bound vars.
+ // This is necessary as elaborating super-trait bounds may result in duplicates.
+ 'search_victim: loop {
+ for (i, this) in candidates.iter().enumerate() {
+ let ParamCandidate(this) = this.candidate else { continue };
+ for (j, other) in candidates.iter().enumerate() {
+ if i == j {
+ continue;
+ }
- // (*) Prefer `BuiltinCandidate { has_nested: false }`, `PointeeCandidate`,
- // or `DiscriminantKindCandidate` to anything else.
- //
- // This is a fix for #53123 and prevents winnowing from accidentally extending the
- // lifetime of a variable.
- match (&other.candidate, &victim.candidate) {
- // FIXME(@jswrenn): this should probably be more sophisticated
- (TransmutabilityCandidate, _) | (_, TransmutabilityCandidate) => DropVictim::No,
-
- // (*)
- (BuiltinCandidate { has_nested: false }, _) => DropVictim::Yes,
- (_, BuiltinCandidate { has_nested: false }) => DropVictim::No,
-
- (ParamCandidate(other), ParamCandidate(victim)) => {
- let same_except_bound_vars = other.skip_binder().trait_ref
- == victim.skip_binder().trait_ref
- && other.skip_binder().polarity == victim.skip_binder().polarity
- && !other.skip_binder().trait_ref.has_escaping_bound_vars();
- if same_except_bound_vars {
- // See issue #84398. In short, we can generate multiple ParamCandidates which are
- // the same except for unused bound vars. Just pick the one with the fewest bound vars
- // or the current one if tied (they should both evaluate to the same answer). This is
- // probably best characterized as a "hack", since we might prefer to just do our
- // best to *not* create essentially duplicate candidates in the first place.
- DropVictim::drop_if(other.bound_vars().len() <= victim.bound_vars().len())
- } else {
- DropVictim::No
+ let ParamCandidate(other) = other.candidate else { continue };
+ if this == other {
+ candidates.remove(j);
+ continue 'search_victim;
+ }
+
+ if this.skip_binder().trait_ref == other.skip_binder().trait_ref
+ && this.skip_binder().polarity == other.skip_binder().polarity
+ && !this.skip_binder().trait_ref.has_escaping_bound_vars()
+ {
+ candidates.remove(j);
+ continue 'search_victim;
+ }
}
}
- (
- ParamCandidate(other_cand),
- ImplCandidate(..)
- | AutoImplCandidate
- | ClosureCandidate { .. }
- | AsyncClosureCandidate
- | AsyncFnKindHelperCandidate
- | CoroutineCandidate
- | FutureCandidate
- | IteratorCandidate
- | AsyncIteratorCandidate
- | FnPointerCandidate { .. }
- | BuiltinObjectCandidate
- | BuiltinUnsizeCandidate
- | TraitUpcastingUnsizeCandidate(_)
- | BuiltinCandidate { .. }
- | TraitAliasCandidate
- | ObjectCandidate(_)
- | ProjectionCandidate(_),
- ) => {
- // We have a where clause so don't go around looking
- // for impls. Arbitrarily give param candidates priority
- // over projection and object candidates.
- //
- // Global bounds from the where clause should be ignored
- // here (see issue #50825).
- DropVictim::drop_if(!is_global(*other_cand))
- }
- (ObjectCandidate(_) | ProjectionCandidate(_), ParamCandidate(victim_cand)) => {
- // Prefer these to a global where-clause bound
- // (see issue #50825).
- if is_global(*victim_cand) { DropVictim::Yes } else { DropVictim::No }
+ break;
+ }
+
+ // The next highest priority is for non-global where-bounds. However, while we don't
+ // prefer global where-clauses here, we do bail with ambiguity when encountering both
+ // a global and a non-global where-clause.
+ //
+ // Our handling of where-bounds is generally fairly messy but necessary for backwards
+ // compatability, see #50825 for why we need to handle global where-bounds like this.
+ let is_global = |c: ty::PolyTraitPredicate<'tcx>| c.is_global() && !c.has_bound_vars();
+ let param_candidates = candidates
+ .iter()
+ .filter_map(|c| if let ParamCandidate(p) = c.candidate { Some(p) } else { None });
+ let mut has_global_bounds = false;
+ let mut param_candidate = None;
+ for c in param_candidates {
+ if is_global(c) {
+ has_global_bounds = true;
+ } else if param_candidate.replace(c).is_some() {
+ // Ambiguity, two potentially different where-clauses
+ return None;
}
- (
- ImplCandidate(_)
- | AutoImplCandidate
- | ClosureCandidate { .. }
- | AsyncClosureCandidate
- | AsyncFnKindHelperCandidate
- | CoroutineCandidate
- | FutureCandidate
- | IteratorCandidate
- | AsyncIteratorCandidate
- | FnPointerCandidate { .. }
- | BuiltinObjectCandidate
- | BuiltinUnsizeCandidate
- | TraitUpcastingUnsizeCandidate(_)
- | BuiltinCandidate { has_nested: true }
- | TraitAliasCandidate,
- ParamCandidate(victim_cand),
- ) => {
- // Prefer these to a global where-clause bound
- // (see issue #50825).
- DropVictim::drop_if(
- is_global(*victim_cand) && other.evaluation.must_apply_modulo_regions(),
- )
+ }
+ if let Some(predicate) = param_candidate {
+ // Ambiguity, a global and a non-global where-bound.
+ if has_global_bounds {
+ return None;
+ } else {
+ return Some(ParamCandidate(predicate));
}
+ }
+
+ // Prefer alias-bounds over blanket impls for rigid associated types. This is
+ // fairly arbitrary but once again necessary for backwards compatibility.
+ // If there are multiple applicable candidates which don't affect type inference,
+ // choose the one with the lowest index.
+ let alias_bound = candidates
+ .iter()
+ .filter_map(|c| if let ProjectionCandidate(i) = c.candidate { Some(i) } else { None })
+ .try_reduce(|c1, c2| if has_non_region_infer { None } else { Some(c1.min(c2)) });
+ match alias_bound {
+ Some(Some(index)) => return Some(ProjectionCandidate(index)),
+ Some(None) => {}
+ None => return None,
+ }
+
+ // Need to prioritize builtin trait object impls as `<dyn Any as Any>::type_id`
+ // should use the vtable method and not the method provided by the user-defined
+ // impl `impl<T: ?Sized> Any for T { .. }`. This really shouldn't exist but is
+ // necessary due to #57893. We again arbitrarily prefer the applicable candidate
+ // with the lowest index.
+ let object_bound = candidates
+ .iter()
+ .filter_map(|c| if let ObjectCandidate(i) = c.candidate { Some(i) } else { None })
+ .try_reduce(|c1, c2| if has_non_region_infer { None } else { Some(c1.min(c2)) });
+ match object_bound {
+ Some(Some(index)) => return Some(ObjectCandidate(index)),
+ Some(None) => {}
+ None => return None,
+ }
- (ProjectionCandidate(i), ProjectionCandidate(j))
- | (ObjectCandidate(i), ObjectCandidate(j)) => {
- // Arbitrarily pick the lower numbered candidate for backwards
- // compatibility reasons. Don't let this affect inference.
- DropVictim::drop_if(i < j && !has_non_region_infer)
+ // Finally, handle overlapping user-written impls.
+ let impls = candidates.iter().filter_map(|c| {
+ if let ImplCandidate(def_id) = c.candidate {
+ Some((def_id, c.evaluation))
+ } else {
+ None
}
- (ObjectCandidate(_), ProjectionCandidate(_))
- | (ProjectionCandidate(_), ObjectCandidate(_)) => {
- bug!("Have both object and projection candidate")
+ });
+ let mut impl_candidate = None;
+ for c in impls {
+ if let Some(prev) = impl_candidate.replace(c) {
+ if self.prefer_lhs_over_victim(has_non_region_infer, c, prev) {
+ // Ok, prefer `c` over the previous entry
+ } else if self.prefer_lhs_over_victim(has_non_region_infer, prev, c) {
+ // Ok, keep `prev` instead of the new entry
+ impl_candidate = Some(prev);
+ } else {
+ // Ambiguity, two potentially different where-clauses
+ return None;
+ }
}
-
- // Arbitrarily give projection and object candidates priority.
- (
- ObjectCandidate(_) | ProjectionCandidate(_),
- ImplCandidate(..)
+ }
+ if let Some((def_id, _evaluation)) = impl_candidate {
+ // Don't use impl candidates which overlap with other candidates.
+ // This should pretty much only ever happen with malformed impls.
+ if candidates.iter().all(|c| match c.candidate {
+ BuiltinCandidate { has_nested: _ }
+ | TransmutabilityCandidate
| AutoImplCandidate
| ClosureCandidate { .. }
| AsyncClosureCandidate
@@ -1955,155 +1933,113 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
| FutureCandidate
| IteratorCandidate
| AsyncIteratorCandidate
- | FnPointerCandidate { .. }
- | BuiltinObjectCandidate
- | BuiltinUnsizeCandidate
+ | FnPointerCandidate
+ | TraitAliasCandidate
| TraitUpcastingUnsizeCandidate(_)
- | BuiltinCandidate { .. }
- | TraitAliasCandidate,
- ) => DropVictim::Yes,
-
- (
- ImplCandidate(..)
- | AutoImplCandidate
- | ClosureCandidate { .. }
- | AsyncClosureCandidate
- | AsyncFnKindHelperCandidate
- | CoroutineCandidate
- | FutureCandidate
- | IteratorCandidate
- | AsyncIteratorCandidate
- | FnPointerCandidate { .. }
| BuiltinObjectCandidate
- | BuiltinUnsizeCandidate
- | TraitUpcastingUnsizeCandidate(_)
- | BuiltinCandidate { .. }
- | TraitAliasCandidate,
- ObjectCandidate(_) | ProjectionCandidate(_),
- ) => DropVictim::No,
-
- (&ImplCandidate(other_def), &ImplCandidate(victim_def)) => {
- // See if we can toss out `victim` based on specialization.
- // While this requires us to know *for sure* that the `other` impl applies
- // we still use modulo regions here.
- //
- // This is fine as specialization currently assumes that specializing
- // impls have to be always applicable, meaning that the only allowed
- // region constraints may be constraints also present on the default impl.
- let tcx = self.tcx();
- if other.evaluation.must_apply_modulo_regions()
- && tcx.specializes((other_def, victim_def))
- {
- return DropVictim::Yes;
- }
-
- match tcx.impls_are_allowed_to_overlap(other_def, victim_def) {
- // For #33140 the impl headers must be exactly equal, the trait must not have
- // any associated items and there are no where-clauses.
- //
- // We can just arbitrarily drop one of the impls.
- Some(ty::ImplOverlapKind::FutureCompatOrderDepTraitObjects) => {
- assert_eq!(other.evaluation, victim.evaluation);
- DropVictim::Yes
- }
- // For candidates which already reference errors it doesn't really
- // matter what we do 🤷
- Some(ty::ImplOverlapKind::Permitted { marker: false }) => {
- DropVictim::drop_if(other.evaluation.must_apply_considering_regions())
- }
- Some(ty::ImplOverlapKind::Permitted { marker: true }) => {
- // Subtle: If the predicate we are evaluating has inference
- // variables, do *not* allow discarding candidates due to
- // marker trait impls.
- //
- // Without this restriction, we could end up accidentally
- // constraining inference variables based on an arbitrarily
- // chosen trait impl.
- //
- // Imagine we have the following code:
- //
- // ```rust
- // #[marker] trait MyTrait {}
- // impl MyTrait for u8 {}
- // impl MyTrait for bool {}
- // ```
- //
- // And we are evaluating the predicate `<_#0t as MyTrait>`.
- //
- // During selection, we will end up with one candidate for each
- // impl of `MyTrait`. If we were to discard one impl in favor
- // of the other, we would be left with one candidate, causing
- // us to "successfully" select the predicate, unifying
- // _#0t with (for example) `u8`.
- //
- // However, we have no reason to believe that this unification
- // is correct - we've essentially just picked an arbitrary
- // *possibility* for _#0t, and required that this be the *only*
- // possibility.
- //
- // Eventually, we will either:
- // 1) Unify all inference variables in the predicate through
- // some other means (e.g. type-checking of a function). We will
- // then be in a position to drop marker trait candidates
- // without constraining inference variables (since there are
- // none left to constrain)
- // 2) Be left with some unconstrained inference variables. We
- // will then correctly report an inference error, since the
- // existence of multiple marker trait impls tells us nothing
- // about which one should actually apply.
- DropVictim::drop_if(
- !has_non_region_infer
- && other.evaluation.must_apply_considering_regions(),
- )
- }
- None => DropVictim::No,
- }
+ | BuiltinUnsizeCandidate => false,
+ // Non-global param candidates have already been handled, global
+ // where-bounds get ignored.
+ ParamCandidate(_) | ImplCandidate(_) => true,
+ ProjectionCandidate(_) | ObjectCandidate(_) => unreachable!(),
+ }) {
+ return Some(ImplCandidate(def_id));
+ } else {
+ return None;
}
+ }
- (AutoImplCandidate, ImplCandidate(_)) | (ImplCandidate(_), AutoImplCandidate) => {
- DropVictim::No
- }
+ if candidates.len() == 1 {
+ Some(candidates.pop().unwrap().candidate)
+ } else {
+ // Also try ignoring all global where-bounds and check whether we end
+ // with a unique candidate in this case.
+ let mut not_a_global_where_bound = candidates
+ .into_iter()
+ .filter(|c| !matches!(c.candidate, ParamCandidate(p) if is_global(p)));
+ not_a_global_where_bound
+ .next()
+ .map(|c| c.candidate)
+ .filter(|_| not_a_global_where_bound.next().is_none())
+ }
+ }
- (AutoImplCandidate, _) | (_, AutoImplCandidate) => {
- bug!(
- "default implementations shouldn't be recorded \
- when there are other global candidates: {:?} {:?}",
- other,
- victim
- );
+ fn prefer_lhs_over_victim(
+ &self,
+ has_non_region_infer: bool,
+ (lhs, lhs_evaluation): (DefId, EvaluationResult),
+ (victim, victim_evaluation): (DefId, EvaluationResult),
+ ) -> bool {
+ let tcx = self.tcx();
+ // See if we can toss out `victim` based on specialization.
+ //
+ // While this requires us to know *for sure* that the `lhs` impl applies
+ // we still use modulo regions here. This is fine as specialization currently
+ // assumes that specializing impls have to be always applicable, meaning that
+ // the only allowed region constraints may be constraints also present on the default impl.
+ if lhs_evaluation.must_apply_modulo_regions() {
+ if tcx.specializes((lhs, victim)) {
+ return true;
}
+ }
- // Everything else is ambiguous
- (
- ImplCandidate(_)
- | ClosureCandidate { .. }
- | AsyncClosureCandidate
- | AsyncFnKindHelperCandidate
- | CoroutineCandidate
- | FutureCandidate
- | IteratorCandidate
- | AsyncIteratorCandidate
- | FnPointerCandidate { .. }
- | BuiltinObjectCandidate
- | BuiltinUnsizeCandidate
- | TraitUpcastingUnsizeCandidate(_)
- | BuiltinCandidate { has_nested: true }
- | TraitAliasCandidate,
- ImplCandidate(_)
- | ClosureCandidate { .. }
- | AsyncClosureCandidate
- | AsyncFnKindHelperCandidate
- | CoroutineCandidate
- | FutureCandidate
- | IteratorCandidate
- | AsyncIteratorCandidate
- | FnPointerCandidate { .. }
- | BuiltinObjectCandidate
- | BuiltinUnsizeCandidate
- | TraitUpcastingUnsizeCandidate(_)
- | BuiltinCandidate { has_nested: true }
- | TraitAliasCandidate,
- ) => DropVictim::No,
+ match tcx.impls_are_allowed_to_overlap(lhs, victim) {
+ // For #33140 the impl headers must be exactly equal, the trait must not have
+ // any associated items and there are no where-clauses.
+ //
+ // We can just arbitrarily drop one of the impls.
+ Some(ty::ImplOverlapKind::FutureCompatOrderDepTraitObjects) => {
+ assert_eq!(lhs_evaluation, victim_evaluation);
+ true
+ }
+ // For candidates which already reference errors it doesn't really
+ // matter what we do 🤷
+ Some(ty::ImplOverlapKind::Permitted { marker: false }) => {
+ lhs_evaluation.must_apply_considering_regions()
+ }
+ Some(ty::ImplOverlapKind::Permitted { marker: true }) => {
+ // Subtle: If the predicate we are evaluating has inference
+ // variables, do *not* allow discarding candidates due to
+ // marker trait impls.
+ //
+ // Without this restriction, we could end up accidentally
+ // constraining inference variables based on an arbitrarily
+ // chosen trait impl.
+ //
+ // Imagine we have the following code:
+ //
+ // ```rust
+ // #[marker] trait MyTrait {}
+ // impl MyTrait for u8 {}
+ // impl MyTrait for bool {}
+ // ```
+ //
+ // And we are evaluating the predicate `<_#0t as MyTrait>`.
+ //
+ // During selection, we will end up with one candidate for each
+ // impl of `MyTrait`. If we were to discard one impl in favor
+ // of the other, we would be left with one candidate, causing
+ // us to "successfully" select the predicate, unifying
+ // _#0t with (for example) `u8`.
+ //
+ // However, we have no reason to believe that this unification
+ // is correct - we've essentially just picked an arbitrary
+ // *possibility* for _#0t, and required that this be the *only*
+ // possibility.
+ //
+ // Eventually, we will either:
+ // 1) Unify all inference variables in the predicate through
+ // some other means (e.g. type-checking of a function). We will
+ // then be in a position to drop marker trait candidates
+ // without constraining inference variables (since there are
+ // none left to constrain)
+ // 2) Be left with some unconstrained inference variables. We
+ // will then correctly report an inference error, since the
+ // existence of multiple marker trait impls tells us nothing
+ // about which one should actually apply.
+ !has_non_region_infer && lhs_evaluation.must_apply_considering_regions()
+ }
+ None => false,
}
}
}
diff --git a/tests/ui/traits/winnowing/global-non-global-env-2.rs b/tests/ui/traits/winnowing/global-non-global-env-2.rs
index 1647606934660..c73d0f06cd95b 100644
--- a/tests/ui/traits/winnowing/global-non-global-env-2.rs
+++ b/tests/ui/traits/winnowing/global-non-global-env-2.rs
@@ -1,3 +1,5 @@
+//@ check-pass
+
// A regression test for an edge case of candidate selection
// in the old trait solver, see #132325 for more details. Unlike
// the first test, this one has two impl candidates.
@@ -12,7 +14,7 @@ where
(): Trait<u32>,
(): Trait<T>,
{
- impls_trait(()) //~ ERROR mismatched types
+ impls_trait(())
}
fn main() {}
diff --git a/tests/ui/traits/winnowing/global-non-global-env-2.stderr b/tests/ui/traits/winnowing/global-non-global-env-2.stderr
deleted file mode 100644
index dee01c72e4820..0000000000000
--- a/tests/ui/traits/winnowing/global-non-global-env-2.stderr
+++ /dev/null
@@ -1,17 +0,0 @@
-error[E0308]: mismatched types
- --> $DIR/global-non-global-env-2.rs:15:5
- |
-LL | fn foo<T>() -> u32
- | - --- expected `u32` because of return type
- | |
- | found this type parameter
-...
-LL | impls_trait(())
- | ^^^^^^^^^^^^^^^ expected `u32`, found type parameter `T`
- |
- = note: expected type `u32`
- found type parameter `T`
-
-error: aborting due to 1 previous error
-
-For more information about this error, try `rustc --explain E0308`.
diff --git a/tests/ui/traits/winnowing/global-non-global-env-4.rs b/tests/ui/traits/winnowing/global-non-global-env-4.rs
index 1f35cf20f83dc..74793620c9e7d 100644
--- a/tests/ui/traits/winnowing/global-non-global-env-4.rs
+++ b/tests/ui/traits/winnowing/global-non-global-env-4.rs
@@ -1,3 +1,5 @@
+//@ check-pass
+
// A regression test for an edge case of candidate selection
// in the old trait solver, see #132325 for more details. Unlike
// the third test, this one has 3 impl candidates.
@@ -13,7 +15,7 @@ where
(): Trait<T>,
(): Trait<u32>,
{
- impls_trait(()) //~ ERROR mismatched types
+ impls_trait(())
}
fn main() {}
diff --git a/tests/ui/traits/winnowing/global-non-global-env-4.stderr b/tests/ui/traits/winnowing/global-non-global-env-4.stderr
deleted file mode 100644
index 6449f3feb08cf..0000000000000
--- a/tests/ui/traits/winnowing/global-non-global-env-4.stderr
+++ /dev/null
@@ -1,17 +0,0 @@
-error[E0308]: mismatched types
- --> $DIR/global-non-global-env-4.rs:16:5
- |
-LL | fn foo<T>() -> u32
- | - --- expected `u32` because of return type
- | |
- | found this type parameter
-...
-LL | impls_trait(())
- | ^^^^^^^^^^^^^^^ expected `u32`, found type parameter `T`
- |
- = note: expected type `u32`
- found type parameter `T`
-
-error: aborting due to 1 previous error
-
-For more information about this error, try `rustc --explain E0308`.