Skip to content

Commit 7696c9d

Browse files
y21y21
committed
allow more div and rem operations that can be checked
1 parent 1e0597c commit 7696c9d

File tree

4 files changed

+124
-45
lines changed

4 files changed

+124
-45
lines changed

‎clippy_utils/src/eager_or_lazy.rs‎

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
//! - or-fun-call
1010
//! - option-if-let-else
1111
12-
use crate::consts::constant;
12+
use crate::consts::{constant, FullInt};
1313
use crate::ty::{all_predicates_of, is_copy};
1414
use crate::visitors::is_const_evaluatable;
1515
use rustc_hir::def::{DefKind, Res};
@@ -227,15 +227,34 @@ fn expr_eagerness<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> EagernessS
227227
self.eagerness |= NoChange;
228228
},
229229

230-
// Similar to `>>` and `<<`, we only want to avoid linting entirely if both sides are unknown and the
230+
ExprKind::Binary(op, left, right)
231+
if matches!(op.node, BinOpKind::Div | BinOpKind::Rem)
232+
&& let left_ty = self.cx.typeck_results().expr_ty(left)
233+
&& let right_ty = self.cx.typeck_results().expr_ty(right)
234+
&& let left = constant(self.cx, self.cx.typeck_results(), left)
235+
.and_then(|c| c.int_value(self.cx, left_ty))
236+
&& let right = constant(self.cx, self.cx.typeck_results(), right)
237+
.and_then(|c| c.int_value(self.cx, right_ty))
238+
&& match (left, right) {
239+
// `1 / x` -- x might be zero
240+
(_, None) => true,
241+
// `x / -1` -- x might be T::MIN = panic
242+
#[expect(clippy::match_same_arms)]
243+
(None, Some(FullInt::S(-1))) => true,
244+
// anything else is either fine or checked by the compiler
245+
_ => false,
246+
} =>
247+
{
248+
self.eagerness |= NoChange;
249+
},
250+
251+
// Similar to `>>` and `<<`, we only want to avoid linting entirely if either side is unknown and the
231252
// compiler can't emit an error for an overflowing expression.
232253
// Suggesting eagerness for `true.then(|| i32::MAX + 1)` is okay because the compiler will emit an
233254
// error and it's good to have the eagerness warning up front when the user fixes the logic error.
234255
ExprKind::Binary(op, left, right)
235-
if matches!(
236-
op.node,
237-
BinOpKind::Add | BinOpKind::Sub | BinOpKind::Mul | BinOpKind::Div | BinOpKind::Rem
238-
) && !self.cx.typeck_results().expr_ty(e).is_floating_point()
256+
if matches!(op.node, BinOpKind::Add | BinOpKind::Sub | BinOpKind::Mul)
257+
&& !self.cx.typeck_results().expr_ty(e).is_floating_point()
239258
&& (constant(self.cx, self.cx.typeck_results(), left).is_none()
240259
|| constant(self.cx, self.cx.typeck_results(), right).is_none()) =>
241260
{

‎tests/ui/unnecessary_lazy_eval.fixed‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -201,11 +201,7 @@ fn issue9422(x: usize) -> Option<usize> {
201201
fn panicky_arithmetic_ops(x: usize, y: isize) {
202202
#![allow(clippy::identity_op, clippy::eq_op)]
203203

204-
// Even though some of these expressions overflow, they're entirely dependent on constants.
205-
// So, the compiler already emits a warning about overflowing expressions.
206-
// It's a logic error and we want both warnings up front.
207-
// ONLY when a binop side that "matters" for overflow (for `>>`, that is always the right side and
208-
// never the left side) has a non-constant value, avoid linting
204+
// See comments in `eager_or_lazy.rs` for the rules that this is meant to follow
209205

210206
let _x = false.then_some(i32::MAX + 1);
211207
//~^ ERROR: unnecessary closure used with `bool::then`
@@ -224,8 +220,6 @@ fn panicky_arithmetic_ops(x: usize, y: isize) {
224220
let _x = false.then_some(255u8 >> 8);
225221
//~^ ERROR: unnecessary closure used with `bool::then`
226222
let _x = false.then(|| 255u8 >> x);
227-
let _x = false.then_some(i32::MIN / -1);
228-
//~^ ERROR: unnecessary closure used with `bool::then`
229223
let _x = false.then_some(i32::MAX + -1);
230224
//~^ ERROR: unnecessary closure used with `bool::then`
231225
let _x = false.then_some(-i32::MAX);
@@ -251,6 +245,22 @@ fn panicky_arithmetic_ops(x: usize, y: isize) {
251245
let _x = false.then(|| x + 1);
252246
let _x = false.then(|| 1 + x);
253247

248+
let _x = false.then_some(x / 0);
249+
//~^ ERROR: unnecessary closure used with `bool::then`
250+
let _x = false.then_some(x % 0);
251+
//~^ ERROR: unnecessary closure used with `bool::then`
252+
let _x = false.then(|| y / -1);
253+
let _x = false.then_some(1 / -1);
254+
//~^ ERROR: unnecessary closure used with `bool::then`
255+
let _x = false.then_some(i32::MIN / -1);
256+
//~^ ERROR: unnecessary closure used with `bool::then`
257+
let _x = false.then(|| i32::MIN / x as i32);
258+
let _x = false.then_some(i32::MIN / 0);
259+
//~^ ERROR: unnecessary closure used with `bool::then`
260+
let _x = false.then_some(4 / 2);
261+
//~^ ERROR: unnecessary closure used with `bool::then`
262+
let _x = false.then(|| 1 / x);
263+
254264
// const eval doesn't read variables, but floating point math never panics, so we can still emit a
255265
// warning
256266
let f1 = 1.0;

‎tests/ui/unnecessary_lazy_eval.rs‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -201,11 +201,7 @@ fn issue9422(x: usize) -> Option<usize> {
201201
fn panicky_arithmetic_ops(x: usize, y: isize) {
202202
#![allow(clippy::identity_op, clippy::eq_op)]
203203

204-
// Even though some of these expressions overflow, they're entirely dependent on constants.
205-
// So, the compiler already emits a warning about overflowing expressions.
206-
// It's a logic error and we want both warnings up front.
207-
// ONLY when a binop side that "matters" for overflow (for `>>`, that is always the right side and
208-
// never the left side) has a non-constant value, avoid linting
204+
// See comments in `eager_or_lazy.rs` for the rules that this is meant to follow
209205

210206
let _x = false.then(|| i32::MAX + 1);
211207
//~^ ERROR: unnecessary closure used with `bool::then`
@@ -224,8 +220,6 @@ fn panicky_arithmetic_ops(x: usize, y: isize) {
224220
let _x = false.then(|| 255u8 >> 8);
225221
//~^ ERROR: unnecessary closure used with `bool::then`
226222
let _x = false.then(|| 255u8 >> x);
227-
let _x = false.then(|| i32::MIN / -1);
228-
//~^ ERROR: unnecessary closure used with `bool::then`
229223
let _x = false.then(|| i32::MAX + -1);
230224
//~^ ERROR: unnecessary closure used with `bool::then`
231225
let _x = false.then(|| -i32::MAX);
@@ -251,6 +245,22 @@ fn panicky_arithmetic_ops(x: usize, y: isize) {
251245
let _x = false.then(|| x + 1);
252246
let _x = false.then(|| 1 + x);
253247

248+
let _x = false.then(|| x / 0);
249+
//~^ ERROR: unnecessary closure used with `bool::then`
250+
let _x = false.then(|| x % 0);
251+
//~^ ERROR: unnecessary closure used with `bool::then`
252+
let _x = false.then(|| y / -1);
253+
let _x = false.then(|| 1 / -1);
254+
//~^ ERROR: unnecessary closure used with `bool::then`
255+
let _x = false.then(|| i32::MIN / -1);
256+
//~^ ERROR: unnecessary closure used with `bool::then`
257+
let _x = false.then(|| i32::MIN / x as i32);
258+
let _x = false.then(|| i32::MIN / 0);
259+
//~^ ERROR: unnecessary closure used with `bool::then`
260+
let _x = false.then(|| 4 / 2);
261+
//~^ ERROR: unnecessary closure used with `bool::then`
262+
let _x = false.then(|| 1 / x);
263+
254264
// const eval doesn't read variables, but floating point math never panics, so we can still emit a
255265
// warning
256266
let f1 = 1.0;

‎tests/ui/unnecessary_lazy_eval.stderr‎

Lines changed: 65 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -329,148 +329,188 @@ LL | | or_else(|_| Ok(ext_str.some_field));
329329
| help: use `or(..)` instead: `or(Ok(ext_str.some_field))`
330330

331331
error: unnecessary closure used with `bool::then`
332-
--> $DIR/unnecessary_lazy_eval.rs:210:14
332+
--> $DIR/unnecessary_lazy_eval.rs:206:14
333333
|
334334
LL | let _x = false.then(|| i32::MAX + 1);
335335
| ^^^^^^---------------------
336336
| |
337337
| help: use `then_some(..)` instead: `then_some(i32::MAX + 1)`
338338

339339
error: unnecessary closure used with `bool::then`
340-
--> $DIR/unnecessary_lazy_eval.rs:212:14
340+
--> $DIR/unnecessary_lazy_eval.rs:208:14
341341
|
342342
LL | let _x = false.then(|| i32::MAX * 2);
343343
| ^^^^^^---------------------
344344
| |
345345
| help: use `then_some(..)` instead: `then_some(i32::MAX * 2)`
346346

347347
error: unnecessary closure used with `bool::then`
348-
--> $DIR/unnecessary_lazy_eval.rs:214:14
348+
--> $DIR/unnecessary_lazy_eval.rs:210:14
349349
|
350350
LL | let _x = false.then(|| i32::MAX - 1);
351351
| ^^^^^^---------------------
352352
| |
353353
| help: use `then_some(..)` instead: `then_some(i32::MAX - 1)`
354354

355355
error: unnecessary closure used with `bool::then`
356-
--> $DIR/unnecessary_lazy_eval.rs:216:14
356+
--> $DIR/unnecessary_lazy_eval.rs:212:14
357357
|
358358
LL | let _x = false.then(|| i32::MIN - 1);
359359
| ^^^^^^---------------------
360360
| |
361361
| help: use `then_some(..)` instead: `then_some(i32::MIN - 1)`
362362

363363
error: unnecessary closure used with `bool::then`
364-
--> $DIR/unnecessary_lazy_eval.rs:218:14
364+
--> $DIR/unnecessary_lazy_eval.rs:214:14
365365
|
366366
LL | let _x = false.then(|| (1 + 2 * 3 - 2 / 3 + 9) << 2);
367367
| ^^^^^^-------------------------------------
368368
| |
369369
| help: use `then_some(..)` instead: `then_some((1 + 2 * 3 - 2 / 3 + 9) << 2)`
370370

371371
error: unnecessary closure used with `bool::then`
372-
--> $DIR/unnecessary_lazy_eval.rs:220:14
372+
--> $DIR/unnecessary_lazy_eval.rs:216:14
373373
|
374374
LL | let _x = false.then(|| 255u8 << 7);
375375
| ^^^^^^-------------------
376376
| |
377377
| help: use `then_some(..)` instead: `then_some(255u8 << 7)`
378378

379379
error: unnecessary closure used with `bool::then`
380-
--> $DIR/unnecessary_lazy_eval.rs:222:14
380+
--> $DIR/unnecessary_lazy_eval.rs:218:14
381381
|
382382
LL | let _x = false.then(|| 255u8 << 8);
383383
| ^^^^^^-------------------
384384
| |
385385
| help: use `then_some(..)` instead: `then_some(255u8 << 8)`
386386

387387
error: unnecessary closure used with `bool::then`
388-
--> $DIR/unnecessary_lazy_eval.rs:224:14
388+
--> $DIR/unnecessary_lazy_eval.rs:220:14
389389
|
390390
LL | let _x = false.then(|| 255u8 >> 8);
391391
| ^^^^^^-------------------
392392
| |
393393
| help: use `then_some(..)` instead: `then_some(255u8 >> 8)`
394394

395395
error: unnecessary closure used with `bool::then`
396-
--> $DIR/unnecessary_lazy_eval.rs:227:14
397-
|
398-
LL | let _x = false.then(|| i32::MIN / -1);
399-
| ^^^^^^----------------------
400-
| |
401-
| help: use `then_some(..)` instead: `then_some(i32::MIN / -1)`
402-
403-
error: unnecessary closure used with `bool::then`
404-
--> $DIR/unnecessary_lazy_eval.rs:229:14
396+
--> $DIR/unnecessary_lazy_eval.rs:223:14
405397
|
406398
LL | let _x = false.then(|| i32::MAX + -1);
407399
| ^^^^^^----------------------
408400
| |
409401
| help: use `then_some(..)` instead: `then_some(i32::MAX + -1)`
410402

411403
error: unnecessary closure used with `bool::then`
412-
--> $DIR/unnecessary_lazy_eval.rs:231:14
404+
--> $DIR/unnecessary_lazy_eval.rs:225:14
413405
|
414406
LL | let _x = false.then(|| -i32::MAX);
415407
| ^^^^^^------------------
416408
| |
417409
| help: use `then_some(..)` instead: `then_some(-i32::MAX)`
418410

419411
error: unnecessary closure used with `bool::then`
420-
--> $DIR/unnecessary_lazy_eval.rs:233:14
412+
--> $DIR/unnecessary_lazy_eval.rs:227:14
421413
|
422414
LL | let _x = false.then(|| -i32::MIN);
423415
| ^^^^^^------------------
424416
| |
425417
| help: use `then_some(..)` instead: `then_some(-i32::MIN)`
426418

427419
error: unnecessary closure used with `bool::then`
428-
--> $DIR/unnecessary_lazy_eval.rs:236:14
420+
--> $DIR/unnecessary_lazy_eval.rs:230:14
429421
|
430422
LL | let _x = false.then(|| 255 >> -7);
431423
| ^^^^^^------------------
432424
| |
433425
| help: use `then_some(..)` instead: `then_some(255 >> -7)`
434426

435427
error: unnecessary closure used with `bool::then`
436-
--> $DIR/unnecessary_lazy_eval.rs:238:14
428+
--> $DIR/unnecessary_lazy_eval.rs:232:14
437429
|
438430
LL | let _x = false.then(|| 255 << -1);
439431
| ^^^^^^------------------
440432
| |
441433
| help: use `then_some(..)` instead: `then_some(255 << -1)`
442434

443435
error: unnecessary closure used with `bool::then`
444-
--> $DIR/unnecessary_lazy_eval.rs:240:14
436+
--> $DIR/unnecessary_lazy_eval.rs:234:14
445437
|
446438
LL | let _x = false.then(|| 1 / 0);
447439
| ^^^^^^--------------
448440
| |
449441
| help: use `then_some(..)` instead: `then_some(1 / 0)`
450442

451443
error: unnecessary closure used with `bool::then`
452-
--> $DIR/unnecessary_lazy_eval.rs:242:14
444+
--> $DIR/unnecessary_lazy_eval.rs:236:14
453445
|
454446
LL | let _x = false.then(|| x << -1);
455447
| ^^^^^^----------------
456448
| |
457449
| help: use `then_some(..)` instead: `then_some(x << -1)`
458450

459451
error: unnecessary closure used with `bool::then`
460-
--> $DIR/unnecessary_lazy_eval.rs:244:14
452+
--> $DIR/unnecessary_lazy_eval.rs:238:14
461453
|
462454
LL | let _x = false.then(|| x << 2);
463455
| ^^^^^^---------------
464456
| |
465457
| help: use `then_some(..)` instead: `then_some(x << 2)`
466458

459+
error: unnecessary closure used with `bool::then`
460+
--> $DIR/unnecessary_lazy_eval.rs:248:14
461+
|
462+
LL | let _x = false.then(|| x / 0);
463+
| ^^^^^^--------------
464+
| |
465+
| help: use `then_some(..)` instead: `then_some(x / 0)`
466+
467+
error: unnecessary closure used with `bool::then`
468+
--> $DIR/unnecessary_lazy_eval.rs:250:14
469+
|
470+
LL | let _x = false.then(|| x % 0);
471+
| ^^^^^^--------------
472+
| |
473+
| help: use `then_some(..)` instead: `then_some(x % 0)`
474+
475+
error: unnecessary closure used with `bool::then`
476+
--> $DIR/unnecessary_lazy_eval.rs:253:14
477+
|
478+
LL | let _x = false.then(|| 1 / -1);
479+
| ^^^^^^---------------
480+
| |
481+
| help: use `then_some(..)` instead: `then_some(1 / -1)`
482+
483+
error: unnecessary closure used with `bool::then`
484+
--> $DIR/unnecessary_lazy_eval.rs:255:14
485+
|
486+
LL | let _x = false.then(|| i32::MIN / -1);
487+
| ^^^^^^----------------------
488+
| |
489+
| help: use `then_some(..)` instead: `then_some(i32::MIN / -1)`
490+
467491
error: unnecessary closure used with `bool::then`
468492
--> $DIR/unnecessary_lazy_eval.rs:258:14
469493
|
494+
LL | let _x = false.then(|| i32::MIN / 0);
495+
| ^^^^^^---------------------
496+
| |
497+
| help: use `then_some(..)` instead: `then_some(i32::MIN / 0)`
498+
499+
error: unnecessary closure used with `bool::then`
500+
--> $DIR/unnecessary_lazy_eval.rs:260:14
501+
|
502+
LL | let _x = false.then(|| 4 / 2);
503+
| ^^^^^^--------------
504+
| |
505+
| help: use `then_some(..)` instead: `then_some(4 / 2)`
506+
507+
error: unnecessary closure used with `bool::then`
508+
--> $DIR/unnecessary_lazy_eval.rs:268:14
509+
|
470510
LL | let _x = false.then(|| f1 + f2);
471511
| ^^^^^^----------------
472512
| |
473513
| help: use `then_some(..)` instead: `then_some(f1 + f2)`
474514

475-
error: aborting due to 58 previous errors
515+
error: aborting due to 63 previous errors
476516

0 commit comments

Comments
 (0)