Skip to content

Commit

Permalink
Auto merge of rust-lang#12365 - Ethiraric:fix-11968, r=Alexendoo
Browse files Browse the repository at this point in the history
[`unnecessary_cast`]: Avoid breaking precedence

 If the whole cast expression is a unary expression (`(*x as T)`) or an  addressof expression (`(&x as T)`), then not surrounding the suggestion  into a block risks us changing the precedence of operators if the cast  expression is followed by an operation with higher precedence than the  unary operator (`(*x as T).foo()` would become `*x.foo()`, which  changes what the `*` applies on).
The same is true if the expression encompassing the cast expression is a unary expression or an addressof expression.

The lint supports the latter case, but missed the former one. This PR fixes that.

Fixes rust-lang#11968

*Please write a short comment explaining your change (or "none" for internal only changes)*

changelog: [`unnecessary_cast`]: Avoid breaking precedence with unary operators (`(*x as T).foo()` --  before: `*x.foo()` -- now: `{*x}.foo()`)
  • Loading branch information
bors committed Feb 27, 2024
2 parents 1013617 + c6cb0e9 commit 4155bec
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 2 deletions.
13 changes: 12 additions & 1 deletion clippy_lints/src/casts/unnecessary_cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,13 +151,24 @@ pub(super) fn check<'tcx>(
return false;
}

// If the whole cast expression is a unary expression (`(*x as T)`) or an addressof
// expression (`(&x as T)`), then not surrounding the suggestion into a block risks us
// changing the precedence of operators if the cast expression is followed by an operation
// with higher precedence than the unary operator (`(*x as T).foo()` would become
// `*x.foo()`, which changes what the `*` applies on).
// The same is true if the expression encompassing the cast expression is a unary
// expression or an addressof expression.
let needs_block = matches!(cast_expr.kind, ExprKind::Unary(..) | ExprKind::AddrOf(..))
|| get_parent_expr(cx, expr)
.map_or(false, |e| matches!(e.kind, ExprKind::Unary(..) | ExprKind::AddrOf(..)));

span_lint_and_sugg(
cx,
UNNECESSARY_CAST,
expr.span,
&format!("casting to the same type is unnecessary (`{cast_from}` -> `{cast_to}`)"),
"try",
if get_parent_expr(cx, expr).map_or(false, |e| matches!(e.kind, ExprKind::AddrOf(..))) {
if needs_block {
format!("{{ {cast_str} }}")
} else {
cast_str
Expand Down
6 changes: 6 additions & 0 deletions tests/ui/unnecessary_cast.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -221,4 +221,10 @@ mod fixable {
fn issue_9603() {
let _: f32 = -0x400 as f32;
}

// Issue #11968: The suggestion for this lint removes the parentheses and leave the code as
// `*x.pow(2)` which tries to dereference the return value rather than `x`.
fn issue_11968(x: &usize) -> usize {
{ *x }.pow(2)
}
}
6 changes: 6 additions & 0 deletions tests/ui/unnecessary_cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,4 +221,10 @@ mod fixable {
fn issue_9603() {
let _: f32 = -0x400 as f32;
}

// Issue #11968: The suggestion for this lint removes the parentheses and leave the code as
// `*x.pow(2)` which tries to dereference the return value rather than `x`.
fn issue_11968(x: &usize) -> usize {
(*x as usize).pow(2)
}
}
8 changes: 7 additions & 1 deletion tests/ui/unnecessary_cast.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -241,5 +241,11 @@ error: casting to the same type is unnecessary (`f32` -> `f32`)
LL | let _num = foo() as f32;
| ^^^^^^^^^^^^ help: try: `foo()`

error: aborting due to 40 previous errors
error: casting to the same type is unnecessary (`usize` -> `usize`)
--> tests/ui/unnecessary_cast.rs:228:9
|
LL | (*x as usize).pow(2)
| ^^^^^^^^^^^^^ help: try: `{ *x }`

error: aborting due to 41 previous errors

0 comments on commit 4155bec

Please sign in to comment.