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

methods in trait impls should override default methods #18446

Closed
nrc opened this issue Oct 30, 2014 · 5 comments
Closed

methods in trait impls should override default methods #18446

nrc opened this issue Oct 30, 2014 · 5 comments
Labels
A-type-system Area: Type system E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added.

Comments

@nrc
Copy link
Member

nrc commented Oct 30, 2014

E.g.,

trait T {
    fn foo(&self) {}
}

impl<'a> T+'a {
    fn foo(&self) {}
}

Should be OK. (It currently gives "duplicate definition of value foo").

Follow up to #17750

@nrc
Copy link
Member Author

nrc commented Oct 30, 2014

To explain what needs doing and why I didn't just do it as part of #17750:

Resolve works by creating modules for namespaces. In the above example there will a top level module, then should create separate modules for the trait T and the impl T. However, resolve uses an ast::Name to lookup these modules. So, when it gets to the impl, it tries to find a module for T and finds the trait, where the method foo is already defined.

The fix, I suppose, is to use something other than just an ast::Name as the key for looking up namespace modules (perhaps a NodeId or a key object combining a kind of item and a Name). But this is a relatively major change to resolve, so I think it is worth separating out from the implementation work for #17750.

@nrc
Copy link
Member Author

nrc commented Oct 30, 2014

When we do this, we must check (and add a test for) coherence still working with trait impls. E.g.,

trait T {
    fn foo(&self);
}

impl<'a> T+'a {
    fn foo(&self) {}
}

impl T for int {
    fn foo(&self) {}
}

fn main() {
    let x: &T = &42i;
    x.foo(); //~ERROR
}

Here, x.foo should cause a coherence error since both impls are in scope.

This is not possible right now because there will a name clash between the required method in the trait and the provided method in the trait impl.

@nikomatsakis
Copy link
Contributor

Interesting. Per the UFCS RFC, T::foo should resolve to the trait, and <T>::foo should resolve to the inherent fn

@steveklabnik steveklabnik added the A-type-system Area: Type system label Jan 27, 2015
@frewsxcv
Copy link
Member

This compiles now. Someone apply the 'needs-test' label

@huonw huonw added the E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. label Nov 18, 2015
@frewsxcv
Copy link
Member

Fixed by #40296

lnicola pushed a commit to lnicola/rust that referenced this issue Nov 4, 2024
fix: Only parse `safe` as contextual kw in extern blocks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-type-system Area: Type system E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added.
Projects
None yet
Development

No branches or pull requests

5 participants