Skip to content

Make sure to treat only param where clauses as inherent #145262

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.

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 23 additions & 0 deletions compiler/rustc_hir_typeck/src/method/probe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1945,6 +1945,29 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
);
(xform_self_ty, xform_ret_ty) =
self.xform_self_ty(probe.item, trait_ref.self_ty(), trait_ref.args);

if matches!(probe.kind, WhereClauseCandidate(_)) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could fast path this for the old trait solver, since we expect things to be normalized always.

Copy link
Contributor

@lcnr lcnr Aug 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm 🤔

so what's going on is:

  • if the normalized self type is a param we assemble_inherent_candidates_from_param
  • this uses fast-reject in an attempt to filter to where-clauses which only apply for the self type
    • doesn't handle unnormalized where-clauses or alias self types

We can't normalize in assembly because it doesn't use a probe, so we instead do it here. This totally makes sense to me, r=me on this impl

Can you add a comment to assemble_inherent_candidates_from_param that we only filter the bounds as an fast-path optimization and we properly reject non-param self type where-clauses in consider_probe?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually, move this structurally_normalize_ty above the xform and then use the normalized ty in xform_self_ty? This should avoid some work given that this change seems to actually impact perf in the new solver

// `WhereClauseCandidate` requires that the self type is a param,
// because it has special behavior with candidate preference as an
// inherent pick.
match ocx.structurally_normalize_ty(
cause,
self.param_env,
trait_ref.self_ty(),
) {
Ok(ty) => {
if !matches!(ty.kind(), ty::Param(_)) {
debug!("--> not a param ty: {xform_self_ty:?}");
return ProbeResult::NoMatch;
}
}
Err(errors) => {
debug!("--> cannot relate self-types {:?}", errors);
return ProbeResult::NoMatch;
}
}
}

xform_self_ty = ocx.normalize(cause, self.param_env, xform_self_ty);
match ocx.relate(cause, self.param_env, self.variance(), self_ty, xform_self_ty)
{
Expand Down
42 changes: 42 additions & 0 deletions tests/ui/methods/rigid-alias-bound-is-not-inherent.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// See the code below.
//
// We were using `DeepRejectCtxt` to ensure that `assemble_inherent_candidates_from_param`
// did not rely on the param-env being eagerly normalized. Since aliases unify with all
// types, this meant that a rigid param-env candidate like `<T as Deref>::Target: Trait1`
// would be registered as a "WhereClauseCandidate", which is treated as inherent. Since
// we evaluate these candidates for all self types in the deref chain, this candidate
// would be satisfied for `<T as Deref>::Target`, meaning that it would be preferred over
// an "extension" candidate like `<T as Deref>::Target: Trait2` even though it holds.
// This is problematic, since it causes ambiguities to be broken somewhat arbitrarily.
// And as a side-effect, it also caused our computation of "used" traits to be miscalculated
// since inherent candidates don't count as an import usage.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also test with new solver?


use std::ops::Deref;

trait Trait1 {
fn method(&self) {
println!("1");
}
}

trait Trait2 {
fn method(&self) {
println!("2");
}
}
impl<T: Other + ?Sized> Trait2 for T {}

trait Other {}

fn foo<T>(x: T)
where
T: Deref,
<T as Deref>::Target: Trait1 + Other,
{
// Make sure that we don't prefer methods from where clauses for rigid aliases,
// just for params. We could revisit this behavior, but it would be a lang change.
x.method();
//~^ ERROR multiple applicable items in scope
}

fn main() {}
30 changes: 30 additions & 0 deletions tests/ui/methods/rigid-alias-bound-is-not-inherent.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
error[E0034]: multiple applicable items in scope
--> $DIR/rigid-alias-bound-is-not-inherent.rs:38:7
|
LL | x.method();
| ^^^^^^ multiple `method` found
|
note: candidate #1 is defined in the trait `Trait1`
--> $DIR/rigid-alias-bound-is-not-inherent.rs:17:5
|
LL | fn method(&self) {
| ^^^^^^^^^^^^^^^^
note: candidate #2 is defined in an impl of the trait `Trait2` for the type `T`
--> $DIR/rigid-alias-bound-is-not-inherent.rs:23:5
|
LL | fn method(&self) {
| ^^^^^^^^^^^^^^^^
help: disambiguate the method for candidate #1
|
LL - x.method();
LL + Trait1::method(&x);
|
help: disambiguate the method for candidate #2
|
LL - x.method();
LL + Trait2::method(&x);
|

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0034`.
Loading