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

unused_parens false positive on addition of blocks? #71290

Closed
vpzomtrrfrt opened this issue Apr 18, 2020 · 2 comments · Fixed by #71910
Closed

unused_parens false positive on addition of blocks? #71290

vpzomtrrfrt opened this issue Apr 18, 2020 · 2 comments · Fixed by #71910
Assignees
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@vpzomtrrfrt
Copy link

vpzomtrrfrt commented Apr 18, 2020

In this example:

pub fn foo(a: bool, b: bool) -> u8 {
    (if a { 1 } else { 0 } + if b { 1 } else { 0 })
}

rustc throws a warning:

warning: unnecessary parentheses around block return value
 --> src/lib.rs:2:5
  |
2 |     (if a { 1 } else { 0 } + if b { 1 } else { 0 })
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove these parentheses
  |
  = note: `#[warn(unused_parens)]` on by default

Removing the parentheses as the message directs causes errors:

error: expected expression, found `+`
 --> src/lib.rs:2:27
  |
2 |     if a { 1 } else { 0 } + if b { 1 } else { 0 }
  |                           ^ expected expression

error[E0308]: mismatched types
 --> src/lib.rs:2:12
  |
2 |     if a { 1 } else { 0 } + if b { 1 } else { 0 }
  |     -------^-------------- help: consider using a semicolon here
  |     |      |
  |     |      expected `()`, found integer
  |     expected this to be `()`

error[E0308]: mismatched types
 --> src/lib.rs:2:23
  |
2 |     if a { 1 } else { 0 } + if b { 1 } else { 0 }
  |     ------------------^--- help: consider using a semicolon here
  |     |                 |
  |     |                 expected `()`, found integer
  |     expected this to be `()`

error: aborting due to 3 previous errors

It seems the warning is wrong, unless this is expected to compile

Meta

rustc --version --verbose:

rustc 1.42.0 (b8cedc004 2020-03-09)
binary: rustc
commit-hash: b8cedc00407a4c56a3bda1ed605c6fc166655447
commit-date: 2020-03-09
host: x86_64-unknown-linux-gnu
release: 1.42.0
LLVM version: 9.0

This also happens on nightly.

rustc 1.44.0-nightly (ce93331e2 2020-04-17)
binary: rustc
commit-hash: ce93331e2cf21ac4b72a53854b105955919114e7
commit-date: 2020-04-17
host: x86_64-unknown-linux-gnu
release: 1.44.0-nightly
LLVM version: 9.0

This issue has been assigned to @mibac138 via this comment.

@vpzomtrrfrt vpzomtrrfrt added the C-bug Category: This is a bug. label Apr 18, 2020
@jonas-schievink jonas-schievink 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. labels Apr 18, 2020
@cuviper
Copy link
Member

cuviper commented Apr 21, 2020

I think this may apply to any binary op where the left-hand side is an expression that can be a statement without a semicolon, per:

pub fn expr_requires_semi_to_be_stmt(e: &ast::Expr) -> bool {
match e.kind {
ast::ExprKind::If(..)
| ast::ExprKind::Match(..)
| ast::ExprKind::Block(..)
| ast::ExprKind::While(..)
| ast::ExprKind::Loop(..)
| ast::ExprKind::ForLoop(..)
| ast::ExprKind::TryBlock(..) => false,
_ => true,
}
}

For example, adding with block expressions also reports unused parens:

pub fn foo(a: bool, b: bool) -> u8 {
    ({ u8::from(a) } + { u8::from(b) })
}
warning: unnecessary parentheses around block return value
 --> src/lib.rs:2:5
  |
2 |     ({ u8::from(a) } + { u8::from(b) })
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove these parentheses
  |
  = note: `#[warn(unused_parens)]` on by default

but removing those gets an error:

error: expected expression, found `+`
 --> src/lib.rs:2:21
  |
2 |     { u8::from(a) } + { u8::from(b) }
  |     --------------- ^ expected expression
  |     |
  |     help: parentheses are required to parse this as an expression: `({ u8::from(a) })`

error[E0308]: mismatched types
 --> src/lib.rs:2:7
  |
2 |     { u8::from(a) } + { u8::from(b) }
  |       ^^^^^^^^^^^- help: try adding a semicolon: `;`
  |       |
  |       expected `()`, found `u8`

error: aborting due to 2 previous errors

As a workaround, it doesn't trigger unused_parens if the operands are wrapped separately:

pub fn foo(a: bool, b: bool) -> u8 {
    (if a { 1 } else { 0 }) + (if b { 1 } else { 0 })
}

or

pub fn foo(a: bool, b: bool) -> u8 {
    ({ u8::from(a) }) + ({ u8::from(b) })
}

@cuviper cuviper added the D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. label Apr 21, 2020
@mibac138
Copy link
Contributor

mibac138 commented May 4, 2020

@rustbot claim

@rustbot rustbot self-assigned this May 4, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 12, 2020
Fix unused_parens false positive when using binary operations

Fixes rust-lang#71290

r? @cuviper who provided instructions
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 12, 2020
Fix unused_parens false positive when using binary operations

Fixes rust-lang#71290

r? @cuviper who provided instructions
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 13, 2020
Fix unused_parens false positive when using binary operations

Fixes rust-lang#71290

r? @cuviper who provided instructions
RalfJung added a commit to RalfJung/rust that referenced this issue May 14, 2020
Fix unused_parens false positive when using binary operations

Fixes rust-lang#71290

r? @cuviper who provided instructions
@bors bors closed this as completed in b20b200 May 14, 2020
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. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants