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

Non-unique argument names should not be allowed in a trait method #104374

Open
greenhat opened this issue Nov 13, 2022 · 7 comments
Open

Non-unique argument names should not be allowed in a trait method #104374

greenhat opened this issue Nov 13, 2022 · 7 comments
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@greenhat
Copy link

greenhat commented Nov 13, 2022

I tried this code (see the A::f arguments have the same names - a):

  trait A {
      fn f(a: u32, a: u32) -> u32;
  }

  struct B;

  impl A for B {
      fn f(a: u32, b: u32) -> u32 {
          a + b
      }
  }

  fn main() {
      println!("{}", B::f(1, 2));
  }

I expected to see this happen: the compiler throws an error. the compiler shows a warning (if enabled). EDIT: The hard error is too much. The lint fits better. See the comments below.

Instead, this happened: the compiler accepted this code.

Meta

rustc --version --verbose:

rustc 1.67.0-nightly (6284998a2 2022-11-12)
binary: rustc
commit-hash: 6284998a2677d7e3e8420db783f3aa4fd80d7423
commit-date: 2022-11-12
host: x86_64-unknown-linux-gnu
release: 1.67.0-nightly
LLVM version: 15.0.4
@greenhat greenhat added the C-bug Category: This is a bug. label Nov 13, 2022
@greenhat
Copy link
Author

I'd like to take a stab at it.

@compiler-errors
Copy link
Member

This is not something that can be turned into a hard-error straight away, since it would break backwards compatibility. This should probably be made a lint first, then discussion can be had whether or not to promote it to a hard error.

@Urgau
Copy link
Member

Urgau commented Nov 13, 2022

This has been valid code (no compiler error and no warnings) since Rust 1.0: https://rust.godbolt.org/z/7daPYqce7

And as @compiler-errors said should really be a lint first, whenever it should be enable by default or made into a error is a question for later.

@est31
Copy link
Member

est31 commented Nov 14, 2022

It used to be allowed to have no parameter name at all, so fn(u32, u32) -> u32; until that was deprecated for editions 2018+, see #41686 .

@greenhat
Copy link
Author

Got it. I'll start with a lint.

@petrochenkov
Copy link
Contributor

Duplicate of #33995.
I'm not even sure it's something that needs to be fixed.

@greenhat
Copy link
Author

Duplicate of #33995. I'm not even sure it's something that needs to be fixed.

I stumbled upon it at bytecodealliance/wasmtime#5259, and I would appreciate rustc's help to avoid such cases.

@ChrisDenton ChrisDenton added the needs-triage-legacy Old issue that were never triaged. Remove this label once the issue has been sufficiently triaged. label Jul 16, 2023
@fmease fmease added A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed needs-triage-legacy Old issue that were never triaged. Remove this label once the issue has been sufficiently triaged. labels Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. 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

7 participants