Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 6d565b7

Browse files
borsbors
committedOct 29, 2024·
Auto merge of rust-lang#132325 - lcnr:damn-son, r=<try>
rework winnowing to sensibly handle global where-bounds this is somewhat weird, but it at least allows us to mirror this behavior in the new solver: - trivial builtin-impls - non-global where-bounds, bailing with ambiguity if at least one global where-bound exists - object ☠️ + alias-bound candidates - merge candidates ignoring global where-bounds r? `@compiler-errors`
2 parents 2dece5b + dc50c64 commit 6d565b7

File tree

2 files changed

+230
-304
lines changed

2 files changed

+230
-304
lines changed
 

‎compiler/rustc_trait_selection/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#![feature(extract_if)]
2424
#![feature(if_let_guard)]
2525
#![feature(iter_intersperse)]
26+
#![feature(iterator_try_reduce)]
2627
#![feature(let_chains)]
2728
#![feature(never_type)]
2829
#![feature(rustdoc_internals)]

‎compiler/rustc_trait_selection/src/traits/select/mod.rs

Lines changed: 229 additions & 304 deletions
Original file line numberDiff line numberDiff line change
@@ -448,7 +448,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
448448

449449
// Winnow, but record the exact outcome of evaluation, which
450450
// is needed for specialization. Propagate overflow if it occurs.
451-
let mut candidates = candidates
451+
let candidates = candidates
452452
.into_iter()
453453
.map(|c| match self.evaluate_candidate(stack, &c) {
454454
Ok(eval) if eval.may_apply() => {
@@ -461,40 +461,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
461461
.flat_map(Result::transpose)
462462
.collect::<Result<Vec<_>, _>>()?;
463463

464-
debug!(?stack, ?candidates, "winnowed to {} candidates", candidates.len());
465-
466-
let has_non_region_infer = stack.obligation.predicate.has_non_region_infer();
467-
468-
// If there are STILL multiple candidates, we can further
469-
// reduce the list by dropping duplicates -- including
470-
// resolving specializations.
471-
if candidates.len() > 1 {
472-
let mut i = 0;
473-
while i < candidates.len() {
474-
let should_drop_i = (0..candidates.len()).filter(|&j| i != j).any(|j| {
475-
self.candidate_should_be_dropped_in_favor_of(
476-
&candidates[i],
477-
&candidates[j],
478-
has_non_region_infer,
479-
) == DropVictim::Yes
480-
});
481-
if should_drop_i {
482-
debug!(candidate = ?candidates[i], "Dropping candidate #{}/{}", i, candidates.len());
483-
candidates.swap_remove(i);
484-
} else {
485-
debug!(candidate = ?candidates[i], "Retaining candidate #{}/{}", i, candidates.len());
486-
i += 1;
487-
488-
// If there are *STILL* multiple candidates, give up
489-
// and report ambiguity.
490-
if i > 1 {
491-
debug!("multiple matches, ambig");
492-
return Ok(None);
493-
}
494-
}
495-
}
496-
}
497-
464+
debug!(?stack, ?candidates, "{} potentially applicable candidates", candidates.len());
498465
// If there are *NO* candidates, then there are no impls --
499466
// that we know of, anyway. Note that in the case where there
500467
// are unbound type variables within the obligation, it might
@@ -511,13 +478,18 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
511478
// to have emitted at least one.
512479
if stack.obligation.predicate.references_error() {
513480
debug!(?stack.obligation.predicate, "found error type in predicate, treating as ambiguous");
514-
return Ok(None);
481+
Ok(None)
482+
} else {
483+
Err(Unimplemented)
484+
}
485+
} else {
486+
let has_non_region_infer = stack.obligation.predicate.has_non_region_infer();
487+
if let Some(candidate) = self.winnow_candidates(has_non_region_infer, candidates) {
488+
self.filter_reservation_impls(candidate)
489+
} else {
490+
Ok(None)
515491
}
516-
return Err(Unimplemented);
517492
}
518-
519-
// Just one candidate left.
520-
self.filter_reservation_impls(candidates.pop().unwrap().candidate)
521493
}
522494

523495
///////////////////////////////////////////////////////////////////////////
@@ -1794,150 +1766,149 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
17941766
}
17951767
}
17961768

1797-
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
1798-
enum DropVictim {
1799-
Yes,
1800-
No,
1801-
}
1802-
1803-
impl DropVictim {
1804-
fn drop_if(should_drop: bool) -> DropVictim {
1805-
if should_drop { DropVictim::Yes } else { DropVictim::No }
1806-
}
1807-
}
1808-
18091769
/// ## Winnowing
18101770
///
18111771
/// Winnowing is the process of attempting to resolve ambiguity by
18121772
/// probing further. During the winnowing process, we unify all
18131773
/// type variables and then we also attempt to evaluate recursive
18141774
/// bounds to see if they are satisfied.
18151775
impl<'tcx> SelectionContext<'_, 'tcx> {
1816-
/// Returns `DropVictim::Yes` if `victim` should be dropped in favor of
1817-
/// `other`. Generally speaking we will drop duplicate
1818-
/// candidates and prefer where-clause candidates.
1776+
/// If there are multiple ways to prove a trait goal, we make some
1777+
/// *fairly arbitrary* choices about which candidate is actually used.
18191778
///
1820-
/// See the comment for "SelectionCandidate" for more details.
1821-
#[instrument(level = "debug", skip(self))]
1822-
fn candidate_should_be_dropped_in_favor_of(
1779+
/// For more details, look at the implementation of this method :)
1780+
#[instrument(level = "debug", skip(self), ret)]
1781+
fn winnow_candidates(
18231782
&mut self,
1824-
victim: &EvaluatedCandidate<'tcx>,
1825-
other: &EvaluatedCandidate<'tcx>,
18261783
has_non_region_infer: bool,
1827-
) -> DropVictim {
1828-
if victim.candidate == other.candidate {
1829-
return DropVictim::Yes;
1784+
mut candidates: Vec<EvaluatedCandidate<'tcx>>,
1785+
) -> Option<SelectionCandidate<'tcx>> {
1786+
if candidates.len() == 1 {
1787+
return Some(candidates.pop().unwrap().candidate);
18301788
}
18311789

1832-
// Check if a bound would previously have been removed when normalizing
1833-
// the param_env so that it can be given the lowest priority. See
1834-
// #50825 for the motivation for this.
1835-
let is_global =
1836-
|cand: ty::PolyTraitPredicate<'tcx>| cand.is_global() && !cand.has_bound_vars();
1790+
// We prefer trivial builtin candidates, i.e. builtin impls without any nested
1791+
// requirements, over all others. This is a fix for #53123 and prevents winnowing
1792+
// from accidentally extending the lifetime of a variable.
1793+
let mut trivial_builtin = candidates
1794+
.iter()
1795+
.filter(|c| matches!(c.candidate, BuiltinCandidate { has_nested: false }));
1796+
if let Some(_trivial) = trivial_builtin.next() {
1797+
// There should only ever be a single trivial builtin candidate
1798+
// as they would otherwise overlap.
1799+
debug_assert_eq!(trivial_builtin.next(), None);
1800+
return Some(BuiltinCandidate { has_nested: false });
1801+
}
18371802

1838-
// (*) Prefer `BuiltinCandidate { has_nested: false }`, `PointeeCandidate`,
1839-
// or `DiscriminantKindCandidate` to anything else.
1803+
// The next highest priority is for non-global where-bounds. However, there are
1804+
// two additional rules here:
1805+
// - if there are also global where-bound which may apply, we bail with ambiguity
1806+
// instead of prefering the non-global where-bound. Note that if there are
1807+
// only global where-bounds, they get ignored
1808+
// - if there are two where-bounds which only differ in their bound vars, but are
1809+
// otherwise equal, we arbitrarily prefer one of them
18401810
//
1841-
// This is a fix for #53123 and prevents winnowing from accidentally extending the
1842-
// lifetime of a variable.
1843-
match (&other.candidate, &victim.candidate) {
1844-
// FIXME(@jswrenn): this should probably be more sophisticated
1845-
(TransmutabilityCandidate, _) | (_, TransmutabilityCandidate) => DropVictim::No,
1846-
1847-
// (*)
1848-
(BuiltinCandidate { has_nested: false }, _) => DropVictim::Yes,
1849-
(_, BuiltinCandidate { has_nested: false }) => DropVictim::No,
1850-
1851-
(ParamCandidate(other), ParamCandidate(victim)) => {
1852-
let same_except_bound_vars = other.skip_binder().trait_ref
1853-
== victim.skip_binder().trait_ref
1854-
&& other.skip_binder().polarity == victim.skip_binder().polarity
1855-
&& !other.skip_binder().trait_ref.has_escaping_bound_vars();
1856-
if same_except_bound_vars {
1857-
// See issue #84398. In short, we can generate multiple ParamCandidates which are
1858-
// the same except for unused bound vars. Just pick the one with the fewest bound vars
1859-
// or the current one if tied (they should both evaluate to the same answer). This is
1860-
// probably best characterized as a "hack", since we might prefer to just do our
1861-
// best to *not* create essentially duplicate candidates in the first place.
1862-
DropVictim::drop_if(other.bound_vars().len() <= victim.bound_vars().len())
1811+
// This is fairly messy but necessary for backwards compatability, see #50825 for why
1812+
// we need to handle global where-bounds like this.
1813+
let param_candidates = candidates
1814+
.iter()
1815+
.filter_map(|c| if let ParamCandidate(p) = c.candidate { Some(p) } else { None });
1816+
let is_global = |c: ty::PolyTraitPredicate<'tcx>| c.is_global() && !c.has_bound_vars();
1817+
let mut has_non_global = false;
1818+
let mut param_candidate = None;
1819+
for c in param_candidates {
1820+
has_non_global |= !is_global(c);
1821+
let has_fewer_bound_vars =
1822+
|this: ty::PolyTraitPredicate<'tcx>, other: ty::PolyTraitPredicate<'tcx>| {
1823+
this.skip_binder().trait_ref == other.skip_binder().trait_ref
1824+
&& this.skip_binder().polarity == other.skip_binder().polarity
1825+
&& !this.skip_binder().trait_ref.has_escaping_bound_vars()
1826+
};
1827+
if let Some(prev) = param_candidate.replace(c) {
1828+
if has_fewer_bound_vars(c, prev) {
1829+
// Ok, prefer `c` over the previous entry
1830+
} else if has_fewer_bound_vars(prev, c) {
1831+
// Ok, keep `prev` instead of the new entry
1832+
param_candidate = Some(prev);
18631833
} else {
1864-
DropVictim::No
1834+
// Ambiguity, two potentially different where-clauses
1835+
return None;
18651836
}
18661837
}
1867-
1868-
(
1869-
ParamCandidate(other_cand),
1870-
ImplCandidate(..)
1871-
| AutoImplCandidate
1872-
| ClosureCandidate { .. }
1873-
| AsyncClosureCandidate
1874-
| AsyncFnKindHelperCandidate
1875-
| CoroutineCandidate
1876-
| FutureCandidate
1877-
| IteratorCandidate
1878-
| AsyncIteratorCandidate
1879-
| FnPointerCandidate { .. }
1880-
| BuiltinObjectCandidate
1881-
| BuiltinUnsizeCandidate
1882-
| TraitUpcastingUnsizeCandidate(_)
1883-
| BuiltinCandidate { .. }
1884-
| TraitAliasCandidate
1885-
| ObjectCandidate(_)
1886-
| ProjectionCandidate(_),
1887-
) => {
1888-
// We have a where clause so don't go around looking
1889-
// for impls. Arbitrarily give param candidates priority
1890-
// over projection and object candidates.
1838+
}
1839+
if let Some(predicate) = param_candidate {
1840+
if is_global(predicate) {
1841+
// If we only have global where-bounds, we prefer
1842+
// impls over them, otherwise we bail with ambiguity.
18911843
//
1892-
// Global bounds from the where clause should be ignored
1893-
// here (see issue #50825).
1894-
DropVictim::drop_if(!is_global(*other_cand))
1895-
}
1896-
(ObjectCandidate(_) | ProjectionCandidate(_), ParamCandidate(victim_cand)) => {
1897-
// Prefer these to a global where-clause bound
1898-
// (see issue #50825).
1899-
if is_global(*victim_cand) { DropVictim::Yes } else { DropVictim::No }
1900-
}
1901-
(
1902-
ImplCandidate(_)
1903-
| AutoImplCandidate
1904-
| ClosureCandidate { .. }
1905-
| AsyncClosureCandidate
1906-
| AsyncFnKindHelperCandidate
1907-
| CoroutineCandidate
1908-
| FutureCandidate
1909-
| IteratorCandidate
1910-
| AsyncIteratorCandidate
1911-
| FnPointerCandidate { .. }
1912-
| BuiltinObjectCandidate
1913-
| BuiltinUnsizeCandidate
1914-
| TraitUpcastingUnsizeCandidate(_)
1915-
| BuiltinCandidate { has_nested: true }
1916-
| TraitAliasCandidate,
1917-
ParamCandidate(victim_cand),
1918-
) => {
1919-
// Prefer these to a global where-clause bound
1920-
// (see issue #50825).
1921-
DropVictim::drop_if(
1922-
is_global(*victim_cand) && other.evaluation.must_apply_modulo_regions(),
1923-
)
1844+
// This is only reachable if there's one higher-ranked
1845+
// and one non-higher ranked version of a global
1846+
// where-clause.
1847+
if has_non_global {
1848+
return None;
1849+
}
1850+
} else {
1851+
return Some(ParamCandidate(predicate));
19241852
}
1853+
}
1854+
1855+
// Prefer alias-bounds over blanket impls for rigid associated types. This is
1856+
// fairly arbitrary but once again necessary for backwards compatibility.
1857+
// If there are multiple applicable candidates which don't affect type inference,
1858+
// choose the one with the lowest index.
1859+
let alias_bound = candidates
1860+
.iter()
1861+
.filter_map(|c| if let ProjectionCandidate(i) = c.candidate { Some(i) } else { None })
1862+
.try_reduce(|c1, c2| if has_non_region_infer { None } else { Some(c1.min(c2)) });
1863+
match alias_bound {
1864+
Some(Some(index)) => return Some(ProjectionCandidate(index)),
1865+
Some(None) => {}
1866+
None => return None,
1867+
}
1868+
1869+
// Need to prioritize builtin trait object impls as `<dyn Any as Any>::type_id`
1870+
// should use the vtable method and not the method provided by the user-defined
1871+
// impl `impl<T: ?Sized> Any for T { .. }`. This really shouldn't exist but is
1872+
// necessary due to #57893. We again arbitrarily prefer the applicable candidate
1873+
// with the lowest index.
1874+
let object_bound = candidates
1875+
.iter()
1876+
.filter_map(|c| if let ObjectCandidate(i) = c.candidate { Some(i) } else { None })
1877+
.try_reduce(|c1, c2| if has_non_region_infer { None } else { Some(c1.min(c2)) });
1878+
match object_bound {
1879+
Some(Some(index)) => return Some(ObjectCandidate(index)),
1880+
Some(None) => {}
1881+
None => return None,
1882+
}
19251883

1926-
(ProjectionCandidate(i), ProjectionCandidate(j))
1927-
| (ObjectCandidate(i), ObjectCandidate(j)) => {
1928-
// Arbitrarily pick the lower numbered candidate for backwards
1929-
// compatibility reasons. Don't let this affect inference.
1930-
DropVictim::drop_if(i < j && !has_non_region_infer)
1884+
// Finally, handle overlapping user-written impls.
1885+
let impls = candidates.iter().filter_map(|c| {
1886+
if let ImplCandidate(def_id) = c.candidate {
1887+
Some((def_id, c.evaluation))
1888+
} else {
1889+
None
19311890
}
1932-
(ObjectCandidate(_), ProjectionCandidate(_))
1933-
| (ProjectionCandidate(_), ObjectCandidate(_)) => {
1934-
bug!("Have both object and projection candidate")
1891+
});
1892+
let mut impl_candidate = None;
1893+
for c in impls {
1894+
if let Some(prev) = impl_candidate.replace(c) {
1895+
if self.prefer_lhs_over_victim(has_non_region_infer, c, prev) {
1896+
// Ok, prefer `c` over the previous entry
1897+
} else if self.prefer_lhs_over_victim(has_non_region_infer, prev, c) {
1898+
// Ok, keep `prev` instead of the new entry
1899+
impl_candidate = Some(prev);
1900+
} else {
1901+
// Ambiguity, two potentially different where-clauses
1902+
return None;
1903+
}
19351904
}
1936-
1937-
// Arbitrarily give projection and object candidates priority.
1938-
(
1939-
ObjectCandidate(_) | ProjectionCandidate(_),
1940-
ImplCandidate(..)
1905+
}
1906+
if let Some((def_id, _evaluation)) = impl_candidate {
1907+
// Don't use impl candidates which overlap with other candidates.
1908+
// This should pretty much only ever happen with malformed impls.
1909+
if candidates.iter().all(|c| match c.candidate {
1910+
BuiltinCandidate { has_nested: _ }
1911+
| TransmutabilityCandidate
19411912
| AutoImplCandidate
19421913
| ClosureCandidate { .. }
19431914
| AsyncClosureCandidate
@@ -1946,155 +1917,109 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
19461917
| FutureCandidate
19471918
| IteratorCandidate
19481919
| AsyncIteratorCandidate
1949-
| FnPointerCandidate { .. }
1950-
| BuiltinObjectCandidate
1951-
| BuiltinUnsizeCandidate
1920+
| FnPointerCandidate
1921+
| TraitAliasCandidate
19521922
| TraitUpcastingUnsizeCandidate(_)
1953-
| BuiltinCandidate { .. }
1954-
| TraitAliasCandidate,
1955-
) => DropVictim::Yes,
1956-
1957-
(
1958-
ImplCandidate(..)
1959-
| AutoImplCandidate
1960-
| ClosureCandidate { .. }
1961-
| AsyncClosureCandidate
1962-
| AsyncFnKindHelperCandidate
1963-
| CoroutineCandidate
1964-
| FutureCandidate
1965-
| IteratorCandidate
1966-
| AsyncIteratorCandidate
1967-
| FnPointerCandidate { .. }
19681923
| BuiltinObjectCandidate
1969-
| BuiltinUnsizeCandidate
1970-
| TraitUpcastingUnsizeCandidate(_)
1971-
| BuiltinCandidate { .. }
1972-
| TraitAliasCandidate,
1973-
ObjectCandidate(_) | ProjectionCandidate(_),
1974-
) => DropVictim::No,
1975-
1976-
(&ImplCandidate(other_def), &ImplCandidate(victim_def)) => {
1977-
// See if we can toss out `victim` based on specialization.
1978-
// While this requires us to know *for sure* that the `other` impl applies
1979-
// we still use modulo regions here.
1980-
//
1981-
// This is fine as specialization currently assumes that specializing
1982-
// impls have to be always applicable, meaning that the only allowed
1983-
// region constraints may be constraints also present on the default impl.
1984-
let tcx = self.tcx();
1985-
if other.evaluation.must_apply_modulo_regions()
1986-
&& tcx.specializes((other_def, victim_def))
1987-
{
1988-
return DropVictim::Yes;
1989-
}
1990-
1991-
match tcx.impls_are_allowed_to_overlap(other_def, victim_def) {
1992-
// For #33140 the impl headers must be exactly equal, the trait must not have
1993-
// any associated items and there are no where-clauses.
1994-
//
1995-
// We can just arbitrarily drop one of the impls.
1996-
Some(ty::ImplOverlapKind::FutureCompatOrderDepTraitObjects) => {
1997-
assert_eq!(other.evaluation, victim.evaluation);
1998-
DropVictim::Yes
1999-
}
2000-
// For candidates which already reference errors it doesn't really
2001-
// matter what we do 🤷
2002-
Some(ty::ImplOverlapKind::Permitted { marker: false }) => {
2003-
DropVictim::drop_if(other.evaluation.must_apply_considering_regions())
2004-
}
2005-
Some(ty::ImplOverlapKind::Permitted { marker: true }) => {
2006-
// Subtle: If the predicate we are evaluating has inference
2007-
// variables, do *not* allow discarding candidates due to
2008-
// marker trait impls.
2009-
//
2010-
// Without this restriction, we could end up accidentally
2011-
// constraining inference variables based on an arbitrarily
2012-
// chosen trait impl.
2013-
//
2014-
// Imagine we have the following code:
2015-
//
2016-
// ```rust
2017-
// #[marker] trait MyTrait {}
2018-
// impl MyTrait for u8 {}
2019-
// impl MyTrait for bool {}
2020-
// ```
2021-
//
2022-
// And we are evaluating the predicate `<_#0t as MyTrait>`.
2023-
//
2024-
// During selection, we will end up with one candidate for each
2025-
// impl of `MyTrait`. If we were to discard one impl in favor
2026-
// of the other, we would be left with one candidate, causing
2027-
// us to "successfully" select the predicate, unifying
2028-
// _#0t with (for example) `u8`.
2029-
//
2030-
// However, we have no reason to believe that this unification
2031-
// is correct - we've essentially just picked an arbitrary
2032-
// *possibility* for _#0t, and required that this be the *only*
2033-
// possibility.
2034-
//
2035-
// Eventually, we will either:
2036-
// 1) Unify all inference variables in the predicate through
2037-
// some other means (e.g. type-checking of a function). We will
2038-
// then be in a position to drop marker trait candidates
2039-
// without constraining inference variables (since there are
2040-
// none left to constrain)
2041-
// 2) Be left with some unconstrained inference variables. We
2042-
// will then correctly report an inference error, since the
2043-
// existence of multiple marker trait impls tells us nothing
2044-
// about which one should actually apply.
2045-
DropVictim::drop_if(
2046-
!has_non_region_infer
2047-
&& other.evaluation.must_apply_considering_regions(),
2048-
)
2049-
}
2050-
None => DropVictim::No,
2051-
}
1924+
| BuiltinUnsizeCandidate => false,
1925+
// Non-global param candidates have already been handled, global
1926+
// where-bounds get ignored.
1927+
ParamCandidate(_) | ImplCandidate(_) => true,
1928+
ProjectionCandidate(_) | ObjectCandidate(_) => unreachable!(),
1929+
}) {
1930+
return Some(ImplCandidate(def_id));
1931+
} else {
1932+
return None;
20521933
}
1934+
}
20531935

2054-
(AutoImplCandidate, ImplCandidate(_)) | (ImplCandidate(_), AutoImplCandidate) => {
2055-
DropVictim::No
2056-
}
1936+
// Also try ignoring all global where-bounds and check whether we end
1937+
// with a unique candidate in this case.
1938+
let mut not_a_global_where_bound = candidates
1939+
.into_iter()
1940+
.filter(|c| !matches!(c.candidate, ParamCandidate(p) if is_global(p)));
1941+
not_a_global_where_bound
1942+
.next()
1943+
.map(|c| c.candidate)
1944+
.filter(|_| not_a_global_where_bound.next().is_none())
1945+
}
20571946

2058-
(AutoImplCandidate, _) | (_, AutoImplCandidate) => {
2059-
bug!(
2060-
"default implementations shouldn't be recorded \
2061-
when there are other global candidates: {:?} {:?}",
2062-
other,
2063-
victim
2064-
);
1947+
fn prefer_lhs_over_victim(
1948+
&self,
1949+
has_non_region_infer: bool,
1950+
(lhs, lhs_evaluation): (DefId, EvaluationResult),
1951+
(victim, victim_evaluation): (DefId, EvaluationResult),
1952+
) -> bool {
1953+
let tcx = self.tcx();
1954+
// See if we can toss out `victim` based on specialization.
1955+
//
1956+
// While this requires us to know *for sure* that the `lhs` impl applies
1957+
// we still use modulo regions here. This is fine as specialization currently
1958+
// assumes that specializing impls have to be always applicable, meaning that
1959+
// the only allowed region constraints may be constraints also present on the default impl.
1960+
if lhs_evaluation.must_apply_modulo_regions() {
1961+
if tcx.specializes((lhs, victim)) {
1962+
return true;
20651963
}
1964+
}
20661965

2067-
// Everything else is ambiguous
2068-
(
2069-
ImplCandidate(_)
2070-
| ClosureCandidate { .. }
2071-
| AsyncClosureCandidate
2072-
| AsyncFnKindHelperCandidate
2073-
| CoroutineCandidate
2074-
| FutureCandidate
2075-
| IteratorCandidate
2076-
| AsyncIteratorCandidate
2077-
| FnPointerCandidate { .. }
2078-
| BuiltinObjectCandidate
2079-
| BuiltinUnsizeCandidate
2080-
| TraitUpcastingUnsizeCandidate(_)
2081-
| BuiltinCandidate { has_nested: true }
2082-
| TraitAliasCandidate,
2083-
ImplCandidate(_)
2084-
| ClosureCandidate { .. }
2085-
| AsyncClosureCandidate
2086-
| AsyncFnKindHelperCandidate
2087-
| CoroutineCandidate
2088-
| FutureCandidate
2089-
| IteratorCandidate
2090-
| AsyncIteratorCandidate
2091-
| FnPointerCandidate { .. }
2092-
| BuiltinObjectCandidate
2093-
| BuiltinUnsizeCandidate
2094-
| TraitUpcastingUnsizeCandidate(_)
2095-
| BuiltinCandidate { has_nested: true }
2096-
| TraitAliasCandidate,
2097-
) => DropVictim::No,
1966+
match tcx.impls_are_allowed_to_overlap(lhs, victim) {
1967+
// For #33140 the impl headers must be exactly equal, the trait must not have
1968+
// any associated items and there are no where-clauses.
1969+
//
1970+
// We can just arbitrarily drop one of the impls.
1971+
Some(ty::ImplOverlapKind::FutureCompatOrderDepTraitObjects) => {
1972+
assert_eq!(lhs_evaluation, victim_evaluation);
1973+
true
1974+
}
1975+
// For candidates which already reference errors it doesn't really
1976+
// matter what we do 🤷
1977+
Some(ty::ImplOverlapKind::Permitted { marker: false }) => {
1978+
lhs_evaluation.must_apply_considering_regions()
1979+
}
1980+
Some(ty::ImplOverlapKind::Permitted { marker: true }) => {
1981+
// Subtle: If the predicate we are evaluating has inference
1982+
// variables, do *not* allow discarding candidates due to
1983+
// marker trait impls.
1984+
//
1985+
// Without this restriction, we could end up accidentally
1986+
// constraining inference variables based on an arbitrarily
1987+
// chosen trait impl.
1988+
//
1989+
// Imagine we have the following code:
1990+
//
1991+
// ```rust
1992+
// #[marker] trait MyTrait {}
1993+
// impl MyTrait for u8 {}
1994+
// impl MyTrait for bool {}
1995+
// ```
1996+
//
1997+
// And we are evaluating the predicate `<_#0t as MyTrait>`.
1998+
//
1999+
// During selection, we will end up with one candidate for each
2000+
// impl of `MyTrait`. If we were to discard one impl in favor
2001+
// of the other, we would be left with one candidate, causing
2002+
// us to "successfully" select the predicate, unifying
2003+
// _#0t with (for example) `u8`.
2004+
//
2005+
// However, we have no reason to believe that this unification
2006+
// is correct - we've essentially just picked an arbitrary
2007+
// *possibility* for _#0t, and required that this be the *only*
2008+
// possibility.
2009+
//
2010+
// Eventually, we will either:
2011+
// 1) Unify all inference variables in the predicate through
2012+
// some other means (e.g. type-checking of a function). We will
2013+
// then be in a position to drop marker trait candidates
2014+
// without constraining inference variables (since there are
2015+
// none left to constrain)
2016+
// 2) Be left with some unconstrained inference variables. We
2017+
// will then correctly report an inference error, since the
2018+
// existence of multiple marker trait impls tells us nothing
2019+
// about which one should actually apply.
2020+
!has_non_region_infer && lhs_evaluation.must_apply_considering_regions()
2021+
}
2022+
None => false,
20982023
}
20992024
}
21002025
}

0 commit comments

Comments
 (0)
This repository has been archived.