From a083b84b783a2c9e622c9d618e751da22adaff37 Mon Sep 17 00:00:00 2001 From: Eduardo Broto Date: Tue, 9 Jun 2020 23:49:21 +0200 Subject: [PATCH 1/2] if_same_then_else: don't assume multiplication is always commutative --- clippy_lints/src/utils/hir_utils.rs | 13 +++++-------- tests/ui/if_same_then_else.rs | 12 ++++++++++++ 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/clippy_lints/src/utils/hir_utils.rs b/clippy_lints/src/utils/hir_utils.rs index f8d197c15e8d..6846658b6e29 100644 --- a/clippy_lints/src/utils/hir_utils.rs +++ b/clippy_lints/src/utils/hir_utils.rs @@ -309,18 +309,15 @@ fn swap_binop<'a>( rhs: &'a Expr<'a>, ) -> Option<(BinOpKind, &'a Expr<'a>, &'a Expr<'a>)> { match binop { - BinOpKind::Add - | BinOpKind::Mul - | BinOpKind::Eq - | BinOpKind::Ne - | BinOpKind::BitAnd - | BinOpKind::BitXor - | BinOpKind::BitOr => Some((binop, rhs, lhs)), + BinOpKind::Add | BinOpKind::Eq | BinOpKind::Ne | BinOpKind::BitAnd | BinOpKind::BitXor | BinOpKind::BitOr => { + Some((binop, rhs, lhs)) + }, BinOpKind::Lt => Some((BinOpKind::Gt, rhs, lhs)), BinOpKind::Le => Some((BinOpKind::Ge, rhs, lhs)), BinOpKind::Ge => Some((BinOpKind::Le, rhs, lhs)), BinOpKind::Gt => Some((BinOpKind::Lt, rhs, lhs)), - BinOpKind::Shl + BinOpKind::Mul + | BinOpKind::Shl | BinOpKind::Shr | BinOpKind::Rem | BinOpKind::Sub diff --git a/tests/ui/if_same_then_else.rs b/tests/ui/if_same_then_else.rs index 6bbf79edfcf7..9c5fe02f7519 100644 --- a/tests/ui/if_same_then_else.rs +++ b/tests/ui/if_same_then_else.rs @@ -142,4 +142,16 @@ fn func() { fn f(val: &[u8]) {} +mod issue_5698 { + fn mul_not_always_commutative(x: i32, y: i32) -> i32 { + if x == 42 { + x * y + } else if x == 21 { + y * x + } else { + 0 + } + } +} + fn main() {} From 2f74283fce768a262387fe7f51e1e4ebb9b0e300 Mon Sep 17 00:00:00 2001 From: Eduardo Broto Date: Wed, 10 Jun 2020 00:14:02 +0200 Subject: [PATCH 2/2] Add a comment linking to the issue --- clippy_lints/src/utils/hir_utils.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/utils/hir_utils.rs b/clippy_lints/src/utils/hir_utils.rs index 6846658b6e29..9c2c96203c03 100644 --- a/clippy_lints/src/utils/hir_utils.rs +++ b/clippy_lints/src/utils/hir_utils.rs @@ -316,7 +316,7 @@ fn swap_binop<'a>( BinOpKind::Le => Some((BinOpKind::Ge, rhs, lhs)), BinOpKind::Ge => Some((BinOpKind::Le, rhs, lhs)), BinOpKind::Gt => Some((BinOpKind::Lt, rhs, lhs)), - BinOpKind::Mul + BinOpKind::Mul // Not always commutative, e.g. with matrices. See issue #5698 | BinOpKind::Shl | BinOpKind::Shr | BinOpKind::Rem