Skip to content

Commit f5eaa77

Browse files
y21y21
committed
use clippy's const eval (and fix overflow handling)
1 parent 63bee93 commit f5eaa77

File tree

5 files changed

+215
-72
lines changed

5 files changed

+215
-72
lines changed

‎clippy_utils/src/consts.rs

Lines changed: 105 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use rustc_lexer::tokenize;
1111
use rustc_lint::LateContext;
1212
use rustc_middle::mir;
1313
use rustc_middle::mir::interpret::Scalar;
14-
use rustc_middle::ty::{self, EarlyBinder, FloatTy, ScalarInt, Ty, TyCtxt};
14+
use rustc_middle::ty::{self, EarlyBinder, FloatTy, IntTy, ScalarInt, Ty, TyCtxt, UintTy};
1515
use rustc_middle::ty::{List, SubstsRef};
1616
use rustc_middle::{bug, span_bug};
1717
use rustc_span::symbol::{Ident, Symbol};
@@ -52,6 +52,63 @@ pub enum Constant<'tcx> {
5252
Err,
5353
}
5454

55+
trait IntTypeBounds: Sized {
56+
type Output: PartialOrd;
57+
58+
fn min_max(self) -> Option<(Self::Output, Self::Output)>;
59+
fn bits(self) -> Self::Output;
60+
fn ensure_fits(self, val: Self::Output) -> Option<Self::Output> {
61+
let (min, max) = self.min_max()?;
62+
(min <= val && val <= max).then_some(val)
63+
}
64+
}
65+
impl IntTypeBounds for UintTy {
66+
type Output = u128;
67+
fn min_max(self) -> Option<(Self::Output, Self::Output)> {
68+
Some(match self {
69+
UintTy::U8 => (u8::MIN.into(), u8::MAX.into()),
70+
UintTy::U16 => (u16::MIN.into(), u16::MAX.into()),
71+
UintTy::U32 => (u32::MIN.into(), u32::MAX.into()),
72+
UintTy::U64 => (u64::MIN.into(), u64::MAX.into()),
73+
UintTy::U128 => (u128::MIN, u128::MAX),
74+
UintTy::Usize => (usize::MIN.try_into().ok()?, usize::MAX.try_into().ok()?),
75+
})
76+
}
77+
fn bits(self) -> Self::Output {
78+
match self {
79+
UintTy::U8 => 8,
80+
UintTy::U16 => 16,
81+
UintTy::U32 => 32,
82+
UintTy::U64 => 64,
83+
UintTy::U128 => 128,
84+
UintTy::Usize => usize::BITS.try_into().unwrap(),
85+
}
86+
}
87+
}
88+
impl IntTypeBounds for IntTy {
89+
type Output = i128;
90+
fn min_max(self) -> Option<(Self::Output, Self::Output)> {
91+
Some(match self {
92+
IntTy::I8 => (i8::MIN.into(), i8::MAX.into()),
93+
IntTy::I16 => (i16::MIN.into(), i16::MAX.into()),
94+
IntTy::I32 => (i32::MIN.into(), i32::MAX.into()),
95+
IntTy::I64 => (i64::MIN.into(), i64::MAX.into()),
96+
IntTy::I128 => (i128::MIN, i128::MAX),
97+
IntTy::Isize => (isize::MIN.try_into().ok()?, isize::MAX.try_into().ok()?),
98+
})
99+
}
100+
fn bits(self) -> Self::Output {
101+
match self {
102+
IntTy::I8 => 8,
103+
IntTy::I16 => 16,
104+
IntTy::I32 => 32,
105+
IntTy::I64 => 64,
106+
IntTy::I128 => 128,
107+
IntTy::Isize => isize::BITS.try_into().unwrap(),
108+
}
109+
}
110+
}
111+
55112
impl<'tcx> PartialEq for Constant<'tcx> {
56113
fn eq(&self, other: &Self) -> bool {
57114
match (self, other) {
@@ -435,8 +492,15 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> {
435492
match *o {
436493
Int(value) => {
437494
let ty::Int(ity) = *ty.kind() else { return None };
495+
let (min, _) = ity.min_max()?;
438496
// sign extend
439497
let value = sext(self.lcx.tcx, value, ity);
498+
499+
// Applying unary - to the most negative value of any signed integer type panicks.
500+
if value == min {
501+
return None;
502+
}
503+
440504
let value = value.checked_neg()?;
441505
// clear unused bits
442506
Some(Int(unsext(self.lcx.tcx, value, ity)))
@@ -568,17 +632,30 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> {
568632
match (l, r) {
569633
(Constant::Int(l), Some(Constant::Int(r))) => match *self.typeck_results.expr_ty_opt(left)?.kind() {
570634
ty::Int(ity) => {
635+
let (ty_min_value, _) = ity.min_max()?;
636+
let bits = ity.bits();
571637
let l = sext(self.lcx.tcx, l, ity);
572638
let r = sext(self.lcx.tcx, r, ity);
639+
640+
// Using / or %, where the left-hand argument is the smallest integer of a signed integer type and
641+
// the right-hand argument is -1 always panicks, even with overflow-checks disabled
642+
if l == ty_min_value && r == -1 {
643+
return None;
644+
}
645+
573646
let zext = |n: i128| Constant::Int(unsext(self.lcx.tcx, n, ity));
574647
match op.node {
575-
BinOpKind::Add => l.checked_add(r).map(zext),
576-
BinOpKind::Sub => l.checked_sub(r).map(zext),
577-
BinOpKind::Mul => l.checked_mul(r).map(zext),
648+
// When +, * or binary - create a value greater than the maximum value, or less than
649+
// the minimum value that can be stored, it panicks.
650+
BinOpKind::Add => l.checked_add(r).and_then(|n| ity.ensure_fits(n)).map(zext),
651+
BinOpKind::Sub => l.checked_sub(r).and_then(|n| ity.ensure_fits(n)).map(zext),
652+
BinOpKind::Mul => l.checked_mul(r).and_then(|n| ity.ensure_fits(n)).map(zext),
578653
BinOpKind::Div if r != 0 => l.checked_div(r).map(zext),
579654
BinOpKind::Rem if r != 0 => l.checked_rem(r).map(zext),
580-
BinOpKind::Shr => l.checked_shr(r.try_into().ok()?).map(zext),
581-
BinOpKind::Shl => l.checked_shl(r.try_into().ok()?).map(zext),
655+
// Using << or >> where the right-hand argument is greater than or equal to the number of bits
656+
// in the type of the left-hand argument, or is negative panicks.
657+
BinOpKind::Shr if r < bits && !l.is_negative() => l.checked_shr(r.try_into().ok()?).map(zext),
658+
BinOpKind::Shl if r < bits && !l.is_negative() => l.checked_shl(r.try_into().ok()?).map(zext),
582659
BinOpKind::BitXor => Some(zext(l ^ r)),
583660
BinOpKind::BitOr => Some(zext(l | r)),
584661
BinOpKind::BitAnd => Some(zext(l & r)),
@@ -591,24 +668,28 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> {
591668
_ => None,
592669
}
593670
},
594-
ty::Uint(_) => match op.node {
595-
BinOpKind::Add => l.checked_add(r).map(Constant::Int),
596-
BinOpKind::Sub => l.checked_sub(r).map(Constant::Int),
597-
BinOpKind::Mul => l.checked_mul(r).map(Constant::Int),
598-
BinOpKind::Div => l.checked_div(r).map(Constant::Int),
599-
BinOpKind::Rem => l.checked_rem(r).map(Constant::Int),
600-
BinOpKind::Shr => l.checked_shr(r.try_into().ok()?).map(Constant::Int),
601-
BinOpKind::Shl => l.checked_shl(r.try_into().ok()?).map(Constant::Int),
602-
BinOpKind::BitXor => Some(Constant::Int(l ^ r)),
603-
BinOpKind::BitOr => Some(Constant::Int(l | r)),
604-
BinOpKind::BitAnd => Some(Constant::Int(l & r)),
605-
BinOpKind::Eq => Some(Constant::Bool(l == r)),
606-
BinOpKind::Ne => Some(Constant::Bool(l != r)),
607-
BinOpKind::Lt => Some(Constant::Bool(l < r)),
608-
BinOpKind::Le => Some(Constant::Bool(l <= r)),
609-
BinOpKind::Ge => Some(Constant::Bool(l >= r)),
610-
BinOpKind::Gt => Some(Constant::Bool(l > r)),
611-
_ => None,
671+
ty::Uint(ity) => {
672+
let bits = ity.bits();
673+
674+
match op.node {
675+
BinOpKind::Add => l.checked_add(r).and_then(|n| ity.ensure_fits(n)).map(Constant::Int),
676+
BinOpKind::Sub => l.checked_sub(r).and_then(|n| ity.ensure_fits(n)).map(Constant::Int),
677+
BinOpKind::Mul => l.checked_mul(r).and_then(|n| ity.ensure_fits(n)).map(Constant::Int),
678+
BinOpKind::Div => l.checked_div(r).map(Constant::Int),
679+
BinOpKind::Rem => l.checked_rem(r).map(Constant::Int),
680+
BinOpKind::Shr if r < bits => l.checked_shr(r.try_into().ok()?).map(Constant::Int),
681+
BinOpKind::Shl if r < bits => l.checked_shl(r.try_into().ok()?).map(Constant::Int),
682+
BinOpKind::BitXor => Some(Constant::Int(l ^ r)),
683+
BinOpKind::BitOr => Some(Constant::Int(l | r)),
684+
BinOpKind::BitAnd => Some(Constant::Int(l & r)),
685+
BinOpKind::Eq => Some(Constant::Bool(l == r)),
686+
BinOpKind::Ne => Some(Constant::Bool(l != r)),
687+
BinOpKind::Lt => Some(Constant::Bool(l < r)),
688+
BinOpKind::Le => Some(Constant::Bool(l <= r)),
689+
BinOpKind::Ge => Some(Constant::Bool(l >= r)),
690+
BinOpKind::Gt => Some(Constant::Bool(l > r)),
691+
_ => None,
692+
}
612693
},
613694
_ => None,
614695
},

‎clippy_utils/src/eager_or_lazy.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
//! - or-fun-call
1010
//! - option-if-let-else
1111
12+
use crate::consts::constant;
1213
use crate::ty::{all_predicates_of, is_copy};
1314
use crate::visitors::is_const_evaluatable;
1415
use rustc_hir::def::{DefKind, Res};
@@ -190,8 +191,8 @@ fn expr_eagerness<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> EagernessS
190191
}
191192
},
192193

193-
// `-i32::MIN` panics with overflow checks
194-
ExprKind::Unary(UnOp::Neg, expr) if self.cx.typeck_results().expr_ty(expr).is_signed() => {
194+
// `-i32::MIN` panics with overflow checks, but `constant` lets us rule out some simple cases
195+
ExprKind::Unary(UnOp::Neg, _) if constant(self.cx, self.cx.typeck_results(), e).is_none() => {
195196
self.eagerness |= NoChange;
196197
},
197198

@@ -212,6 +213,7 @@ fn expr_eagerness<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> EagernessS
212213

213214
// Arithmetic operations panic on under-/overflow with overflow checks, so don't suggest changing it:
214215
// https://doc.rust-lang.org/stable/reference/expressions/operator-expr.html#overflow
216+
// Using `constant` lets us allow some simple cases where we know for sure it can't overflow
215217
ExprKind::Binary(op, ..)
216218
if matches!(
217219
op.node,
@@ -222,7 +224,7 @@ fn expr_eagerness<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> EagernessS
222224
| BinOpKind::Rem
223225
| BinOpKind::Shl
224226
| BinOpKind::Shr
225-
) =>
227+
) && constant(self.cx, self.cx.typeck_results(), e).is_none() =>
226228
{
227229
self.eagerness |= NoChange;
228230
},

‎tests/ui/unnecessary_lazy_eval.fixed

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
#![allow(clippy::map_identity)]
77
#![allow(clippy::needless_borrow)]
88
#![allow(clippy::unnecessary_literal_unwrap)]
9+
#![allow(arithmetic_overflow)]
10+
#![allow(unconditional_panic)]
911

1012
use std::ops::Deref;
1113

@@ -183,7 +185,7 @@ fn main() {
183185
let _: Result<usize, usize> = res.or_else(|err| Ok(err));
184186

185187
issue9422(3);
186-
issue9422_v2(3);
188+
panicky_arithmetic_ops(3);
187189
}
188190

189191
#[allow(unused)]
@@ -197,6 +199,18 @@ fn issue9422(x: usize) -> Option<usize> {
197199
// (x >= 5).then_some(x - 5) // clippy suggestion panics
198200
}
199201

200-
fn issue9422_v2(x: usize) -> Option<i32> {
201-
(x >= 5).then(|| -i32::MAX) // .then_some(-i32::MAX) always panics
202+
// https://doc.rust-lang.org/stable/reference/expressions/operator-expr.html#overflow
203+
fn panicky_arithmetic_ops(x: usize) {
204+
let _x = false.then(|| i32::MAX + 1); // don't lint
205+
let _x = false.then(|| i32::MAX * 2); // don't lint
206+
let _x = false.then_some(i32::MAX - 1); // lint
207+
#[allow(clippy::identity_op)]
208+
let _x = false.then_some((1 + 2 * 3 - 2 / 3 + 9) << 2); // lint
209+
let _x = false.then_some(255u8 << 7); // lint
210+
let _x = false.then(|| 255u8 << 8); // don't lint
211+
let _x = false.then(|| 255u8 >> 8); // don't lint
212+
let _x = false.then(|| 255u8 >> x); // don't lint
213+
let _x = false.then(|| i32::MIN / -1); // don't lint
214+
let _x = false.then_some(-i32::MAX); // lint
215+
let _x = false.then(|| -i32::MIN); // don't lint
202216
}

‎tests/ui/unnecessary_lazy_eval.rs

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
#![allow(clippy::map_identity)]
77
#![allow(clippy::needless_borrow)]
88
#![allow(clippy::unnecessary_literal_unwrap)]
9+
#![allow(arithmetic_overflow)]
10+
#![allow(unconditional_panic)]
911

1012
use std::ops::Deref;
1113

@@ -183,7 +185,7 @@ fn main() {
183185
let _: Result<usize, usize> = res.or_else(|err| Ok(err));
184186

185187
issue9422(3);
186-
issue9422_v2(3);
188+
panicky_arithmetic_ops(3);
187189
}
188190

189191
#[allow(unused)]
@@ -197,6 +199,18 @@ fn issue9422(x: usize) -> Option<usize> {
197199
// (x >= 5).then_some(x - 5) // clippy suggestion panics
198200
}
199201

200-
fn issue9422_v2(x: usize) -> Option<i32> {
201-
(x >= 5).then(|| -i32::MAX) // .then_some(-i32::MAX) always panics
202+
// https://doc.rust-lang.org/stable/reference/expressions/operator-expr.html#overflow
203+
fn panicky_arithmetic_ops(x: usize) {
204+
let _x = false.then(|| i32::MAX + 1); // don't lint
205+
let _x = false.then(|| i32::MAX * 2); // don't lint
206+
let _x = false.then(|| i32::MAX - 1); // lint
207+
#[allow(clippy::identity_op)]
208+
let _x = false.then(|| (1 + 2 * 3 - 2 / 3 + 9) << 2); // lint
209+
let _x = false.then(|| 255u8 << 7); // lint
210+
let _x = false.then(|| 255u8 << 8); // don't lint
211+
let _x = false.then(|| 255u8 >> 8); // don't lint
212+
let _x = false.then(|| 255u8 >> x); // don't lint
213+
let _x = false.then(|| i32::MIN / -1); // don't lint
214+
let _x = false.then(|| -i32::MAX); // lint
215+
let _x = false.then(|| -i32::MIN); // don't lint
202216
}

0 commit comments

Comments
 (0)