Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

elaborate obligations in coherence #124532

Closed
wants to merge 3 commits into from
Closed

elaborate obligations in coherence #124532

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4fde241
elaborate obligations in coherence
lcnr Apr 29, 2024
c565a0a
remove unreachable code
lcnr May 1, 2024
03d9e84
refactor winnowing
lcnr May 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion compiler/rustc_middle/src/traits/select.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,18 @@ pub enum SelectionCandidate<'tcx> {
/// Implementation of transmutability trait.
TransmutabilityCandidate,

ParamCandidate(ty::PolyTraitPredicate<'tcx>),
/// A candidate from the `ParamEnv`.
ParamCandidate {
/// The actual `where`-bound, e.g. `T: Trait`.
predicate: ty::PolyTraitPredicate<'tcx>,
/// `true` if the where-bound has no bound vars and does
/// not refer to any parameters or inference variables.
///
/// We prefer all other candidates over global where-bounds.
/// Notably, global where-bounds do not shadow impls.
is_global: bool,
},

ImplCandidate(DefId),
AutoImplCandidate,

Expand Down
7 changes: 5 additions & 2 deletions compiler/rustc_trait_selection/src/traits/coherence.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,9 +358,12 @@ fn impl_intersection_has_impossible_obligation<'a, 'cx, 'tcx>(
) -> IntersectionHasImpossibleObligations<'tcx> {
let infcx = selcx.infcx;

// Elaborate obligations in case the current obligation is unknowable,
// but its super trait bound is not. See #124532 for more details.
let obligations = util::elaborate(infcx.tcx, obligations.iter().cloned());
if infcx.next_trait_solver() {
let ocx = ObligationCtxt::new(infcx);
ocx.register_obligations(obligations.iter().cloned());
ocx.register_obligations(obligations);
let errors_and_ambiguities = ocx.select_all_or_error();
// We only care about the obligations that are *definitely* true errors.
// Ambiguities do not prove the disjointness of two impls.
Expand All @@ -387,7 +390,7 @@ fn impl_intersection_has_impossible_obligation<'a, 'cx, 'tcx>(
for obligation in obligations {
// We use `evaluate_root_obligation` to correctly track intercrate
// ambiguity clauses.
let evaluation_result = selcx.evaluate_root_obligation(obligation);
let evaluation_result = selcx.evaluate_root_obligation(&obligation);

match evaluation_result {
Ok(result) => {
Expand Down
10 changes: 6 additions & 4 deletions compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,16 +251,18 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
all_bounds.filter(|p| p.def_id() == stack.obligation.predicate.def_id());

// Keep only those bounds which may apply, and propagate overflow if it occurs.
for bound in matching_bounds {
if bound.skip_binder().polarity != stack.obligation.predicate.skip_binder().polarity {
for predicate in matching_bounds {
if predicate.skip_binder().polarity != stack.obligation.predicate.skip_binder().polarity
{
continue;
}

// FIXME(oli-obk): it is suspicious that we are dropping the constness and
// polarity here.
let wc = self.where_clause_may_apply(stack, bound.map_bound(|t| t.trait_ref))?;
let wc = self.where_clause_may_apply(stack, predicate.map_bound(|t| t.trait_ref))?;
if wc.may_apply() {
candidates.vec.push(ParamCandidate(bound));
let is_global = predicate.is_global() && !predicate.has_bound_vars();
candidates.vec.push(ParamCandidate { predicate, is_global });
}
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_trait_selection/src/traits/select/confirmation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
ImplSource::Builtin(BuiltinImplSource::Misc, data)
}

ParamCandidate(param) => {
ParamCandidate { predicate, is_global: _ } => {
let obligations =
self.confirm_param_candidate(obligation, param.map_bound(|t| t.trait_ref));
self.confirm_param_candidate(obligation, predicate.map_bound(|t| t.trait_ref));
ImplSource::Param(obligations)
}

Expand Down
192 changes: 44 additions & 148 deletions compiler/rustc_trait_selection/src/traits/select/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1575,7 +1575,9 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
return false;
}
match result {
Ok(Some(SelectionCandidate::ParamCandidate(trait_ref))) => !trait_ref.has_infer(),
Ok(Some(SelectionCandidate::ParamCandidate { predicate, .. })) => {
!predicate.has_infer()
}
_ => true,
}
}
Expand Down Expand Up @@ -1827,31 +1829,35 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
return DropVictim::Yes;
}

// 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();

// (*) Prefer `BuiltinCandidate { has_nested: false }`, `PointeeCandidate`,
// `DiscriminantKindCandidate`, `ConstDestructCandidate`
// 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,

// (*)
// Prefer `BuiltinCandidate { has_nested: false }`, `ConstDestructCandidate`
// to anything else.
//
// This is a fix for #53123 and prevents winnowing from accidentally extending the
// lifetime of a variable.
(
BuiltinCandidate { has_nested: false } | ConstDestructCandidate(_),
BuiltinCandidate { has_nested: false } | ConstDestructCandidate(_),
) => bug!("two trivial builtin candidates: {other:?} {victim:?}"),
(BuiltinCandidate { has_nested: false } | ConstDestructCandidate(_), _) => {
DropVictim::Yes
}
(_, BuiltinCandidate { has_nested: false } | ConstDestructCandidate(_)) => {
DropVictim::No
}

(ParamCandidate(other), ParamCandidate(victim)) => {
// Global bounds from the where clause should be ignored
// here (see issue #50825).
(ParamCandidate { is_global: true, .. }, ParamCandidate { is_global: true, .. }) => {
DropVictim::No
}
(_, ParamCandidate { is_global: true, .. }) => DropVictim::Yes,
(ParamCandidate { is_global: true, .. }, _) => DropVictim::No,

(
ParamCandidate { is_global: false, predicate: other },
ParamCandidate { is_global: false, predicate: victim },
) => {
let same_except_bound_vars = other.skip_binder().trait_ref
== victim.skip_binder().trait_ref
&& other.skip_binder().polarity == victim.skip_binder().polarity
Expand All @@ -1868,68 +1874,8 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
}
}

// Drop otherwise equivalent non-const fn pointer candidates
(FnPointerCandidate { .. }, FnPointerCandidate { fn_host_effect }) => {
DropVictim::drop_if(*fn_host_effect == self.tcx().consts.true_)
}

(
ParamCandidate(ref 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(ref victim_cand)) => {
// Prefer these to a global where-clause bound
// (see issue #50825).
if is_global(victim_cand) { DropVictim::Yes } else { DropVictim::No }
}
(
ImplCandidate(_)
| AutoImplCandidate
| ClosureCandidate { .. }
| AsyncClosureCandidate
| AsyncFnKindHelperCandidate
| CoroutineCandidate
| FutureCandidate
| IteratorCandidate
| AsyncIteratorCandidate
| FnPointerCandidate { .. }
| BuiltinObjectCandidate
| BuiltinUnsizeCandidate
| TraitUpcastingUnsizeCandidate(_)
| BuiltinCandidate { has_nested: true }
| TraitAliasCandidate,
ParamCandidate(ref 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(),
)
}
(ParamCandidate { is_global: false, .. }, _) => DropVictim::Yes,
(_, ParamCandidate { is_global: false, .. }) => DropVictim::No,

(ProjectionCandidate(i), ProjectionCandidate(j))
| (ObjectCandidate(i), ObjectCandidate(j)) => {
Expand All @@ -1942,44 +1888,18 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
bug!("Have both object and projection candidate")
}

// Arbitrarily give projection and object candidates priority.
(
ObjectCandidate(_) | ProjectionCandidate(_),
ImplCandidate(..)
| AutoImplCandidate
| ClosureCandidate { .. }
| AsyncClosureCandidate
| AsyncFnKindHelperCandidate
| CoroutineCandidate
| FutureCandidate
| IteratorCandidate
| AsyncIteratorCandidate
| FnPointerCandidate { .. }
| BuiltinObjectCandidate
| BuiltinUnsizeCandidate
| TraitUpcastingUnsizeCandidate(_)
| BuiltinCandidate { .. }
| TraitAliasCandidate,
) => DropVictim::Yes,
// Arbitrarily give projection candidates priority.
(ProjectionCandidate(_), _) => DropVictim::Yes,
(_, ProjectionCandidate(_)) => DropVictim::No,

(
ImplCandidate(..)
| AutoImplCandidate
| ClosureCandidate { .. }
| AsyncClosureCandidate
| AsyncFnKindHelperCandidate
| CoroutineCandidate
| FutureCandidate
| IteratorCandidate
| AsyncIteratorCandidate
| FnPointerCandidate { .. }
| BuiltinObjectCandidate
| BuiltinUnsizeCandidate
| TraitUpcastingUnsizeCandidate(_)
| BuiltinCandidate { .. }
| TraitAliasCandidate,
ObjectCandidate(_) | ProjectionCandidate(_),
) => DropVictim::No,
// 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 { .. }`.
//
// cc #57893
(ObjectCandidate(_), _) => DropVictim::Yes,
(_, ObjectCandidate(_)) => DropVictim::No,

(&ImplCandidate(other_def), &ImplCandidate(victim_def)) => {
// See if we can toss out `victim` based on specialization.
Expand Down Expand Up @@ -2059,49 +1979,25 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
}
}

(AutoImplCandidate, ImplCandidate(_)) | (ImplCandidate(_), AutoImplCandidate) => {
DropVictim::No
}

(AutoImplCandidate, _) | (_, AutoImplCandidate) => {
bug!(
"default implementations shouldn't be recorded \
when there are other global candidates: {:?} {:?}",
other,
victim
);
}

// Everything else is ambiguous
// Treat all non-trivial builtin impls and user-defined impls the same way.
(
ImplCandidate(_)
| ClosureCandidate { .. }
| AsyncClosureCandidate
| AsyncFnKindHelperCandidate
| CoroutineCandidate
| FutureCandidate
| IteratorCandidate
| AsyncIteratorCandidate
| FnPointerCandidate { .. }
| BuiltinObjectCandidate
| BuiltinUnsizeCandidate
| TraitUpcastingUnsizeCandidate(_)
| AutoImplCandidate
| BuiltinCandidate { has_nested: true }
| TraitAliasCandidate,
ImplCandidate(_)
| ClosureCandidate { .. }
| AsyncClosureCandidate
| AsyncFnKindHelperCandidate
| CoroutineCandidate
| FutureCandidate
| IteratorCandidate
| AsyncIteratorCandidate
| FnPointerCandidate { .. }
| BuiltinObjectCandidate
| ClosureCandidate { .. }
| TraitAliasCandidate
| BuiltinUnsizeCandidate
| TraitUpcastingUnsizeCandidate(_)
| BuiltinCandidate { has_nested: true }
| TraitAliasCandidate,
| TransmutabilityCandidate
| BuiltinObjectCandidate,
_,
) => DropVictim::No,
}
}
Expand Down
1 change: 0 additions & 1 deletion src/tools/tidy/src/issues.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4061,7 +4061,6 @@ ui/traits/issue-6128.rs
ui/traits/issue-6334.rs
ui/traits/issue-65284-suggest-generic-trait-bound.rs
ui/traits/issue-65673.rs
ui/traits/issue-66768.rs
ui/traits/issue-68295.rs
ui/traits/issue-7013.rs
ui/traits/issue-70944.rs
Expand Down
5 changes: 3 additions & 2 deletions tests/ui/associated-item/issue-105449.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
//@ check-pass
//@ compile-flags: -C debug_assertions=yes -Zunstable-options

#[allow(dead_code)]
// This is a mutated variant of #66768 which has been removed
// as it no longer tests the original issue.
fn problematic_function<Space>()
where
DefaultAlloc: FinAllok<R1, Space>,
{
let e = Edge2dElement;
let _ = Into::<Point>::into(e.map_reference_coords());
//~^ ERROR the trait bound `Point: From<(Ure, R1, MStorage)>` is not satisfied
}
impl<N> Allocator<N, R0> for DefaultAlloc {
type Buffer = MStorage;
Expand Down
15 changes: 15 additions & 0 deletions tests/ui/associated-item/issue-105449.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
error[E0277]: the trait bound `Point: From<(Ure, R1, MStorage)>` is not satisfied
--> $DIR/issue-105449.rs:10:33
|
LL | let _ = Into::<Point>::into(e.map_reference_coords());
| ------------------- ^^^^^^^^^^^^^^^^^^^^^^^^ the trait `From<(Ure, R1, MStorage)>` is not implemented for `Point`, which is required by `(Ure, R1, MStorage): Into<Point>`
| |
| required by a bound introduced by this call
|
= help: the trait `From<(Ure, Space, <DefaultAlloc as Allocator<Ure, Space>>::Buffer)>` is implemented for `Point`
= help: for that trait implementation, expected `Space`, found `R1`
= note: required for `(Ure, R1, MStorage)` to implement `Into<Point>`

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0277`.
2 changes: 1 addition & 1 deletion tests/ui/coherence/coherence-overlap-unnormalizable-projection-0.classic.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ LL | | for<'a> <T as WithAssoc<'a>>::Assoc: WhereBound,
LL | impl<T> Trait for Box<T> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `Box<_>`
|
= note: downstream crates may implement trait `WithAssoc<'a>` for type `std::boxed::Box<_>`
= note: downstream crates may implement trait `WhereBound` for type `<std::boxed::Box<_> as WithAssoc<'a>>::Assoc`
= note: downstream crates may implement trait `WithAssoc<'a>` for type `std::boxed::Box<_>`

error: aborting due to 1 previous error

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/coherence/coherence-overlap-unnormalizable-projection-1.classic.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ LL | | for<'a> Box<<T as WithAssoc<'a>>::Assoc>: WhereBound,
LL | impl<T> Trait for Box<T> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `Box<_>`
|
= note: downstream crates may implement trait `WithAssoc<'a>` for type `std::boxed::Box<_>`
= note: downstream crates may implement trait `WhereBound` for type `std::boxed::Box<<std::boxed::Box<_> as WithAssoc<'a>>::Assoc>`
= note: downstream crates may implement trait `WithAssoc<'a>` for type `std::boxed::Box<_>`

error: aborting due to 1 previous error

Expand Down
1 change: 1 addition & 0 deletions tests/ui/coherence/normalize-for-errors.current.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ LL | impl<S: Iterator> MyTrait<S> for (Box<<(MyType,) as Mirror>::Assoc>, S::Ite
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `(Box<(MyType,)>, _)`
|
= note: upstream crates may add a new impl of trait `std::marker::Copy` for type `std::boxed::Box<(MyType,)>` in future versions
= note: upstream crates may add a new impl of trait `std::clone::Clone` for type `(MyType,)` in future versions

error: aborting due to 1 previous error

Expand Down
Loading
Loading