Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[arithmetic-side-effects] Finish non-overflowing ops #9483

Merged
merged 3 commits into from Sep 19, 2022
Merged

[arithmetic-side-effects] Finish non-overflowing ops #9483

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dba5ada
[arithmetic-side-effects] Finish non-overflowing ops
c410-f3r Sep 15, 2022
8bfe3db
Typo
c410-f3r Sep 15, 2022
b7bef4c
Change method's name
c410-f3r Sep 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
41 changes: 19 additions & 22 deletions clippy_lints/src/operators/arithmetic_side_effects.rs
View file
Open in desktop
Expand Up @@ -42,39 +42,30 @@ impl ArithmeticSideEffects {
}
}

/// Checks assign operators (+=, -=, *=, /=) of integers in a non-constant environment that
/// won't overflow.
fn has_valid_assign_op(op: &Spanned<hir::BinOpKind>, rhs: &hir::Expr<'_>, rhs_refs: Ty<'_>) -> bool {
if !Self::is_literal_integer(rhs, rhs_refs) {
return false;
}
/// Assuming that `expr` is a literal integer, checks operators (+=, -=, *, /) in a
/// non-constant environment that won't overflow.
fn has_valid_op(op: &Spanned<hir::BinOpKind>, expr: &hir::Expr<'_>) -> bool {
if let hir::BinOpKind::Add | hir::BinOpKind::Sub = op.node
&& let hir::ExprKind::Lit(ref lit) = rhs.kind
&& let hir::ExprKind::Lit(ref lit) = expr.kind
&& let ast::LitKind::Int(0, _) = lit.node
{
return true;
}
if let hir::BinOpKind::Div | hir::BinOpKind::Rem = op.node
&& let hir::ExprKind::Lit(ref lit) = rhs.kind
&& let hir::ExprKind::Lit(ref lit) = expr.kind
&& !matches!(lit.node, ast::LitKind::Int(0, _))
{
return true;
}
if let hir::BinOpKind::Mul = op.node
&& let hir::ExprKind::Lit(ref lit) = rhs.kind
&& let hir::ExprKind::Lit(ref lit) = expr.kind
&& let ast::LitKind::Int(0 | 1, _) = lit.node
{
return true;
}
false
}

/// Checks "raw" binary operators (+, -, *, /) of integers in a non-constant environment
/// already handled by the CTFE.
fn has_valid_bin_op(lhs: &hir::Expr<'_>, lhs_refs: Ty<'_>, rhs: &hir::Expr<'_>, rhs_refs: Ty<'_>) -> bool {
Self::is_literal_integer(lhs, lhs_refs) && Self::is_literal_integer(rhs, rhs_refs)
}

/// Checks if the given `expr` has any of the inner `allowed` elements.
fn is_allowed_ty(&self, cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool {
self.allowed.contains(
Expand All @@ -95,7 +86,8 @@ impl ArithmeticSideEffects {
}

fn issue_lint(&mut self, cx: &LateContext<'_>, expr: &hir::Expr<'_>) {
span_lint(cx, ARITHMETIC_SIDE_EFFECTS, expr.span, "arithmetic detected");
let msg = "arithmetic operation that can potentially result in unexpected side-effects";
span_lint(cx, ARITHMETIC_SIDE_EFFECTS, expr.span, msg);
self.expr_span = Some(expr.span);
}

Expand Down Expand Up @@ -127,13 +119,18 @@ impl ArithmeticSideEffects {
if self.is_allowed_ty(cx, lhs) || self.is_allowed_ty(cx, rhs) {
return;
}
let lhs_refs = cx.typeck_results().expr_ty(lhs).peel_refs();
let rhs_refs = cx.typeck_results().expr_ty(rhs).peel_refs();
let has_valid_assign_op = Self::has_valid_assign_op(op, rhs, rhs_refs);
if has_valid_assign_op || Self::has_valid_bin_op(lhs, lhs_refs, rhs, rhs_refs) {
return;
let has_valid_op = match (
Self::is_literal_integer(lhs, cx.typeck_results().expr_ty(lhs).peel_refs()),
Self::is_literal_integer(rhs, cx.typeck_results().expr_ty(rhs).peel_refs()),
) {
(true, true) => true,
(true, false) => Self::has_valid_op(op, lhs),
(false, true) => Self::has_valid_op(op, rhs),
(false, false) => false,
};
if !has_valid_op {
self.issue_lint(cx, expr);
}
self.issue_lint(cx, expr);
}
}

Expand Down
74 changes: 49 additions & 25 deletions tests/ui/arithmetic_side_effects.rs
View file
Open in desktop
@@ -1,7 +1,10 @@
#![allow(
clippy::assign_op_pattern,
unconditional_panic,
clippy::unnecessary_owned_empty_strings
clippy::erasing_op,
clippy::identity_op,
clippy::unnecessary_owned_empty_strings,
arithmetic_overflow,
unconditional_panic
)]
#![feature(inline_const, saturating_int_impl)]
#![warn(clippy::arithmetic_side_effects)]
Expand Down Expand Up @@ -30,7 +33,7 @@ pub fn hard_coded_allowed() {
}

#[rustfmt::skip]
pub fn non_overflowing_ops() {
pub fn non_overflowing_const_ops() {
const _: i32 = { let mut n = 1; n += 1; n };
let _ = const { let mut n = 1; n += 1; n };

Expand All @@ -41,33 +44,54 @@ pub fn non_overflowing_ops() {
let _ = const { let mut n = 1; n = 1 + n; n };

const _: i32 = 1 + 1;
let _ = 1 + 1;
let _ = const { 1 + 1 };
}

let mut _a = 1;
_a += 0;
_a -= 0;
_a /= 99;
_a %= 99;
_a *= 0;
_a *= 1;
pub fn non_overflowing_runtime_ops() {
let mut _n = i32::MAX;

// Assign
_n += 0;
_n -= 0;
_n /= 99;
_n %= 99;
_n *= 0;
_n *= 1;

// Binary
_n = _n + 0;
_n = 0 + _n;
_n = _n - 0;
_n = 0 - _n;
_n = _n / 99;
_n = _n % 99;
_n = _n * 0;
_n = 0 * _n;
_n = _n * 1;
_n = 1 * _n;
_n = 23 + 85;
}

#[rustfmt::skip]
pub fn overflowing_ops() {
let mut _a = 1; _a += 1;

let mut _b = 1; _b = _b + 1;

let mut _c = 1; _c = 1 + _c;

let mut _a = 1;
_a += 1;
_a -= 1;
_a /= 0;
_a %= 0;
_a *= 2;
_a *= 3;
pub fn overflowing_runtime_ops() {
let mut _n = i32::MAX;

// Assign
_n += 1;
_n -= 1;
_n /= 0;
_n %= 0;
_n *= 2;

// Binary
_n = _n + 1;
_n = 1 + _n;
_n = _n - 1;
_n = 1 - _n;
_n = _n / 0;
_n = _n % 0;
_n = _n * 2;
_n = 2 * _n;
}

fn main() {}
94 changes: 59 additions & 35 deletions tests/ui/arithmetic_side_effects.stderr
View file
Open in desktop
@@ -1,58 +1,82 @@
error: arithmetic detected
--> $DIR/arithmetic_side_effects.rs:58:21
error: arithmetic operation that can potentially result in unexpected side-effects
--> $DIR/arithmetic_side_effects.rs:80:5
|
LL | let mut _a = 1; _a += 1;
| ^^^^^^^
LL | _n += 1;
| ^^^^^^^
|
= note: `-D clippy::arithmetic-side-effects` implied by `-D warnings`

error: arithmetic detected
--> $DIR/arithmetic_side_effects.rs:60:26
error: arithmetic operation that can potentially result in unexpected side-effects
--> $DIR/arithmetic_side_effects.rs:81:5
|
LL | let mut _b = 1; _b = _b + 1;
| ^^^^^^
LL | _n -= 1;
| ^^^^^^^

error: arithmetic detected
--> $DIR/arithmetic_side_effects.rs:62:26
error: arithmetic operation that can potentially result in unexpected side-effects
--> $DIR/arithmetic_side_effects.rs:82:5
|
LL | let mut _c = 1; _c = 1 + _c;
| ^^^^^^
LL | _n /= 0;
| ^^^^^^^

error: arithmetic detected
--> $DIR/arithmetic_side_effects.rs:65:5
error: arithmetic operation that can potentially result in unexpected side-effects
--> $DIR/arithmetic_side_effects.rs:83:5
|
LL | _a += 1;
LL | _n %= 0;
| ^^^^^^^

error: arithmetic detected
--> $DIR/arithmetic_side_effects.rs:66:5
error: arithmetic operation that can potentially result in unexpected side-effects
--> $DIR/arithmetic_side_effects.rs:84:5
|
LL | _a -= 1;
LL | _n *= 2;
| ^^^^^^^

error: arithmetic detected
--> $DIR/arithmetic_side_effects.rs:67:5
error: arithmetic operation that can potentially result in unexpected side-effects
--> $DIR/arithmetic_side_effects.rs:87:10
|
LL | _a /= 0;
| ^^^^^^^
LL | _n = _n + 1;
| ^^^^^^

error: arithmetic detected
--> $DIR/arithmetic_side_effects.rs:68:5
error: arithmetic operation that can potentially result in unexpected side-effects
--> $DIR/arithmetic_side_effects.rs:88:10
|
LL | _a %= 0;
| ^^^^^^^
LL | _n = 1 + _n;
| ^^^^^^

error: arithmetic detected
--> $DIR/arithmetic_side_effects.rs:69:5
error: arithmetic operation that can potentially result in unexpected side-effects
--> $DIR/arithmetic_side_effects.rs:89:10
|
LL | _a *= 2;
| ^^^^^^^
LL | _n = _n - 1;
| ^^^^^^

error: arithmetic detected
--> $DIR/arithmetic_side_effects.rs:70:5
error: arithmetic operation that can potentially result in unexpected side-effects
--> $DIR/arithmetic_side_effects.rs:90:10
|
LL | _a *= 3;
| ^^^^^^^
LL | _n = 1 - _n;
| ^^^^^^

error: arithmetic operation that can potentially result in unexpected side-effects
--> $DIR/arithmetic_side_effects.rs:91:10
|
LL | _n = _n / 0;
| ^^^^^^

error: arithmetic operation that can potentially result in unexpected side-effects
--> $DIR/arithmetic_side_effects.rs:92:10
|
LL | _n = _n % 0;
| ^^^^^^

error: arithmetic operation that can potentially result in unexpected side-effects
--> $DIR/arithmetic_side_effects.rs:93:10
|
LL | _n = _n * 2;
| ^^^^^^

error: arithmetic operation that can potentially result in unexpected side-effects
--> $DIR/arithmetic_side_effects.rs:94:10
|
LL | _n = 2 * _n;
| ^^^^^^

error: aborting due to 9 previous errors
error: aborting due to 13 previous errors