Skip to content

Tracking issue for duplicate_matcher_binding_names compatibility lint #57742

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
3 tasks done
mark-i-m opened this issue Jan 18, 2019 · 5 comments · Fixed by #59858
Closed
3 tasks done

Tracking issue for duplicate_matcher_binding_names compatibility lint #57742

mark-i-m opened this issue Jan 18, 2019 · 5 comments · Fixed by #59858
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) C-future-incompatibility Category: Future-incompatibility lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@mark-i-m
Copy link
Member

mark-i-m commented Jan 18, 2019

What is this lint about

In #57593, it was observed that duplicate macro matcher bindings are allowed in declarations, but unusable in invocations:

macro_rules! foo {
  ($a:expr, $a:expr) => {}
}

This is macro compiles fine on its own, but any attempt to use it will result in an error because $a is bound twice. This is likely an oversight in the original implementation, but it really feels like a bug, so we want to make this a hard error.

This compatibility lint warns on such a declaration. The end goal is to make this a hard error in a few releases.

How to fix this warning/error

Change the binding names to be different.

Current status

@mark-i-m
Copy link
Member Author

cc @Centril

@Centril Centril added T-lang Relevant to the language team, which will review and decide on the PR/issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-future-incompatibility Category: Future-incompatibility lints A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) labels Jan 18, 2019
@mark-i-m
Copy link
Member Author

Note: The future compatibility lint has hit stable by now...

@Centril
Copy link
Contributor

Centril commented Mar 23, 2019

@mark-i-m I think we have given it enough time; would you like to prepare a PR that turns it into a hard error directly and let's crater + merge that one?

@mark-i-m
Copy link
Member Author

@Centril Sure, but should we not make it deny-by-default first? I don't there is a particular hurry on this, right?

Centril added a commit to Centril/rust that referenced this issue Mar 29, 2019
…Centril

warn -> deny duplicate match bindings

This is the next step of rust-lang#57742

r? @Centril

- [x] Decide whether to go to deny-by-default or hard error.
     - My preference is to make this deny-by-default, rather than going straight to a hard error. The CI should fail because I haven't updated the ui test yet. I'll update it when we decide which to do.
- [x] Update [test](https://github.com/mark-i-m/rust/blob/c25d6b83441e0c060ee0273193ef27b29e1318cd/src/test/ui/macros/macro-multiple-matcher-bindings.rs)
- [ ] ~Crater run~ see rust-lang#59394 (comment)
@mark-i-m
Copy link
Member Author

Note to self: The next step is to merge this branch: master...mark-i-m:dup-matcher-bindings-3

Centril added a commit to Centril/rust that referenced this issue Apr 12, 2019
…Centril

Make duplicate matcher bindings a hard error

r? @Centril

Closes rust-lang#57742
Centril added a commit to Centril/rust that referenced this issue Apr 13, 2019
…Centril

Make duplicate matcher bindings a hard error

r? @Centril

Closes rust-lang#57742
Centril added a commit to Centril/rust that referenced this issue Apr 13, 2019
…Centril

Make duplicate matcher bindings a hard error

r? @Centril

Closes rust-lang#57742
Centril added a commit to Centril/rust that referenced this issue Apr 13, 2019
…Centril

Make duplicate matcher bindings a hard error

r? @Centril

Closes rust-lang#57742
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) C-future-incompatibility Category: Future-incompatibility lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants