Skip to content

Commit 425e1ea

Browse files
committed
Auto merge of #9587 - c410-f3r:arith, r=Alexendoo
[arithmetic-side-effects] Do not ignore literal references To my utter surprise, `rustc` does does not warn stuff like `let n: u8 = &255 + &1` or `let n: u8 = 255 + &1`. changelog: [arithmetic-side-effects] Do not ignore literal references
2 parents 2f90b2a + a44914f commit 425e1ea

File tree

3 files changed

+104
-74
lines changed

3 files changed

+104
-74
lines changed

clippy_lints/src/operators/arithmetic_side_effects.rs

+30-20
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,3 @@
1-
#![allow(
2-
// False positive
3-
clippy::match_same_arms
4-
)]
5-
61
use super::ARITHMETIC_SIDE_EFFECTS;
72
use clippy_utils::{consts::constant_simple, diagnostics::span_lint};
83
use rustc_ast as ast;
@@ -14,12 +9,12 @@ use rustc_session::impl_lint_pass;
149
use rustc_span::source_map::{Span, Spanned};
1510

1611
const HARD_CODED_ALLOWED: &[&str] = &[
12+
"&str",
1713
"f32",
1814
"f64",
1915
"std::num::Saturating",
2016
"std::num::Wrapping",
2117
"std::string::String",
22-
"&str",
2318
];
2419

2520
#[derive(Debug)]
@@ -50,10 +45,10 @@ impl ArithmeticSideEffects {
5045
let ast::LitKind::Int(value, _) = lit.node
5146
{
5247
match (&op.node, value) {
53-
(hir::BinOpKind::Add | hir::BinOpKind::Sub, 0) |
54-
(hir::BinOpKind::Mul, 0 | 1) => true,
5548
(hir::BinOpKind::Div | hir::BinOpKind::Rem, 0) => false,
56-
(hir::BinOpKind::Div | hir::BinOpKind::Rem, _) => true,
49+
(hir::BinOpKind::Add | hir::BinOpKind::Sub, 0)
50+
| (hir::BinOpKind::Div | hir::BinOpKind::Rem, _)
51+
| (hir::BinOpKind::Mul, 0 | 1) => true,
5752
_ => false,
5853
}
5954
} else {
@@ -79,16 +74,13 @@ impl ArithmeticSideEffects {
7974
self.expr_span = Some(expr.span);
8075
}
8176

82-
/// * If `expr` is a literal integer like `1` or `i32::MAX`, returns itself.
83-
/// * Is `expr` is a literal integer reference like `&199`, returns the literal integer without
84-
/// references.
85-
/// * If `expr` is anything else, returns `None`.
86-
fn literal_integer<'expr, 'tcx>(expr: &'expr hir::Expr<'tcx>) -> Option<&'expr hir::Expr<'tcx>> {
77+
/// If `expr` does not match any variant of `LiteralIntegerTy`, returns `None`.
78+
fn literal_integer<'expr, 'tcx>(expr: &'expr hir::Expr<'tcx>) -> Option<LiteralIntegerTy<'expr, 'tcx>> {
8779
if matches!(expr.kind, hir::ExprKind::Lit(_)) {
88-
return Some(expr);
80+
return Some(LiteralIntegerTy::Value(expr));
8981
}
9082
if let hir::ExprKind::AddrOf(.., inn) = expr.kind && let hir::ExprKind::Lit(_) = inn.kind {
91-
return Some(inn)
83+
return Some(LiteralIntegerTy::Ref(inn));
9284
}
9385
None
9486
}
@@ -126,10 +118,9 @@ impl ArithmeticSideEffects {
126118
}
127119
let has_valid_op = if Self::is_integral(lhs_ty) && Self::is_integral(rhs_ty) {
128120
match (Self::literal_integer(lhs), Self::literal_integer(rhs)) {
129-
(None, None) => false,
130-
(None, Some(local_expr)) => Self::has_valid_op(op, local_expr),
131-
(Some(local_expr), None) => Self::has_valid_op(op, local_expr),
132-
(Some(_), Some(_)) => true,
121+
(None, Some(lit_int_ty)) | (Some(lit_int_ty), None) => Self::has_valid_op(op, lit_int_ty.into()),
122+
(Some(LiteralIntegerTy::Value(_)), Some(LiteralIntegerTy::Value(_))) => true,
123+
(None, None) | (Some(_), Some(_)) => false,
133124
}
134125
} else {
135126
false
@@ -186,3 +177,22 @@ impl<'tcx> LateLintPass<'tcx> for ArithmeticSideEffects {
186177
}
187178
}
188179
}
180+
181+
/// Tells if an expression is a integer declared by value or by reference.
182+
///
183+
/// If `LiteralIntegerTy::Ref`, then the contained value will be `hir::ExprKind::Lit` rather
184+
/// than `hirExprKind::Addr`.
185+
enum LiteralIntegerTy<'expr, 'tcx> {
186+
/// For example, `&199`
187+
Ref(&'expr hir::Expr<'tcx>),
188+
/// For example, `1` or `i32::MAX`
189+
Value(&'expr hir::Expr<'tcx>),
190+
}
191+
192+
impl<'expr, 'tcx> From<LiteralIntegerTy<'expr, 'tcx>> for &'expr hir::Expr<'tcx> {
193+
fn from(from: LiteralIntegerTy<'expr, 'tcx>) -> Self {
194+
match from {
195+
LiteralIntegerTy::Ref(elem) | LiteralIntegerTy::Value(elem) => elem,
196+
}
197+
}
198+
}

tests/ui/arithmetic_side_effects.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,6 @@ pub fn non_overflowing_ops_or_ops_already_handled_by_the_compiler_should_not_tri
148148
_n = 1 * _n;
149149
_n = &1 * _n;
150150
_n = 23 + 85;
151-
_n = &23 + &85;
152151

153152
// Unary
154153
_n = -1;
@@ -187,6 +186,9 @@ pub fn runtime_ops() {
187186
_n = _n * &2;
188187
_n = 2 * _n;
189188
_n = &2 * _n;
189+
_n = 23 + &85;
190+
_n = &23 + 85;
191+
_n = &23 + &85;
190192

191193
// Custom
192194
let _ = Custom + 0;

0 commit comments

Comments
 (0)