Skip to content

Commit b5e701e

Browse files
authored
arithmetic_side_effects: don't warn on NonZeroU*.get() - 1 (#15238)
changelog: [`arithmetic_side_effects`]: don't warn on `NonZeroU*.get() - 1` fixes #15225
2 parents df5a0ee + a745e2c commit b5e701e

File tree

3 files changed

+43
-26
lines changed

3 files changed

+43
-26
lines changed

clippy_lints/src/operators/arithmetic_side_effects.rs

Lines changed: 27 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,11 @@ use clippy_config::Conf;
33
use clippy_utils::consts::{ConstEvalCtxt, Constant};
44
use clippy_utils::diagnostics::span_lint;
55
use clippy_utils::ty::is_type_diagnostic_item;
6-
use clippy_utils::{expr_or_init, is_from_proc_macro, is_lint_allowed, peel_hir_expr_refs, peel_hir_expr_unary};
6+
use clippy_utils::{expr_or_init, is_from_proc_macro, is_lint_allowed, peel_hir_expr_refs, peel_hir_expr_unary, sym};
77
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
88
use rustc_lint::{LateContext, LateLintPass};
99
use rustc_middle::ty::{self, Ty};
1010
use rustc_session::impl_lint_pass;
11-
use rustc_span::symbol::sym;
1211
use rustc_span::{Span, Symbol};
1312
use {rustc_ast as ast, rustc_hir as hir};
1413

@@ -89,6 +88,18 @@ impl ArithmeticSideEffects {
8988
self.allowed_unary.contains(ty_string_elem)
9089
}
9190

91+
fn is_non_zero_u(cx: &LateContext<'_>, ty: Ty<'_>) -> bool {
92+
if let ty::Adt(adt, substs) = ty.kind()
93+
&& cx.tcx.is_diagnostic_item(sym::NonZero, adt.did())
94+
&& let int_type = substs.type_at(0)
95+
&& matches!(int_type.kind(), ty::Uint(_))
96+
{
97+
true
98+
} else {
99+
false
100+
}
101+
}
102+
92103
/// Verifies built-in types that have specific allowed operations
93104
fn has_specific_allowed_type_and_operation<'tcx>(
94105
cx: &LateContext<'tcx>,
@@ -97,33 +108,12 @@ impl ArithmeticSideEffects {
97108
rhs_ty: Ty<'tcx>,
98109
) -> bool {
99110
let is_div_or_rem = matches!(op, hir::BinOpKind::Div | hir::BinOpKind::Rem);
100-
let is_non_zero_u = |cx: &LateContext<'tcx>, ty: Ty<'tcx>| {
101-
let tcx = cx.tcx;
102-
103-
let ty::Adt(adt, substs) = ty.kind() else { return false };
104-
105-
if !tcx.is_diagnostic_item(sym::NonZero, adt.did()) {
106-
return false;
107-
}
108-
109-
let int_type = substs.type_at(0);
110-
let unsigned_int_types = [
111-
tcx.types.u8,
112-
tcx.types.u16,
113-
tcx.types.u32,
114-
tcx.types.u64,
115-
tcx.types.u128,
116-
tcx.types.usize,
117-
];
118-
119-
unsigned_int_types.contains(&int_type)
120-
};
121111
let is_sat_or_wrap = |ty: Ty<'_>| {
122112
is_type_diagnostic_item(cx, ty, sym::Saturating) || is_type_diagnostic_item(cx, ty, sym::Wrapping)
123113
};
124114

125115
// If the RHS is `NonZero<u*>`, then division or module by zero will never occur.
126-
if is_non_zero_u(cx, rhs_ty) && is_div_or_rem {
116+
if Self::is_non_zero_u(cx, rhs_ty) && is_div_or_rem {
127117
return true;
128118
}
129119

@@ -219,6 +209,18 @@ impl ArithmeticSideEffects {
219209
let (mut actual_rhs, rhs_ref_counter) = peel_hir_expr_refs(rhs);
220210
actual_lhs = expr_or_init(cx, actual_lhs);
221211
actual_rhs = expr_or_init(cx, actual_rhs);
212+
213+
// `NonZeroU*.get() - 1`, will never overflow
214+
if let hir::BinOpKind::Sub = op
215+
&& let hir::ExprKind::MethodCall(method, receiver, [], _) = actual_lhs.kind
216+
&& method.ident.name == sym::get
217+
&& let receiver_ty = cx.typeck_results().expr_ty(receiver).peel_refs()
218+
&& Self::is_non_zero_u(cx, receiver_ty)
219+
&& let Some(1) = Self::literal_integer(cx, actual_rhs)
220+
{
221+
return;
222+
}
223+
222224
let lhs_ty = cx.typeck_results().expr_ty(actual_lhs).peel_refs();
223225
let rhs_ty = cx.typeck_results().expr_ty_adjusted(actual_rhs).peel_refs();
224226
if self.has_allowed_binary(lhs_ty, rhs_ty) {
@@ -227,6 +229,7 @@ impl ArithmeticSideEffects {
227229
if Self::has_specific_allowed_type_and_operation(cx, lhs_ty, op, rhs_ty) {
228230
return;
229231
}
232+
230233
let has_valid_op = if Self::is_integral(lhs_ty) && Self::is_integral(rhs_ty) {
231234
if let hir::BinOpKind::Shl | hir::BinOpKind::Shr = op {
232235
// At least for integers, shifts are already handled by the CTFE

tests/ui/arithmetic_side_effects.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -664,6 +664,20 @@ pub fn issue_12318() {
664664
//~^ arithmetic_side_effects
665665
}
666666

667+
pub fn issue_15225() {
668+
use core::num::{NonZero, NonZeroU8};
669+
670+
let one = const { NonZeroU8::new(1).unwrap() };
671+
let _ = one.get() - 1;
672+
673+
let one: NonZero<u8> = const { NonZero::new(1).unwrap() };
674+
let _ = one.get() - 1;
675+
676+
type AliasedType = u8;
677+
let one: NonZero<AliasedType> = const { NonZero::new(1).unwrap() };
678+
let _ = one.get() - 1;
679+
}
680+
667681
pub fn explicit_methods() {
668682
use core::ops::Add;
669683
let one: i32 = 1;

tests/ui/arithmetic_side_effects.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -758,13 +758,13 @@ LL | one.sub_assign(1);
758758
| ^^^^^^^^^^^^^^^^^
759759

760760
error: arithmetic operation that can potentially result in unexpected side-effects
761-
--> tests/ui/arithmetic_side_effects.rs:670:5
761+
--> tests/ui/arithmetic_side_effects.rs:684:5
762762
|
763763
LL | one.add(&one);
764764
| ^^^^^^^^^^^^^
765765

766766
error: arithmetic operation that can potentially result in unexpected side-effects
767-
--> tests/ui/arithmetic_side_effects.rs:672:5
767+
--> tests/ui/arithmetic_side_effects.rs:686:5
768768
|
769769
LL | Box::new(one).add(one);
770770
| ^^^^^^^^^^^^^^^^^^^^^^

0 commit comments

Comments
 (0)