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

Conflicting method names in the same trait hierachy can only ever be called with UFCS syntax #17151

Closed
Kimundi opened this issue Sep 10, 2014 · 7 comments
Labels
A-type-system Area: Type system C-feature-request Category: A feature request, i.e: not implemented / a PR. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@Kimundi
Copy link
Member

Kimundi commented Sep 10, 2014

During the writing of this issue I reached the conclusion that the current semantic is probably the best solution to this problem already, but I'm opening this issue anyway to encourage discussion or at least getting confirmation from official site that this is the intended behavior.


Right now, you can use the same method name in different traits, even if they exist in a inheritance relation, eg this compiles:

mod foo {
    pub trait Foo {
        fn foo(&self) {}
    }

    pub trait Bar: Foo {
        fn foo(&self) {}
    }

    impl Foo for uint{}
    impl Bar for uint{}
    impl Foo for int{}
    impl Bar for int{}
}

This is currently possible to prevent a fragile base class problem: A library maintainer providing a supertrait should be able to add an new method without breaking downstream code.

However, even if you don't explicitly import Foo or Bar, any place where you try to use the foo() method of Bar you get a "conflicting methods in scope" error, because super traits are automatically brought in scope for method resolution:

In generics:

fn gen<T: foo::Bar>(t: T) {
    t.foo();
}
<anon>:18:5: 18:12 error: multiple applicable methods in scope [E0034]
<anon>:18     t.foo();
              ^~~~~~~
<anon>:18:5: 18:12 note: candidate #1 derives from the bound `foo::Bar`
<anon>:18     t.foo();
              ^~~~~~~
<anon>:18:5: 18:12 note: candidate #2 derives from the bound `foo::Foo`
<anon>:18     t.foo();
              ^~~~~~~

In trait objects:

fn main() {
    let y = vec![box 0u as Box<foo::Bar>, box 0i as Box<foo::Bar>];
    for e in y.iter() {
        e.foo();
    }
}
<anon>:24:9: 24:16 error: multiple applicable methods in scope [E0034]
<anon>:24         e.foo();
                  ^~~~~~~
<anon>:24:9: 24:16 note: candidate #1 derives from the type of the receiver, which is the trait `foo::Bar`
<anon>:24         e.foo();
                  ^~~~~~~
<anon>:24:9: 24:16 note: candidate #2 derives from the type of the receiver, which is the trait `foo::Foo`
<anon>:24         e.foo();
                  ^~~~~~~

Which means in practice you'd run into this problem anyway.

Calling associated functions works though, so you could use UFCS to differentiate the methods:

fn gen<T: foo::Bar>(t: T) {
    foo::Foo::foo(&t);
    foo::Bar::foo(&t);
}

However, a conflict like this still forces a code change decision on the provider of the trait and all downstream users:

  • Changing all code calling the method to the UFCS syntax. (forced for downstream users because else they get conflicting methods errors)
  • Renaming the method and all its uses in the sub trait (Can lead to downstream users accidentally calling the new super trait method instead)

Both options are approximately equally big changes for downstream users, and none of them allows conflicting method names to be used with the original dot syntax, so the question is if this is working as intended, or whether it might be improved upon.

At the very least, it might be a good idea to implement a warn-per-default lint for the conflicting method name in the sub trait definition.

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

I believe that the decision we came to here: https://github.com/rust-lang/rfcs/blob/master/text/1105-api-evolution.md#traits

Doesn't explicitly cover this case, but "you need to disambiguate with UFCS" is considered a minor enough fix that it's acceptable.

@gnzlbg
Copy link
Contributor

gnzlbg commented Nov 14, 2016

Doesn't explicitly cover this case, but "you need to disambiguate with UFCS" is considered a minor enough fix that it's acceptable.

I ran into this and when the traits involved have associated types, input/output parameters disambiguation is painful. Anyhow I wish the error messages in E0034 will tell me what do I need to type to disambiguate the calls for each candidate (gonna fill that as a different issue).

@aidanhs
Copy link
Member

aidanhs commented Mar 1, 2017

Possibly related(?) is the following:

use std::ops::Deref;
struct S;
struct T;
static USIZE: usize = 5;
impl Deref for T {
    type Target = usize;
    fn deref(&self) -> &Self::Target { &USIZE }
}
trait Check<T> {
    fn check(&self, _: &T) {}
}
impl Check<usize> for S {}
//impl Check<u64> for S {} // XXX
fn main() {
    S.check(&T);
}

https://is.gd/MmCCwI

This compiles fine, but if you uncomment the line with XXX it will error with "the trait Check<T> is not implemented for S". While this is technically true, it could unambiguously coerce. Is there a rule against this written down somewhere (I can imagine there might be, just to keep things simple)?

@Mark-Simulacrum Mark-Simulacrum added I-papercut T-lang Relevant to the language team, which will review and decide on the PR/issue. C-feature-request Category: A feature request, i.e: not implemented / a PR. and removed I-papercut labels Jul 22, 2017
@Havvy
Copy link
Contributor

Havvy commented Jun 30, 2018

Triage: The previous comment is unrelated to the rest of the issue. Otherwise, the only actionable part of this issue is to add a lint to warn when a trait has a function with the same name as one of its supertraits' functions. Due to the edge-case nature and the errors that'll be seen first, I propose refiling that lint against Clippy and closing this issue.

@Havvy
Copy link
Contributor

Havvy commented Sep 3, 2018

Refiled as rust-lang/rust-clippy#3117

@lcdr
Copy link

lcdr commented Mar 2, 2019

I ran into this as well. The UFCS syntax is unfortunately not as ergonomic as normal method syntax, it'd be nice if there was a way to use method syntax.

Would it be possible to resolve this by having the subtrait's items shadow the supertrait's items, so that a method syntax call would resolve to the subtrait? The supertrait's items could still be referenced through UFCS.

I realize that changing the language to work this way would probably require an RFC. I'd just like to ask if there are any known issues that would prevent this.

@Dylan-DPC
Copy link
Member

Closing this as the clippy issue covers it to the extent of what can be done.

@Dylan-DPC Dylan-DPC closed this as not planned Won't fix, can't repro, duplicate, stale Nov 18, 2023
lnicola pushed a commit to lnicola/rust that referenced this issue May 19, 2024
fix: Fix attributes on generic parameters colliding in item tree

Fixes rust-lang/rust-analyzer#16141
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 C-feature-request Category: A feature request, i.e: not implemented / a PR. 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

8 participants