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

Unnecessary parentheses warning for (A | B) as :pat in 2018 edition #86959

Open
ExpHP opened this issue Jul 7, 2021 · 3 comments
Open

Unnecessary parentheses warning for (A | B) as :pat in 2018 edition #86959

ExpHP opened this issue Jul 7, 2021 · 3 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. L-unused_parens Lint: unused_parens P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@ExpHP
Copy link
Contributor

ExpHP commented Jul 7, 2021

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=30f392098326a970e468c3125f7c5413

macro_rules! mac {
    ($expr:expr, $pat:pat) => {
        match $expr {
            $pat => {},
            _ => panic!(),
        }
    };
}

fn main() {
    mac!('a', ('a' | 'A'));
}

The current output is:

   Compiling playground v0.0.1 (/playground)
warning: unnecessary parentheses around pattern
  --> src/main.rs:12:15
   |
12 |     mac!('a', ('a' | 'A'));
   |               ^^^^^^^^^^^ help: remove these parentheses
   |
   = note: `#[warn(unused_parens)]` on by default

warning: 1 warning emitted

While it would be true that these parentheses are unnecessary in 2021 edition, (and also of course in the macro expanded output as well, which is why the warning occurs), they are required here in 2018 edition because :pat cannot match 'a' | 'A' in this edition, making the warning a nuisance.

@ExpHP ExpHP added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 7, 2021
@jonas-schievink jonas-schievink added C-bug Category: This is a bug. I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Jul 7, 2021
@apiraino
Copy link
Contributor

apiraino commented Jul 8, 2021

I've tried bisecting this and it seems that the warning was introduced a long time ago, exactly on nightly-2019-08-19 (before was a hard error). Probably goes back to #54883

found 7 bors merge commits in the specified range
  commit[0] 2019-08-17UTC: Auto merge of #63658 - RalfJung:miri-op, r=oli-obk
  commit[1] 2019-08-17UTC: Auto merge of #63671 - Centril:rollup-zufavt5, r=Centril
  commit[2] 2019-08-18UTC: Auto merge of #61708 - dlrobertson:or-patterns-0, r=centril
  commit[3] 2019-08-18UTC: Auto merge of #62948 - matklad:failable-file-loading, r=petrochenkov
  commit[4] 2019-08-18UTC: Auto merge of #63269 - Aaron1011:feature/proc-macro-data, r=eddyb,petrochenkov
  commit[5] 2019-08-18UTC: Auto merge of #63635 - oli-obk:default-slice-dangles, r=eddyb
  commit[6] 2019-08-18UTC: Auto merge of #63659 - gilescope:async-in-closure, r=Centril
ERROR: no commits between 2111aed0a38c819acb140c7153e9366964a37f2f and 4cf7673076e6975532213e494dd3f7f9d8c2328e within last 167 days

bisectecd with

RUST_SRC_REPO=~/projects/rust/rust cargo-bisect-rustc --preserve --start 2019-01-01 --script=./test.sh

and this helper script

#!/usr/bin/env bash
out=$(cargo check 2>&1)
if echo "${out}" | grep -q "remove these paren"; then
    exit 1
fi
exit 0

@ExpHP
Copy link
Contributor Author

ExpHP commented Jul 8, 2021

Yes, I wouldn't expect bisecting to tell much in this instance because I wouldn't expect this to be a regression? I could be wrong, but it seems pretty clear how this behavior arises as a logical consequence of other behaviors of the compiler, and normally "unused" warnings are just a basic fact of life when working with macros.

I created an issue for this warning because I feel that this one is particularly easy to run into (i.e. it's "a paper cut"); there is a clear use case for writing parenthesized | patterns in 2018 edition, and those parens will almost always be "unused" once expanded.

@apiraino
Copy link
Contributor

apiraino commented Jul 9, 2021

@ExpHP yes, totally agree. I've tried a diagnose because I was confused about the "edition 2021" part of your report: also trying to make this code "2021-ready", the warning does not disappear without forcing it out by adding #![allow(unused_parens)] (which is a bit inconvenient).

Perhaps related to #80636 and (in cascade) to #78747

Assigning priority as discussed in the Zulip thread of the Prioritization Working Group.

@rustbot label -I-prioritize +P-medium

@rustbot rustbot added P-medium Medium priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Jul 9, 2021
@jieyouxu jieyouxu added the L-unused_parens Lint: unused_parens label May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. L-unused_parens Lint: unused_parens P-medium Medium priority 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

5 participants