Skip to content

Commit 2a6a98f

Browse files
committed
[arithmetic-side-effects] Do not ignore literal references
1 parent e8c1b54 commit 2a6a98f

File tree

3 files changed

+103
-70
lines changed

3 files changed

+103
-70
lines changed

clippy_lints/src/operators/arithmetic_side_effects.rs

Lines changed: 29 additions & 16 deletions
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)]
@@ -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
}
@@ -127,9 +119,10 @@ impl ArithmeticSideEffects {
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)) {
129121
(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,
122+
(None, Some(lit_int_ty)) => Self::has_valid_op(op, lit_int_ty.into()),
123+
(Some(lit_int_ty), None) => Self::has_valid_op(op, lit_int_ty.into()),
124+
(Some(LiteralIntegerTy::Value(_)), Some(LiteralIntegerTy::Value(_))) => true,
125+
(Some(_), Some(_)) => false,
133126
}
134127
} else {
135128
false
@@ -186,3 +179,23 @@ impl<'tcx> LateLintPass<'tcx> for ArithmeticSideEffects {
186179
}
187180
}
188181
}
182+
183+
/// Tells if an expression is a integer passed by value or by reference.
184+
///
185+
/// If [LiteralIntegerTy::Ref], then the contained value will be `hir::ExprKind::Lit` rather
186+
/// than `hirExprKind::Addr`.
187+
enum LiteralIntegerTy<'expr, 'tcx> {
188+
/// For example, `&199`
189+
Ref(&'expr hir::Expr<'tcx>),
190+
/// For example, `1` or `i32::MAX`
191+
Value(&'expr hir::Expr<'tcx>),
192+
}
193+
194+
impl<'expr, 'tcx> From<LiteralIntegerTy<'expr, 'tcx>> for &'expr hir::Expr<'tcx> {
195+
fn from(from: LiteralIntegerTy<'expr, 'tcx>) -> Self {
196+
match from {
197+
LiteralIntegerTy::Ref(elem) => elem,
198+
LiteralIntegerTy::Value(elem) => elem,
199+
}
200+
}
201+
}

tests/ui/arithmetic_side_effects.rs

Lines changed: 3 additions & 1 deletion
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)