nordic-dev.net/rust
nordic-dev.net/rust
2
0
Fork 0
mirror of https://github.com/rust-lang/rust.git synced 2024-11-30 18:53:39 +00:00
Code Issues Packages Projects Releases Wiki Activity

Auto merge of #12692 - c410-f3r:arrrr, r=Manishearth

Browse Source 
[arithmetic_side_effects] Fix #12318

Fix #12318

changelog: [arithmetic_side_effects]: Consider method calls that correspond to arithmetic symbols
This commit is contained in:
bors 2024-04-22 18:52:03 +00:00
commit 7fcaa60efd
3 changed files with 61 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
clippy_lints/src/operators/arithmetic_side_effects.rs
View File

@ -7,14 +7,13 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::{self, Ty};
use rustc_session::impl_lint_pass;
use rustc_span::source_map::Spanned;
use rustc_span::symbol::sym;
use rustc_span::{Span, Symbol};
use {rustc_ast as ast, rustc_hir as hir};
const HARD_CODED_ALLOWED_BINARY: &[[&str; 2]] = &[["f32", "f32"], ["f64", "f64"], ["std::string::String", "str"]];
const HARD_CODED_ALLOWED_UNARY: &[&str] = &["f32", "f64", "std::num::Saturating", "std::num::Wrapping"];
const INTEGER_METHODS: &[Symbol] = &[
const DISALLOWED_INT_METHODS: &[Symbol] = &[
sym::saturating_div,
sym::wrapping_div,
sym::wrapping_rem,
@ -27,8 +26,8 @@ pub struct ArithmeticSideEffects {
allowed_unary: FxHashSet<String>,
// Used to check whether expressions are constants, such as in enum discriminants and consts
const_span: Option<Span>,
disallowed_int_methods: FxHashSet<Symbol>,
expr_span: Option<Span>,
integer_methods: FxHashSet<Symbol>,
}
impl_lint_pass!(ArithmeticSideEffects => [ARITHMETIC_SIDE_EFFECTS]);
@ -53,8 +52,8 @@ impl ArithmeticSideEffects {
allowed_binary,
allowed_unary,
const_span: None,
disallowed_int_methods: DISALLOWED_INT_METHODS.iter().copied().collect(),
expr_span: None,
integer_methods: INTEGER_METHODS.iter().copied().collect(),
}
}
@ -91,10 +90,10 @@ impl ArithmeticSideEffects {
fn has_specific_allowed_type_and_operation<'tcx>(
cx: &LateContext<'tcx>,
lhs_ty: Ty<'tcx>,
op: &Spanned<hir::BinOpKind>,
op: hir::BinOpKind,
rhs_ty: Ty<'tcx>,
) -> bool {
let is_div_or_rem = matches!(op.node, hir::BinOpKind::Div | hir::BinOpKind::Rem);
let is_div_or_rem = matches!(op, hir::BinOpKind::Div | hir::BinOpKind::Rem);
let is_non_zero_u = |cx: &LateContext<'tcx>, ty: Ty<'tcx>| {
let tcx = cx.tcx;
@ -166,13 +165,35 @@ impl ArithmeticSideEffects {
None
}
/// Methods like `add_assign` are send to their `BinOps` references.
fn manage_sugar_methods<'tcx>(
&mut self,
cx: &LateContext<'tcx>,
expr: &'tcx hir::Expr<'_>,
lhs: &'tcx hir::Expr<'_>,
ps: &hir::PathSegment<'_>,
rhs: &'tcx hir::Expr<'_>,
) {
if ps.ident.name == sym::add || ps.ident.name == sym::add_assign {
self.manage_bin_ops(cx, expr, hir::BinOpKind::Add, lhs, rhs);
} else if ps.ident.name == sym::div || ps.ident.name == sym::div_assign {
self.manage_bin_ops(cx, expr, hir::BinOpKind::Div, lhs, rhs);
} else if ps.ident.name == sym::mul || ps.ident.name == sym::mul_assign {
self.manage_bin_ops(cx, expr, hir::BinOpKind::Mul, lhs, rhs);
} else if ps.ident.name == sym::rem || ps.ident.name == sym::rem_assign {
self.manage_bin_ops(cx, expr, hir::BinOpKind::Rem, lhs, rhs);
} else if ps.ident.name == sym::sub || ps.ident.name == sym::sub_assign {
self.manage_bin_ops(cx, expr, hir::BinOpKind::Sub, lhs, rhs);
}
}
/// Manages when the lint should be triggered. Operations in constant environments, hard coded
/// types, custom allowed types and non-constant operations that won't overflow are ignored.
/// types, custom allowed types and non-constant operations that don't overflow are ignored.
fn manage_bin_ops<'tcx>(
&mut self,
cx: &LateContext<'tcx>,
expr: &'tcx hir::Expr<'_>,
op: &Spanned<hir::BinOpKind>,
op: hir::BinOpKind,
lhs: &'tcx hir::Expr<'_>,
rhs: &'tcx hir::Expr<'_>,
) {
@ -180,7 +201,7 @@ impl ArithmeticSideEffects {
return;
}
if !matches!(
op.node,
op,
hir::BinOpKind::Add
| hir::BinOpKind::Div
| hir::BinOpKind::Mul
@ -204,7 +225,7 @@ impl ArithmeticSideEffects {
return;
}
let has_valid_op = if Self::is_integral(lhs_ty) && Self::is_integral(rhs_ty) {
if let hir::BinOpKind::Shl | hir::BinOpKind::Shr = op.node {
if let hir::BinOpKind::Shl | hir::BinOpKind::Shr = op {
// At least for integers, shifts are already handled by the CTFE
return;
}
@ -213,7 +234,7 @@ impl ArithmeticSideEffects {
Self::literal_integer(cx, actual_rhs),
) {
(None, None) => false,
(None, Some(n)) => match (&op.node, n) {
(None, Some(n)) => match (&op, n) {
// Division and module are always valid if applied to non-zero integers
(hir::BinOpKind::Div | hir::BinOpKind::Rem, local_n) if local_n != 0 => true,
// Adding or subtracting zeros is always a no-op
@ -223,7 +244,7 @@ impl ArithmeticSideEffects {
=> true,
_ => false,
},
(Some(n), None) => match (&op.node, n) {
(Some(n), None) => match (&op, n) {
// Adding or subtracting zeros is always a no-op
(hir::BinOpKind::Add | hir::BinOpKind::Sub, 0)
// Multiplication by 1 or 0 will never overflow
@ -249,6 +270,7 @@ impl ArithmeticSideEffects {
&mut self,
args: &'tcx [hir::Expr<'_>],
cx: &LateContext<'tcx>,
expr: &'tcx hir::Expr<'_>,
ps: &'tcx hir::PathSegment<'_>,
receiver: &'tcx hir::Expr<'_>,
) {
@ -262,7 +284,8 @@ impl ArithmeticSideEffects {
if !Self::is_integral(instance_ty) {
return;
}
if !self.integer_methods.contains(&ps.ident.name) {
self.manage_sugar_methods(cx, expr, receiver, ps, arg);
if !self.disallowed_int_methods.contains(&ps.ident.name) {
return;
}
let (actual_arg, _) = peel_hir_expr_refs(arg);
@ -310,10 +333,10 @@ impl<'tcx> LateLintPass<'tcx> for ArithmeticSideEffects {
}
match &expr.kind {
hir::ExprKind::AssignOp(op, lhs, rhs) | hir::ExprKind::Binary(op, lhs, rhs) => {
self.manage_bin_ops(cx, expr, op, lhs, rhs);
self.manage_bin_ops(cx, expr, op.node, lhs, rhs);
},
hir::ExprKind::MethodCall(ps, receiver, args, _) => {
self.manage_method_call(args, cx, ps, receiver);
self.manage_method_call(args, cx, expr, ps, receiver);
},
hir::ExprKind::Unary(un_op, un_expr) => {
self.manage_unary_ops(cx, expr, un_expr, *un_op);

10
tests/ui/arithmetic_side_effects.rs
View File

@ -521,4 +521,14 @@ pub fn issue_11393() {
example_rem(x, maybe_zero);
}
pub fn issue_12318() {
use core::ops::{AddAssign, DivAssign, MulAssign, RemAssign, SubAssign};
let mut one: i32 = 1;
one.add_assign(1);
one.div_assign(1);
one.mul_assign(1);
one.rem_assign(1);
one.sub_assign(1);
}
fn main() {}

14
tests/ui/arithmetic_side_effects.stderr
View File

@ -715,5 +715,17 @@ error: arithmetic operation that can potentially result in unexpected side-effec
LL | x % maybe_zero
| ^^^^^^^^^^^^^^
error: aborting due to 119 previous errors
error: arithmetic operation that can potentially result in unexpected side-effects
--> tests/ui/arithmetic_side_effects.rs:527:5
|
LL | one.add_assign(1);
| ^^^^^^^^^^^^^^^^^
error: arithmetic operation that can potentially result in unexpected side-effects
--> tests/ui/arithmetic_side_effects.rs:531:5
|
LL | one.sub_assign(1);
| ^^^^^^^^^^^^^^^^^
error: aborting due to 121 previous errors