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.

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

Method resolution for trait object types does not prefer inherent methods #51402

Open
withoutboats opened this issue Jun 6, 2018 · 8 comments
Labels
A-trait-system Area: Trait system C-bug Category: This is a bug. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@withoutboats
Copy link
Contributor

withoutboats commented Jun 6, 2018

Normally, inherent methods are given precedence over all trait methods, so that there is no ambiguity.

However, this behavior does not appear to be implemented for trait object types:

trait Foo {
    fn f(&self) { }
}

impl Foo {
    fn f(&self) { }
}

impl Foo for i32 { }

struct Bar;

impl Bar {
    fn f(&self) { }
}

impl Foo for Bar { }

fn main() {
    let x: &Foo = &42i32;
    x.f();
    let bar: &Bar = &Bar;
    bar.f();
}

https://play.rust-lang.org/?gist=7d4534a5d973249c66244b9affa03199&version=stable&mode=debug

What's worse, UFCS does not allow for specifying that you want the inherent method, because Foo::f(x) in this code will remain ambiguous. In general, UFCS isn't designed to support disambiguating to inherent methods because inherent methods are supposed to always take precedence.

@matthewjasper
Copy link
Contributor

cc #45251

@withoutboats withoutboats added I-nominated T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Jun 6, 2018
@withoutboats
Copy link
Contributor Author

Tagging and nominating to discuss whether this behavior is a bug or intended.

@sgrif
Copy link
Contributor

sgrif commented Jun 6, 2018

Worth noting in any discussion around this that since inherent methods can only be defined in the same crate as the type, this would only affect any crate which intentionally tried to provide an inherent method for a trait object with the same name as a method on the trait itself.

@withoutboats
Copy link
Contributor Author

Niko explained to me that this issue is actually inverted from what I thought: all trait methods are considered inherent methods of the trait object type. I've removed the I-nominated tag.

However, in my use case there's this interesting problem, which can be demonstrated roughly like this:

trait Foo {
    fn dynamic(&self) -> &dyn Foo where Self: SIzed { self as &dyn Foo }
}

impl dyn Foo {
    fn dynamic(&self) -> &dyn Foo { self }
}

The conversion requires where Self: Sized on the method.

There's no way to provide a method with a single name for all Sized impls and for the trait object type today.

@Havvy
Copy link
Contributor

Havvy commented Jun 9, 2018

It'd be backwards compatible to remove the whole trait methods are psuedo-inherent methods on trait objects thing. And it seems to be causing more confusion than it seems to save. It's also one extra complexity in the language that's already complex enough.

A more conservative option is to only have inherent methods for the methods that are object safe. So in the case of the where Self: Sized fns, the trait object wouldn't have them as inherent methods.

@Diggsey
Copy link
Contributor

Diggsey commented Oct 7, 2018

@stepancheg
Copy link
Contributor

@Diggsey

actually, it is possible if you use an additional trait

But the user has to explicitly import that trait, which is worse for discoverability and for typing in less advanced IDEs.

@Rua
Copy link
Contributor

Rua commented Feb 27, 2022

I just ran into this issue, pretty much in the situation described by @withoutboats. In my case, the return type contains Self but it's the same idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-trait-system Area: Trait system C-bug Category: This is a bug. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants