[Linter] Checks for equal operands to comparison, logical and bitwise, difference and division binary operators (==, >, etc., &&, ||, &, |, ^, - and /).

crates/sui-framework/packages/deepbook/tests/clob_tests.move

@@ -4133,7 +4133,7 @@ module deepbook::clob_test {
                 quote_custodian,
                 account_cap_user,
                 5500 + 6 + 5,
-                9500 - 2500 - 5000 - 2000
+                0, // 9500 - 2500 - 5000 - 2000
             );
             custodian::assert_user_balance<SUI>(base_custodian, account_cap_user, 1500, 10000);
             {

external-crates/move/crates/move-compiler/src/cfgir/optimize/constant_fold.rs

@@ -4,6 +4,8 @@
 
 use crate::{
     cfgir::cfg::MutForwardCFG,
+    diag,
+    diagnostics::DiagnosticReporter,
     expansion::ast::Mutability,
     hlir::ast::{
         BaseType, BaseType_, Command, Command_, Exp, FunctionSignature, SingleType, TypeName,
@@ -19,17 +21,22 @@ use std::convert::TryFrom;
 
 /// returns true if anything changed
 pub fn optimize(
+    reporter: &DiagnosticReporter,
     _signature: &FunctionSignature,
     _locals: &UniqueMap<Var, (Mutability, SingleType)>,
     constants: &UniqueMap<ConstantName, Value>,
     cfg: &mut MutForwardCFG,
 ) -> bool {
+    let context = Context {
+        reporter,
+        constants,
+    };
     let mut changed = false;
     for block_ref in cfg.blocks_mut().values_mut() {
         let block = std::mem::take(block_ref);
         *block_ref = block
             .into_iter()
-            .filter_map(|mut cmd| match optimize_cmd(constants, &mut cmd) {
+            .filter_map(|mut cmd| match optimize_cmd(&context, &mut cmd) {
                 None => {
                     changed = true;
                     None
@@ -44,31 +51,33 @@ pub fn optimize(
     changed
 }
 
+struct Context<'a> {
+    reporter: &'a DiagnosticReporter<'a>,
+    constants: &'a UniqueMap<ConstantName, Value>,
+}
+
 //**************************************************************************************************
 // Scaffolding
 //**************************************************************************************************
 
 // Some(changed) to keep
 // None to remove the cmd
 #[growing_stack]
-fn optimize_cmd(
-    consts: &UniqueMap<ConstantName, Value>,
-    sp!(_, cmd_): &mut Command,
-) -> Option<bool> {
+fn optimize_cmd(context: &Context, sp!(_, cmd_): &mut Command) -> Option<bool> {
     use Command_ as C;
     Some(match cmd_ {
-        C::Assign(_, _ls, e) => optimize_exp(consts, e),
+        C::Assign(_, _ls, e) => optimize_exp(context, e),
         C::Mutate(el, er) => {
-            let c1 = optimize_exp(consts, er);
-            let c2 = optimize_exp(consts, el);
+            let c1 = optimize_exp(context, er);
+            let c2 = optimize_exp(context, el);
             c1 || c2
         }
         C::Return { exp: e, .. }
         | C::Abort(_, e)
         | C::JumpIf { cond: e, .. }
-        | C::VariantSwitch { subject: e, .. } => optimize_exp(consts, e),
+        | C::VariantSwitch { subject: e, .. } => optimize_exp(context, e),
         C::IgnoreAndPop { exp: e, .. } => {
-            let c = optimize_exp(consts, e);
+            let c = optimize_exp(context, e);
             if ignorable_exp(e) {
                 // value(s), so the command can be removed
                 return None;
@@ -83,9 +92,9 @@ fn optimize_cmd(
 }
 
 #[growing_stack]
-fn optimize_exp(consts: &UniqueMap<ConstantName, Value>, e: &mut Exp) -> bool {
+fn optimize_exp(context: &Context, e: &mut Exp) -> bool {
     use UnannotatedExp_ as E;
-    let optimize_exp = |e| optimize_exp(consts, e);
+    let optimize_exp = |e| optimize_exp(context, e);
     match &mut e.exp.value {
         //************************************
         // Pass through cases
@@ -103,7 +112,7 @@ fn optimize_exp(consts: &UniqueMap<ConstantName, Value>, e: &mut Exp) -> bool {
             let E::Constant(name) = e_ else {
                 unreachable!()
             };
-            if let Some(value) = consts.get(name) {
+            if let Some(value) = context.constants.get(name) {
                 *e_ = E::Value(value.clone());
                 true
             } else {
@@ -152,7 +161,20 @@ fn optimize_exp(consts: &UniqueMap<ConstantName, Value>, e: &mut Exp) -> bool {
             let changed1 = optimize_exp(e1);
             let changed2 = optimize_exp(e2);
             let changed = changed1 || changed2;
-            if let (Some(v1), Some(v2)) = (foldable_exp(e1), foldable_exp(e2)) {
+            let v1_opt = foldable_exp(e1);
+            let v2_opt = foldable_exp(e2);
+            // check operands for warnings before folding
+            // TODO warn on operations that always fail
+            check_operands(
+                context,
+                e.exp.loc,
+                e1.exp.loc,
+                v1_opt.as_ref(),
+                op,
+                e2.exp.loc,
+                v2_opt.as_ref(),
+            );
+            if let (Some(v1), Some(v2)) = (v1_opt, v2_opt) {
                 if let Some(folded) = fold_binary_op(e.exp.loc, op, v1, v2) {
                     *e_ = folded;
                     true
@@ -170,6 +192,7 @@ fn optimize_exp(consts: &UniqueMap<ConstantName, Value>, e: &mut Exp) -> bool {
                 _ => unreachable!(),
             };
             let changed = optimize_exp(e);
+            // TODO warn on operations that always fail
             let v = match foldable_exp(e) {
                 Some(v) => v,
                 None => return changed,
@@ -459,3 +482,59 @@ fn ignorable_exp(e: &Exp) -> bool {
         _ => false,
     }
 }
+
+//**************************************************************************************************
+// Warnings
+//**************************************************************************************************
+
+fn check_operands(
+    context: &Context,
+    loc: Loc,
+    lhs_loc: Loc,
+    lhs: Option<&Value_>,
+    op: &BinOp,
+    rhs_loc: Loc,
+    rhs: Option<&Value_>,
+) {
+    let Some(resulting_value) =
+        lhs.and_then(|lhs| rhs.and_then(|rhs| equal_operands(lhs, op.value, rhs)))
+    else {
+        return;
+    };
+    let msg = format!(
+        "Equal operands detected in binary operation, \
+            which always evaluates to {resulting_value}"
+    );
+    let lhs_msg = "This expression";
+    let rhs_msg = "Evaluates to the same value as this expression";
+    context.reporter.add_diag(diag!(
+        CodeGeneration::EqualOperands,
+        (loc, msg),
+        (lhs_loc, lhs_msg),
+        (rhs_loc, rhs_msg)
+    ));
+}
+
+fn equal_operands(lhs: &Value_, op: BinOp_, rhs: &Value_) -> Option<&'static str> {
+    let resulting_value = match op {
+        // warning reported elsewhere
+        BinOp_::Div | BinOp_::Mod if rhs.is_zero() => return None,
+        BinOp_::Sub | BinOp_::Mod | BinOp_::Xor => "'0'",
+        BinOp_::Div => "'1'",
+        BinOp_::BitOr | BinOp_::BitAnd | BinOp_::And | BinOp_::Or => "the same value",
+        BinOp_::Neq | BinOp_::Lt | BinOp_::Gt => "'false'",
+        BinOp_::Eq | BinOp_::Le | BinOp_::Ge => "'true'",
+        BinOp_::Add
+        | BinOp_::Mul
+        | BinOp_::Shl
+        | BinOp_::Shr
+        | BinOp_::Range
+        | BinOp_::Implies
+        | BinOp_::Iff => return None,
+    };
+    if lhs == rhs {
+        Some(resulting_value)
+    } else {
+        None
+    }
+}

external-crates/move/crates/move-compiler/src/cfgir/optimize/eliminate_locals.rs

@@ -4,6 +4,7 @@
 
 use crate::{
     cfgir::{cfg::MutForwardCFG, remove_no_ops},
+    diagnostics::DiagnosticReporter,
     expansion::ast::Mutability,
     hlir::ast::{FunctionSignature, SingleType, Value, Var},
     parser,
@@ -13,6 +14,7 @@ use std::collections::BTreeSet;
 
 /// returns true if anything changed
 pub fn optimize(
+    _reporter: &DiagnosticReporter,
     signature: &FunctionSignature,
     _locals: &UniqueMap<Var, (Mutability, SingleType)>,
     _constants: &UniqueMap<parser::ast::ConstantName, Value>,

external-crates/move/crates/move-compiler/src/cfgir/optimize/forwarding_jumps.rs

@@ -33,6 +33,7 @@ use crate::{
         ast::remap_labels,
         cfg::{MutForwardCFG, CFG},
     },
+    diagnostics::DiagnosticReporter,
     expansion::ast::Mutability,
     hlir::ast::{BasicBlocks, Command, Command_, FunctionSignature, Label, SingleType, Value, Var},
     parser::ast::ConstantName,
@@ -43,6 +44,7 @@ use std::collections::{BTreeMap, BTreeSet};
 
 /// returns true if anything changed
 pub fn optimize(
+    _reporter: &DiagnosticReporter,
     _signature: &FunctionSignature,
     _locals: &UniqueMap<Var, (Mutability, SingleType)>,
     _constants: &UniqueMap<ConstantName, Value>,

external-crates/move/crates/move-compiler/src/cfgir/optimize/inline_blocks.rs

@@ -9,6 +9,7 @@ use crate::{
         ast::remap_labels,
         cfg::{MutForwardCFG, CFG},
     },
+    diagnostics::DiagnosticReporter,
     expansion::ast::Mutability,
     hlir::ast::{BasicBlocks, Command_, FunctionSignature, Label, SingleType, Value, Var},
     parser::ast::ConstantName,
@@ -18,6 +19,7 @@ use std::collections::{BTreeMap, BTreeSet};
 
 /// returns true if anything changed
 pub fn optimize(
+    _reporter: &DiagnosticReporter,
     _signature: &FunctionSignature,
     _locals: &UniqueMap<Var, (Mutability, SingleType)>,
     _constants: &UniqueMap<ConstantName, Value>,

external-crates/move/crates/move-compiler/src/cfgir/optimize/mod.rs

@@ -12,6 +12,7 @@ use move_symbol_pool::Symbol;
 
 use crate::{
     cfgir::cfg::MutForwardCFG,
+    diagnostics::DiagnosticReporter,
     editions::FeatureGate,
     expansion::ast::Mutability,
     hlir::ast::*,
@@ -20,6 +21,7 @@ use crate::{
 };
 
 pub type Optimization = fn(
+    &DiagnosticReporter,
     &FunctionSignature,
     &UniqueMap<Var, (Mutability, SingleType)>,
     &UniqueMap<ConstantName, Value>,
@@ -44,6 +46,7 @@ const MOVE_2024_OPTIMIZATIONS: &[Optimization] = &[
 #[growing_stack]
 pub fn optimize(
     env: &CompilationEnv,
+    reporter: &DiagnosticReporter,
     package: Option<Symbol>,
     signature: &FunctionSignature,
     locals: &UniqueMap<Var, (Mutability, SingleType)>,
@@ -66,7 +69,7 @@ pub fn optimize(
         }
 
         // reset the count if something has changed
-        if optimization(signature, locals, constants, cfg) {
+        if optimization(reporter, signature, locals, constants, cfg) {
             count = 0
         } else {
             count += 1

external-crates/move/crates/move-compiler/src/cfgir/optimize/simplify_jumps.rs

@@ -6,6 +6,7 @@ use move_proc_macros::growing_stack;
 
 use crate::{
     cfgir::cfg::MutForwardCFG,
+    diagnostics::DiagnosticReporter,
     expansion::ast::Mutability,
     hlir::ast::{
         Command, Command_, Exp, FunctionSignature, SingleType, UnannotatedExp_, Value, Value_, Var,
@@ -16,6 +17,7 @@ use crate::{
 
 /// returns true if anything changed
 pub fn optimize(
+    _reporter: &DiagnosticReporter,
     _signature: &FunctionSignature,
     _locals: &UniqueMap<Var, (Mutability, SingleType)>,
     _constants: &UniqueMap<ConstantName, Value>,

external-crates/move/crates/move-compiler/src/cfgir/translate.rs

@@ -249,7 +249,7 @@ fn constants(
                 .collect::<Vec<_>>()
                 .join(", ");
             let mut diag = diag!(
-                BytecodeGeneration::UnfoldableConstant,
+                CodeGeneration::UnfoldableConstant,
                 (
                     *consts.get_loc(&scc[0]).unwrap(),
                     format!("Constant definitions form a circular dependency: {}", names),
@@ -275,7 +275,7 @@ fn constants(
             .collect();
         for node in neighbors {
             context.add_diag(diag!(
-                BytecodeGeneration::UnfoldableConstant,
+                CodeGeneration::UnfoldableConstant,
                 (
                     *consts.get_loc(&node).unwrap(),
                     format!(
@@ -517,6 +517,7 @@ fn constant_(
     );
     cfgir::optimize(
         context.env,
+        &context.reporter,
         context.current_package,
         &fake_signature,
         &locals,
@@ -526,7 +527,7 @@ fn constant_(
 
     if blocks.len() != 1 {
         context.add_diag(diag!(
-            BytecodeGeneration::UnfoldableConstant,
+            CodeGeneration::UnfoldableConstant,
             (full_loc, CANNOT_FOLD)
         ));
         return None;
@@ -538,7 +539,7 @@ fn constant_(
             C::IgnoreAndPop { exp, .. } => exp,
             _ => {
                 context.add_diag(diag!(
-                    BytecodeGeneration::UnfoldableConstant,
+                    CodeGeneration::UnfoldableConstant,
                     (*cloc, CANNOT_FOLD)
                 ));
                 continue;
@@ -560,7 +561,7 @@ fn check_constant_value(context: &mut Context, e: &H::Exp) {
     match &e.exp.value {
         E::Value(_) => (),
         _ => context.add_diag(diag!(
-            BytecodeGeneration::UnfoldableConstant,
+            CodeGeneration::UnfoldableConstant,
             (e.exp.loc, CANNOT_FOLD)
         )),
     }
@@ -677,6 +678,7 @@ fn function_body(
             if !context.env.has_errors() {
                 cfgir::optimize(
                     context.env,
+                    &context.reporter,
                     context.current_package,
                     signature,
                     &locals,