mirror of
https://github.com/rust-lang/rust-clippy
synced 2024-11-27 15:11:30 +00:00
lint on nested binary operations involving local and handle projections
This commit is contained in:
y21
parent
7343db9ce3
commit
affde93041
4 changed files with 117 additions and 15 deletions
|
|
@ -1,6 +1,6 @@
|
|||
use clippy_utils::diagnostics::span_lint_and_then;
|
||||
use clippy_utils::ty::is_normalizable;
|
||||
use clippy_utils::{path_to_local, path_to_local_id};
|
||||
use clippy_utils::{eq_expr_value, path_to_local};
|
||||
use rustc_abi::WrappingRange;
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir::{Expr, ExprKind, Node};
|
||||
|
|
@ -25,6 +25,36 @@ fn range_fully_contained(from: WrappingRange, to: WrappingRange) -> bool {
|
|||
to.contains(from.start) && to.contains(from.end)
|
||||
}
|
||||
|
||||
/// Checks if a given expression is a binary operation involving a local variable or is made up of
|
||||
/// other (nested) binary expressions involving the local. There must be at least one local
|
||||
/// reference that is the same as `local_expr`.
|
||||
///
|
||||
/// This is used as a heuristic to detect if a variable
|
||||
/// is checked to be within the valid range of a transmuted type.
|
||||
/// All of these would return true:
|
||||
/// * `x < 4`
|
||||
/// * `x < 4 && x > 1`
|
||||
/// * `x.field < 4 && x.field > 1` (given `x.field`)
|
||||
/// * `x.field < 4 && unrelated()`
|
||||
fn binops_with_local(cx: &LateContext<'_>, local_expr: &Expr<'_>, expr: &Expr<'_>) -> bool {
|
||||
match expr.kind {
|
||||
ExprKind::Binary(_, lhs, rhs) => {
|
||||
binops_with_local(cx, local_expr, lhs) || binops_with_local(cx, local_expr, rhs)
|
||||
},
|
||||
_ => eq_expr_value(cx, local_expr, expr),
|
||||
}
|
||||
}
|
||||
|
||||
/// Checks if an expression is a path to a local variable (with optional projections), e.g.
|
||||
/// `x.field[0].field2` would return true.
|
||||
fn is_local_with_projections(expr: &Expr<'_>) -> bool {
|
||||
match expr.kind {
|
||||
ExprKind::Path(_) => path_to_local(expr).is_some(),
|
||||
ExprKind::Field(expr, _) | ExprKind::Index(expr, ..) => is_local_with_projections(expr),
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn check<'tcx>(
|
||||
cx: &LateContext<'tcx>,
|
||||
expr: &'tcx Expr<'tcx>,
|
||||
|
|
@ -36,9 +66,8 @@ pub(super) fn check<'tcx>(
|
|||
&& let ExprKind::MethodCall(path, receiver, [arg], _) = then_some_call.kind
|
||||
&& cx.typeck_results().expr_ty(receiver).is_bool()
|
||||
&& path.ident.name == sym!(then_some)
|
||||
&& let ExprKind::Binary(_, lhs, rhs) = receiver.kind
|
||||
&& let Some(local_id) = path_to_local(transmutable)
|
||||
&& (path_to_local_id(lhs, local_id) || path_to_local_id(rhs, local_id))
|
||||
&& is_local_with_projections(transmutable)
|
||||
&& binops_with_local(cx, transmutable, receiver)
|
||||
&& is_normalizable(cx, cx.param_env, from_ty)
|
||||
&& is_normalizable(cx, cx.param_env, to_ty)
|
||||
// we only want to lint if the target type has a niche that is larger than the one of the source type
|
||||
|
|
|
|||
|
|
@ -12,16 +12,36 @@ enum Opcode {
|
|||
Div = 3,
|
||||
}
|
||||
|
||||
struct Data {
|
||||
foo: &'static [u8],
|
||||
bar: &'static [u8],
|
||||
}
|
||||
|
||||
fn int_to_opcode(op: u8) -> Option<Opcode> {
|
||||
(op < 4).then(|| unsafe { std::mem::transmute(op) })
|
||||
}
|
||||
|
||||
fn f(op: u8, unrelated: u8) {
|
||||
fn f(op: u8, op2: Data, unrelated: u8) {
|
||||
true.then_some(unsafe { std::mem::transmute::<_, Opcode>(op) });
|
||||
(unrelated < 4).then_some(unsafe { std::mem::transmute::<_, Opcode>(op) });
|
||||
(op < 4).then(|| unsafe { std::mem::transmute::<_, Opcode>(op) });
|
||||
(op > 4).then(|| unsafe { std::mem::transmute::<_, Opcode>(op) });
|
||||
(op == 0).then(|| unsafe { std::mem::transmute::<_, Opcode>(op) });
|
||||
|
||||
let _: Option<Opcode> = (op > 0 && op < 10).then(|| unsafe { std::mem::transmute(op) });
|
||||
let _: Option<Opcode> = (op > 0 && op < 10 && unrelated == 0).then(|| unsafe { std::mem::transmute(op) });
|
||||
|
||||
// lint even when the transmutable goes through field/array accesses
|
||||
let _: Option<Opcode> = (op2.foo[0] > 0 && op2.foo[0] < 10).then(|| unsafe { std::mem::transmute(op2.foo[0]) });
|
||||
|
||||
// don't lint: wrong index used in the transmute
|
||||
let _: Option<Opcode> = (op2.foo[0] > 0 && op2.foo[0] < 10).then_some(unsafe { std::mem::transmute(op2.foo[1]) });
|
||||
|
||||
// don't lint: no check for the transmutable in the condition
|
||||
let _: Option<Opcode> = (op2.foo[0] > 0 && op2.bar[1] < 10).then_some(unsafe { std::mem::transmute(op2.bar[0]) });
|
||||
|
||||
// don't lint: wrong variable
|
||||
let _: Option<Opcode> = (op2.foo[0] > 0 && op2.bar[1] < 10).then_some(unsafe { std::mem::transmute(op) });
|
||||
}
|
||||
|
||||
unsafe fn f2(op: u8) {
|
||||
|
|
|
|||
|
|
@ -12,16 +12,36 @@ enum Opcode {
|
|||
Div = 3,
|
||||
}
|
||||
|
||||
struct Data {
|
||||
foo: &'static [u8],
|
||||
bar: &'static [u8],
|
||||
}
|
||||
|
||||
fn int_to_opcode(op: u8) -> Option<Opcode> {
|
||||
(op < 4).then_some(unsafe { std::mem::transmute(op) })
|
||||
}
|
||||
|
||||
fn f(op: u8, unrelated: u8) {
|
||||
fn f(op: u8, op2: Data, unrelated: u8) {
|
||||
true.then_some(unsafe { std::mem::transmute::<_, Opcode>(op) });
|
||||
(unrelated < 4).then_some(unsafe { std::mem::transmute::<_, Opcode>(op) });
|
||||
(op < 4).then_some(unsafe { std::mem::transmute::<_, Opcode>(op) });
|
||||
(op > 4).then_some(unsafe { std::mem::transmute::<_, Opcode>(op) });
|
||||
(op == 0).then_some(unsafe { std::mem::transmute::<_, Opcode>(op) });
|
||||
|
||||
let _: Option<Opcode> = (op > 0 && op < 10).then_some(unsafe { std::mem::transmute(op) });
|
||||
let _: Option<Opcode> = (op > 0 && op < 10 && unrelated == 0).then_some(unsafe { std::mem::transmute(op) });
|
||||
|
||||
// lint even when the transmutable goes through field/array accesses
|
||||
let _: Option<Opcode> = (op2.foo[0] > 0 && op2.foo[0] < 10).then_some(unsafe { std::mem::transmute(op2.foo[0]) });
|
||||
|
||||
// don't lint: wrong index used in the transmute
|
||||
let _: Option<Opcode> = (op2.foo[0] > 0 && op2.foo[0] < 10).then_some(unsafe { std::mem::transmute(op2.foo[1]) });
|
||||
|
||||
// don't lint: no check for the transmutable in the condition
|
||||
let _: Option<Opcode> = (op2.foo[0] > 0 && op2.bar[1] < 10).then_some(unsafe { std::mem::transmute(op2.bar[0]) });
|
||||
|
||||
// don't lint: wrong variable
|
||||
let _: Option<Opcode> = (op2.foo[0] > 0 && op2.bar[1] < 10).then_some(unsafe { std::mem::transmute(op) });
|
||||
}
|
||||
|
||||
unsafe fn f2(op: u8) {
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
error: this transmute is always evaluated eagerly, even if the condition is false
|
||||
--> $DIR/eager_transmute.rs:16:33
|
||||
--> $DIR/eager_transmute.rs:21:33
|
||||
|
|
||||
LL | (op < 4).then_some(unsafe { std::mem::transmute(op) })
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
@ -12,7 +12,7 @@ LL | (op < 4).then(|| unsafe { std::mem::transmute(op) })
|
|||
| ~~~~ ++
|
||||
|
||||
error: this transmute is always evaluated eagerly, even if the condition is false
|
||||
--> $DIR/eager_transmute.rs:22:33
|
||||
--> $DIR/eager_transmute.rs:27:33
|
||||
|
|
||||
LL | (op < 4).then_some(unsafe { std::mem::transmute::<_, Opcode>(op) });
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
@ -23,7 +23,7 @@ LL | (op < 4).then(|| unsafe { std::mem::transmute::<_, Opcode>(op) });
|
|||
| ~~~~ ++
|
||||
|
||||
error: this transmute is always evaluated eagerly, even if the condition is false
|
||||
--> $DIR/eager_transmute.rs:23:33
|
||||
--> $DIR/eager_transmute.rs:28:33
|
||||
|
|
||||
LL | (op > 4).then_some(unsafe { std::mem::transmute::<_, Opcode>(op) });
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
@ -34,7 +34,7 @@ LL | (op > 4).then(|| unsafe { std::mem::transmute::<_, Opcode>(op) });
|
|||
| ~~~~ ++
|
||||
|
||||
error: this transmute is always evaluated eagerly, even if the condition is false
|
||||
--> $DIR/eager_transmute.rs:24:34
|
||||
--> $DIR/eager_transmute.rs:29:34
|
||||
|
|
||||
LL | (op == 0).then_some(unsafe { std::mem::transmute::<_, Opcode>(op) });
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
@ -45,7 +45,40 @@ LL | (op == 0).then(|| unsafe { std::mem::transmute::<_, Opcode>(op) });
|
|||
| ~~~~ ++
|
||||
|
||||
error: this transmute is always evaluated eagerly, even if the condition is false
|
||||
--> $DIR/eager_transmute.rs:28:24
|
||||
--> $DIR/eager_transmute.rs:31:68
|
||||
|
|
||||
LL | let _: Option<Opcode> = (op > 0 && op < 10).then_some(unsafe { std::mem::transmute(op) });
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
help: consider using `bool::then` to only transmute if the condition holds
|
||||
|
|
||||
LL | let _: Option<Opcode> = (op > 0 && op < 10).then(|| unsafe { std::mem::transmute(op) });
|
||||
| ~~~~ ++
|
||||
|
||||
error: this transmute is always evaluated eagerly, even if the condition is false
|
||||
--> $DIR/eager_transmute.rs:32:86
|
||||
|
|
||||
LL | let _: Option<Opcode> = (op > 0 && op < 10 && unrelated == 0).then_some(unsafe { std::mem::transmute(op) });
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
help: consider using `bool::then` to only transmute if the condition holds
|
||||
|
|
||||
LL | let _: Option<Opcode> = (op > 0 && op < 10 && unrelated == 0).then(|| unsafe { std::mem::transmute(op) });
|
||||
| ~~~~ ++
|
||||
|
||||
error: this transmute is always evaluated eagerly, even if the condition is false
|
||||
--> $DIR/eager_transmute.rs:35:84
|
||||
|
|
||||
LL | let _: Option<Opcode> = (op2.foo[0] > 0 && op2.foo[0] < 10).then_some(unsafe { std::mem::transmute(op2.foo[0]) });
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
help: consider using `bool::then` to only transmute if the condition holds
|
||||
|
|
||||
LL | let _: Option<Opcode> = (op2.foo[0] > 0 && op2.foo[0] < 10).then(|| unsafe { std::mem::transmute(op2.foo[0]) });
|
||||
| ~~~~ ++
|
||||
|
||||
error: this transmute is always evaluated eagerly, even if the condition is false
|
||||
--> $DIR/eager_transmute.rs:48:24
|
||||
|
|
||||
LL | (op < 4).then_some(std::mem::transmute::<_, Opcode>(op));
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
@ -56,7 +89,7 @@ LL | (op < 4).then(|| std::mem::transmute::<_, Opcode>(op));
|
|||
| ~~~~ ++
|
||||
|
||||
error: this transmute is always evaluated eagerly, even if the condition is false
|
||||
--> $DIR/eager_transmute.rs:57:60
|
||||
--> $DIR/eager_transmute.rs:77:60
|
||||
|
|
||||
LL | let _: Option<NonZeroU8> = (v1 > 0).then_some(unsafe { std::mem::transmute(v1) });
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
@ -67,7 +100,7 @@ LL | let _: Option<NonZeroU8> = (v1 > 0).then(|| unsafe { std::mem::transmut
|
|||
| ~~~~ ++
|
||||
|
||||
error: this transmute is always evaluated eagerly, even if the condition is false
|
||||
--> $DIR/eager_transmute.rs:63:86
|
||||
--> $DIR/eager_transmute.rs:83:86
|
||||
|
|
||||
LL | let _: Option<NonMaxU8> = (v2 < NonZeroU8::new(255).unwrap()).then_some(unsafe { std::mem::transmute(v2) });
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
@ -78,7 +111,7 @@ LL | let _: Option<NonMaxU8> = (v2 < NonZeroU8::new(255).unwrap()).then(|| u
|
|||
| ~~~~ ++
|
||||
|
||||
error: this transmute is always evaluated eagerly, even if the condition is false
|
||||
--> $DIR/eager_transmute.rs:69:93
|
||||
--> $DIR/eager_transmute.rs:89:93
|
||||
|
|
||||
LL | let _: Option<NonZeroNonMaxU8> = (v2 < NonZeroU8::new(255).unwrap()).then_some(unsafe { std::mem::transmute(v2) });
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
@ -88,5 +121,5 @@ help: consider using `bool::then` to only transmute if the condition holds
|
|||
LL | let _: Option<NonZeroNonMaxU8> = (v2 < NonZeroU8::new(255).unwrap()).then(|| unsafe { std::mem::transmute(v2) });
|
||||
| ~~~~ ++
|
||||
|
||||
error: aborting due to 8 previous errors
|
||||
error: aborting due to 11 previous errors
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue