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

Does not fail on multiple methods definition #58942

Closed
rom1v opened this issue Mar 5, 2019 · 4 comments
Closed

Does not fail on multiple methods definition #58942

rom1v opened this issue Mar 5, 2019 · 4 comments
Labels
C-bug Category: This is a bug. regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@rom1v
Copy link

rom1v commented Mar 5, 2019

This obviously fails to compile:

struct X;
impl X { 
   fn f(&self) {}
}
impl X {
   fn f(&self) {}
}
error[E0592]: duplicate definitions with name `f`
 --> a.rs:3:4
  |
3 |    fn f(&self) {}
  |    ^^^^^^^^^^^^^^ duplicate definitions for `f`
...
6 |    fn f(&self) {}
  |    -------------- other definition for `f`

But this does not:

struct X;
impl X {
   fn f(&self) {}
}
macro_rules! do_impl {
    ($name: ident) => {
        impl $name {
            fn f(&self) {}
        }
    }
}
do_impl!(X);

unless you use it:

fn main() {
    X.f()
}
error[E0034]: multiple applicable items in scope
  --> a.rs:14:7
   |
14 |     X.f()
   |       ^ multiple `f` found
   |
note: candidate #1 is defined in an impl for the type `X`
  --> a.rs:3:4
   |
3  |    fn f(&self) {}
   |    ^^^^^^^^^^^
note: candidate #2 is defined in an impl for the type `X`
  --> a.rs:8:13
   |
8  |             fn f(&self) {}
   |             ^^^^^^^^^^^
...
12 | do_impl!(X);
   | ------------ in this macro invocation

I think it should fail even when we don't use the duplicate function.

@jonas-schievink jonas-schievink added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. labels Mar 5, 2019
@Centril
Copy link
Contributor

Centril commented Mar 5, 2019

cc @petrochenkov

This compiles all the way back to 1.0.0 (tested with godbolt).

@petrochenkov
Copy link
Contributor

Ugh, fn check_for_common_items_in_impls compares Idents (which are not equal if they come from different macros).

I would actually check the intermediate versions of Rust between 1.0 and 1.33, IIRC this error should be impossible in some version range due to how identifiers worked back then.

Slightly more minimized reproduction:

struct X;

impl X {
   fn f() {}
}

macro_rules! do_impl { () => {
    impl X {
       fn f() {}
    }
}}
do_impl!();

fn main() {}

@petrochenkov
Copy link
Contributor

Fixed in #58948

@petrochenkov petrochenkov removed their assignment Mar 5, 2019
@petrochenkov
Copy link
Contributor

The regression was introduced in Rust 1.29, the error was reported before that.
(This case became an error in Rust 1.18 after a long deprecation lint period.)

@petrochenkov petrochenkov added the regression-from-stable-to-stable Performance or correctness regression from one stable version to another. label Mar 5, 2019
bors added a commit that referenced this issue Mar 18, 2019
hygiene: Fix identifier comparison in impl overlap check

Fixes #58942
bors added a commit that referenced this issue Mar 21, 2019
hygiene: Fix identifier comparison in impl overlap check

Fixes #58942
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants