Skip to content

Commit 63bee93

Browse files
committed
teach eager_or_lazy that arithmetic ops can panic
1 parent 8fd021f commit 63bee93

File tree

3 files changed

+49
-0
lines changed

3 files changed

+49
-0
lines changed

clippy_utils/src/eager_or_lazy.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ use crate::ty::{all_predicates_of, is_copy};
1313
use crate::visitors::is_const_evaluatable;
1414
use rustc_hir::def::{DefKind, Res};
1515
use rustc_hir::intravisit::{walk_expr, Visitor};
16+
use rustc_hir::BinOpKind;
1617
use rustc_hir::{def_id::DefId, Block, Expr, ExprKind, QPath, UnOp};
1718
use rustc_lint::LateContext;
1819
use rustc_middle::ty::adjustment::Adjust;
@@ -188,6 +189,12 @@ fn expr_eagerness<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> EagernessS
188189
self.eagerness = Lazy;
189190
}
190191
},
192+
193+
// `-i32::MIN` panics with overflow checks
194+
ExprKind::Unary(UnOp::Neg, expr) if self.cx.typeck_results().expr_ty(expr).is_signed() => {
195+
self.eagerness |= NoChange;
196+
},
197+
191198
// Custom `Deref` impl might have side effects
192199
ExprKind::Unary(UnOp::Deref, e)
193200
if self.cx.typeck_results().expr_ty(e).builtin_deref(true).is_none() =>
@@ -202,6 +209,24 @@ fn expr_eagerness<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> EagernessS
202209
self.cx.typeck_results().expr_ty(e).kind(),
203210
ty::Bool | ty::Int(_) | ty::Uint(_),
204211
) => {},
212+
213+
// Arithmetic operations panic on under-/overflow with overflow checks, so don't suggest changing it:
214+
// https://doc.rust-lang.org/stable/reference/expressions/operator-expr.html#overflow
215+
ExprKind::Binary(op, ..)
216+
if matches!(
217+
op.node,
218+
BinOpKind::Add
219+
| BinOpKind::Mul
220+
| BinOpKind::Sub
221+
| BinOpKind::Div
222+
| BinOpKind::Rem
223+
| BinOpKind::Shl
224+
| BinOpKind::Shr
225+
) =>
226+
{
227+
self.eagerness |= NoChange;
228+
},
229+
205230
ExprKind::Binary(_, lhs, rhs)
206231
if self.cx.typeck_results().expr_ty(lhs).is_primitive()
207232
&& self.cx.typeck_results().expr_ty(rhs).is_primitive() => {},

tests/ui/unnecessary_lazy_eval.fixed

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,10 +181,22 @@ fn main() {
181181
// neither bind_instead_of_map nor unnecessary_lazy_eval applies here
182182
let _: Result<usize, usize> = res.and_then(|x| Err(x));
183183
let _: Result<usize, usize> = res.or_else(|err| Ok(err));
184+
185+
issue9422(3);
186+
issue9422_v2(3);
184187
}
185188

186189
#[allow(unused)]
187190
fn issue9485() {
188191
// should not lint, is in proc macro
189192
with_span!(span Some(42).unwrap_or_else(|| 2););
190193
}
194+
195+
fn issue9422(x: usize) -> Option<usize> {
196+
(x >= 5).then(|| x - 5)
197+
// (x >= 5).then_some(x - 5) // clippy suggestion panics
198+
}
199+
200+
fn issue9422_v2(x: usize) -> Option<i32> {
201+
(x >= 5).then(|| -i32::MAX) // .then_some(-i32::MAX) always panics
202+
}

tests/ui/unnecessary_lazy_eval.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,10 +181,22 @@ fn main() {
181181
// neither bind_instead_of_map nor unnecessary_lazy_eval applies here
182182
let _: Result<usize, usize> = res.and_then(|x| Err(x));
183183
let _: Result<usize, usize> = res.or_else(|err| Ok(err));
184+
185+
issue9422(3);
186+
issue9422_v2(3);
184187
}
185188

186189
#[allow(unused)]
187190
fn issue9485() {
188191
// should not lint, is in proc macro
189192
with_span!(span Some(42).unwrap_or_else(|| 2););
190193
}
194+
195+
fn issue9422(x: usize) -> Option<usize> {
196+
(x >= 5).then(|| x - 5)
197+
// (x >= 5).then_some(x - 5) // clippy suggestion panics
198+
}
199+
200+
fn issue9422_v2(x: usize) -> Option<i32> {
201+
(x >= 5).then(|| -i32::MAX) // .then_some(-i32::MAX) always panics
202+
}

0 commit comments

Comments
 (0)