fix: correct the args for disambiguate the associated function diagnostic

[Young-Flash]

This is somehow silimar to #118502, we shouldn't take receiver as first arg all the cases.

close #118819

[rustbot]

r? @compiler-errors

(rustbot has picked a reviewer for you, use r? to override)

[compiler-errors]

r? fmease

[Young-Flash]

EDIT: got it, upstream modify some internal api while I hadn't sync with that

can't figure out why CI failed with the following, but it compile on my local. any hints? @fmease @compiler-errors

error[E0599]: no method named `find_by_def_id` found for struct `rustc_middle::hir::map::Map` in the current scope
    --> compiler/rustc_hir_typeck/src/method/suggest.rs:3356:61
     |
3356 |                         Some(local_def_id) => match hir_map.find_by_def_id(local_def_id) {
     |                                                             ^^^^^^^^^^^^^^ method not found in `Map<'_>`

[fmease]

Sorry for taking so long! I have some style nits and some suggestions.

This diagnostic generally has quite a few false positives and negatives before and after your changes, so I guess it's not worth making it perfect in this PR. I have some suggestions like trying out can_eq. If you can't make it work, then that's fine, too.

[fmease]

While this check works for a lot of cases, it doesn't work for more complex ones. E.g., here

struct S;

trait TraitA { fn f(this: Box<Self>); }
trait TraitB { fn f(this: Box<Self>); }

impl TraitA for S { fn f(this: Box<Self>) {} }
impl TraitB for S { fn f(this: Box<Self>) {} }

fn main() {
    Box::new(S).f(); // trait_impl_type `Box<S>` != first_arg_type `Box<Self>`
}

However, I don't think you need to address this in this PR since it's super broken anyway at the moment, it suggests <Box<S> as TraitA>::f(); & <Box<S> as TraitB>::f() while it should suggest TraitA::f(Box::new(S))/<S as TraitA>::f(Box::new(S)) & TraitB::f(Box::new(S))/<S as TraitB>::f(Box::new(S)).

I think this could be solved by using FnCtxt::can_eq instead of ==.

[fmease]

Further, with plain == we will never consider type parameters to be valid receiver types while they very well could be:

struct S;

trait TraitA { fn f(self); }
trait TraitB { fn f(self); }

impl<T> TraitA for T { fn f(self) {} }
impl<T> TraitB for T { fn f(self) {} }

fn main() { S.f(); }

You could think about instantiate'ing the fn_sig with fresh_args_for_item instead of calling skip_binder and using FnCtxt::can_eq with the ParamEnv of the assoc fn. Well, it can still have false positives due to autoref/autoderef. This way, we would rely less on the check arbitrary_self_types_check(…) / item.fn_has_self_parameter.

[fmease]

This makes sense but it's prone to errors:

trait A { fn f(self); }
trait B { fn f(self); }

#[derive(Clone, Copy)]
struct S;

impl A for S { fn f(self) {} }
impl B for S { fn f(self) {} }

fn main() {
    (&&&S).f(); // Here, we suggest `A::f((&&&S))` but we should suggest `A::f(***S)`
}

Doesn't need to be fixed in this PR just making note of it.

[fmease]

The amount of references is not kept in sync between first_arg_type and rcvr_ref. E.g.,

trait A { fn f(self: &mut &Self); }
trait B { fn f(self: &mut &Self); }

struct S;

impl A for S { fn f(self: &mut &Self) {} }
impl B for S { fn f(self: &mut &Self) {} }

fn main() {
    (&mut &S).f(); // We suggest `A::f(&mut (&mut &S))` but it should be `A::f(&mut &S);`
}

[Young-Flash]

sorry for the slow response and thank you for taking the time to review this. I make some modification and leave the rest for future work (so this PR could be focused), LMK if there are somethings can improve

[fmease]

Thanks, for implementing my feedback, it looks good! Further improvements can potentially be made in follow-up PRs.

[fmease]

@bors r+ rollup

[bors]

📌 Commit 9e9353c has been approved by fmease

It is now in the queue for this repository.

[bors]

⌛ Testing commit 9e9353c with merge 3cdd004...

[bors]

☀️ Test successful - checks-actions
Approved by: fmease
Pushing 3cdd004 to master...

[rust-timer]

Finished benchmarking commit (3cdd004): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	3.0%	[2.8%, 3.3%]	2
Regressions ❌ (secondary)	2.3%	[2.3%, 2.3%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.5%	[-3.9%, -3.2%]	2
All ❌✅ (primary)	3.0%	[2.8%, 3.3%]	2

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 669.249s -> 671.886s (0.39%)
Artifact size: 312.02 MiB -> 312.01 MiB (-0.00%)