Duplicate inherent static methods can be defined in separate `impl` blocks

[Byron]

This code I used to compile fine with a version of Rust that was about 9 days old. All the comments in the code have been there before, now the code fails to compile at the lines assert_eq!(Foo::id(), "FOO"); and assert_eq!(Bar::id(), "BAR");:

#[test]
fn type_aliases() {
    struct Foo {
        a: u32,
    };
    impl Foo {
        fn bark(&self) {
            println!("Wuff {}", self.a);
        }

        fn id() -> &'static str {
            "FOO"
        }
    }
    assert_eq!(Foo::id(), "FOO");
    let f = Foo { a: 1 };
    f.bark();

    type Bar = Foo;
    // assert_eq!(!Bar::id(), "FOO");
    // error: unresolved name `Bar::id`
    // tests/lang.rs:628     assert_eq!(!Bar::id(), "FOO");
    let b = Bar { a: 2 };
    b.bark(); // this works though

    impl Bar {
        // This would add a similarly named implementation, that is difficult to call
        // due to ambiguity.
        // Interestingly, it also affects Foo, as well as Bar !!
        // fn bark(&self) {
        //     println!("Grrrr {}", self.a);
        // }

        fn id() -> &'static str {
            "BAR"
        }   
    }
    // b.bark(); // or f.bark();
    // error: multiple applicable methods in scope [E0034]
    // tests/lang.rs:625     f.bark();
    //                         ^~~~~~
    // tests/lang.rs:615:9: 617:10 note: candidate #1 is defined in an impl for the type `type_aliases::Foo`
    // tests/lang.rs:615         fn bark(&self) {
    // tests/lang.rs:616             println!("Wuff {}", self.a);
    // tests/lang.rs:617         }
    // tests/lang.rs:637:9: 639:10 note: candidate #2 is defined in an impl for the type `type_aliases::Foo`
    // tests/lang.rs:637         fn bark(&self) {
    // tests/lang.rs:638             println!("Grrrr {}", self.a);
    assert_eq!(Bar::id(), "BAR");
}

When compiling this now, I get this error, which seems wrong as Bar affects Foo:

tests/lang.rs:636:16: 636:23 error: multiple applicable methods in scope [E0034]
tests/lang.rs:636     assert_eq!(Foo::id(), "FOO");
                                 ^~~~~~~
<std macros>:1:1: 9:39 note: in expansion of assert_eq!
tests/lang.rs:636:5: 636:34 note: expansion site
tests/lang.rs:632:9: 634:10 note: candidate #1 is defined in an impl for the type `type_aliases::Foo`
tests/lang.rs:632         fn id() -> &'static str {
tests/lang.rs:633             "FOO"
tests/lang.rs:634         }
tests/lang.rs:655:9: 657:10 note: candidate #2 is defined in an impl for the type `type_aliases::Foo`
tests/lang.rs:655         fn id() -> &'static str {
tests/lang.rs:656             "BAR"
tests/lang.rs:657         }
<std macros>:5:10: 5:20 error: the type of this value must be known in this context
<std macros>:5 if ! ( ( * left_val == * right_val ) && ( * right_val == * left_val ) ) {
                        ^~~~~~~~~~
<std macros>:1:1: 9:39 note: in expansion of assert_eq!
tests/lang.rs:636:5: 636:34 note: expansion site
<std macros>:5:24: 5:35 error: the type of this value must be known in this context
<std macros>:5 if ! ( ( * left_val == * right_val ) && ( * right_val == * left_val ) ) {
                                      ^~~~~~~~~~~
<std macros>:1:1: 9:39 note: in expansion of assert_eq!
tests/lang.rs:636:5: 636:34 note: expansion site
tests/lang.rs:670:16: 670:23 error: multiple applicable methods in scope [E0034]
tests/lang.rs:670     assert_eq!(Bar::id(), "BAR");
                                 ^~~~~~~
<std macros>:1:1: 9:39 note: in expansion of assert_eq!
tests/lang.rs:670:5: 670:34 note: expansion site
tests/lang.rs:632:9: 634:10 note: candidate #1 is defined in an impl for the type `type_aliases::Foo`
tests/lang.rs:632         fn id() -> &'static str {
tests/lang.rs:633             "FOO"
tests/lang.rs:634         }
tests/lang.rs:655:9: 657:10 note: candidate #2 is defined in an impl for the type `type_aliases::Foo`
tests/lang.rs:655         fn id() -> &'static str {
tests/lang.rs:656             "BAR"
tests/lang.rs:657         }
<std macros>:5:10: 5:20 error: the type of this value must be known in this context
<std macros>:5 if ! ( ( * left_val == * right_val ) && ( * right_val == * left_val ) ) {
                        ^~~~~~~~~~
<std macros>:1:1: 9:39 note: in expansion of assert_eq!
tests/lang.rs:670:5: 670:34 note: expansion site
<std macros>:5:24: 5:35 error: the type of this value must be known in this context
<std macros>:5 if ! ( ( * left_val == * right_val ) && ( * right_val == * left_val ) ) {
                                      ^~~~~~~~~~~
<std macros>:1:1: 9:39 note: in expansion of assert_eq!
tests/lang.rs:670:5: 670:34 note: expansion site

Even if this is now desired, I think the compiler message could be improved - it looks similar for the two candidates, and if it wasn't for the code shown below it, I would look like it means the same thing.

Meta

$ rustc --verbose --version
rustc 1.0.0-nightly (4db0b3246 2015-02-25) (built 2015-02-25)
binary: rustc
commit-hash: 4db0b32467535d718d6474de7ae8d1007d900818
commit-date: 2015-02-25
build-date: 2015-02-25
host: x86_64-apple-darwin
release: 1.0.0-nightly

[huonw]

triage: I-nominated

I think this is fundamentally caused by the following compiling, which seems fairly bad:

struct Foo;
impl Foo {
    fn id() {}
}
impl Foo {
    fn id() {}
}
fn main() {}

Putting both id's in the same impl fails to compile, as does trying to call Foo::id().

[nikomatsakis]

So this came up in the context of the specialization RFC rust-lang/rfcs#1210. Initially that RFC tried to rationalize inherent impls as being from an anonymous trait, but that language was pulled. Nonetheless, I (and @aturon) think that this behavior is basically a bug and we should deprecate overlapping inherent impls of this kind. The question is what the precise rules OUGHT to be.

Some possibilities:

• cannot have two inherent impls with the same fn, period.
• cannot have two inherent impls with the same fn unless coherence judges them to be disjoint (using the overlap checker rules).

[nikomatsakis]

The latter is obviously more backwards compatible, so it'd be good to try and get some crater results.

[eddyb]

While there may be interactions with specialization, inherent methods are more flexible, so even the disjoint rule can be non-trivial, if we ever want argument-type-based-lookup.

At least it wouldn't affect existing code with callable methods, as the current lookup implementation would result in ambiguity even if the signatures are disjoint (but Self isn't).

[nikomatsakis]

At least it wouldn't affect existing code with callable methods, as the current lookup implementation would result in ambiguity even if the signatures are disjoint (but Self isn't).

Yeah, good point. Hopefully impact would be minimal. Still, it's not so hard to have types that are disjoint in practice, but not disjoint enough for the coherence checker -- particularly given the conservative rules imposed by RFC 1023.

[nikomatsakis]

triage: P-high