Skip to content

fix: not complete same name inherent deref methods#22376

Merged
ChayimFriedman2 merged 2 commits into
rust-lang:masterfrom
A4-Tacks:method-hide-same-name
May 17, 2026
Merged

fix: not complete same name inherent deref methods#22376
ChayimFriedman2 merged 2 commits into
rust-lang:masterfrom
A4-Tacks:method-hide-same-name

Conversation

@A4-Tacks
Copy link
Copy Markdown
Member

@A4-Tacks A4-Tacks commented May 16, 2026

Fixes #20773

Motivation: #20773 (comment)

I'm not sure if Object and WhereClause were handled correctly

Example

fn test(a: A) {
    a.$0
}
impl core::ops::Deref for A {
    type Target = B;
    fn deref(&self) -> &Self::Target { loop {} }
}
trait Foo { fn foo(&self) -> u32 {} }
impl Foo for A {}
impl Foo for B {}
impl A { fn foo(&self) -> u8 {} }
impl B { fn foo(&self) -> u16 {} }
struct A {}
struct B {}

Before this PR

me foo()             fn(&self) -> u8
me foo()            fn(&self) -> u16
me foo() (as Foo)   fn(&self) -> u32

After this PR

me foo()             fn(&self) -> u8
me foo() (as Foo)   fn(&self) -> u32

Example
---
```rust
fn test(a: A) {
    a.$0
}
impl core::ops::Deref for A {
    type Target = B;
    fn deref(&self) -> &Self::Target { loop {} }
}
trait Foo { fn foo(&self) -> u32 {} }
impl Foo for A {}
impl Foo for B {}
impl A { fn foo(&self) -> u8 {} }
impl B { fn foo(&self) -> u16 {} }
struct A {}
struct B {}
```

**Before this PR**

```text
me foo()             fn(&self) -> u8
me foo()            fn(&self) -> u16
me foo() (as Foo)   fn(&self) -> u32
```

**After this PR**

```text
me foo()             fn(&self) -> u8
me foo() (as Foo)   fn(&self) -> u32
```
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 16, 2026
@A4-Tacks A4-Tacks requested a review from ChayimFriedman2 May 16, 2026 04:05
Copy link
Copy Markdown
Contributor

@ChayimFriedman2 ChayimFriedman2 left a comment

Choose a reason for hiding this comment

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

One request. Also it'll be nice to add a test for the invisible method case.

View changes since this review

}
}
};
same_name.insert_entry(func);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should move that inside the if, otherwise invisible methods can override visible methods.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thank you for catch this, but fixing it is not that simple, I am currently working on it

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why inserting it under the if can't solve this?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Because on_inherent_method is not called ordered

I think we need to block until we fix the order based on another PR

For the following code, I printed the log and output FEACDB:

pub struct A {}
pub struct B {}
pub struct C {}
pub struct D {}
pub struct E {}
pub struct F {}
impl core::ops::Deref for A {
    type Target = B;
    fn deref(&self) -> &Self::Target { loop {} }
}
impl core::ops::Deref for B {
    type Target = C;
    fn deref(&self) -> &Self::Target { loop {} }
}
impl core::ops::Deref for C {
    type Target = D;
    fn deref(&self) -> &Self::Target { loop {} }
}
impl core::ops::Deref for D {
    type Target = E;
    fn deref(&self) -> &Self::Target { loop {} }
}
impl core::ops::Deref for E {
    type Target = F;
    fn deref(&self) -> &Self::Target { loop {} }
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmm, weird.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Changing the FxHashMap to FxIndexMap here:

candidates: RefCell<FxHashMap<CandidateId, CandidateWithPrivate<'db>>>,

Should fix this.

@A4-Tacks
Copy link
Copy Markdown
Member Author

Also it'll be nice to add a test for the invisible method case.

This test is exists, for non pub methods in other crates

@A4-Tacks A4-Tacks marked this pull request as draft May 17, 2026 05:03
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 17, 2026
@A4-Tacks A4-Tacks marked this pull request as ready for review May 17, 2026 05:48
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 17, 2026
Co-authored-by: Chayim Refael Friedman <chayimfr@gmail.com>
@A4-Tacks A4-Tacks force-pushed the method-hide-same-name branch from b430eba to be99012 Compare May 17, 2026 06:03
@ChayimFriedman2 ChayimFriedman2 enabled auto-merge May 17, 2026 06:06
@ChayimFriedman2 ChayimFriedman2 added this pull request to the merge queue May 17, 2026
Merged via the queue into rust-lang:master with commit 4c52c35 May 17, 2026
18 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 17, 2026
@A4-Tacks A4-Tacks deleted the method-hide-same-name branch May 17, 2026 06:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Duplicate deref-chain method names

3 participants