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

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

@@ -4,6 +4,7 @@
 
 use crate::{
     cfgir::cfg::MutForwardCFG,
+    diagnostics::DiagnosticReporter,
     expansion::ast::Mutability,
     hlir::ast::{
         BaseType, BaseType_, Command, Command_, Exp, FunctionSignature, SingleType, TypeName,
@@ -19,17 +20,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 +50,34 @@ pub fn optimize(
     changed
 }
 
+struct Context<'a> {
+    #[allow(dead_code)]
+    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,10 @@ 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);
+            // TODO warn on operations that always fail
+            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 +182,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,

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,

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

@@ -14,7 +14,7 @@ use crate::{
     diagnostics::{warning_filters::WarningFilters, Diagnostic, Diagnostics},
     expansion::ast::ModuleIdent,
     hlir::ast::{self as H, Command, Exp, LValue, LValue_, Label, ModuleCall, Type, Type_, Var},
-    parser::ast::{ConstantName, DatatypeName, FunctionName},
+    parser::ast::{ConstantName, DatatypeName, Field, FunctionName},
     shared::CompilationEnv,
 };
 use move_core_types::account_address::AccountAddress;
@@ -938,3 +938,76 @@ fn exp_satisfies_(e: &Exp, p: &mut impl FnMut(&Exp) -> bool) -> bool {
         }
     }
 }
+
+/// Assumes equal types and as such will not check type arguments for equality.
+/// Assumes function calls, assignments, and similar expressions are effectful and thus not equal.
+pub fn same_value_exp(e1: &H::Exp, e2: &H::Exp) -> bool {
+    same_value_exp_(&e1.exp.value, &e2.exp.value)
+}
+
+#[growing_stack]
+pub fn same_value_exp_(e1: &H::UnannotatedExp_, e2: &H::UnannotatedExp_) -> bool {
+    use H::UnannotatedExp_ as E;
+    match (e1, e2) {
+        (E::Value(v1), E::Value(v2)) => v1 == v2,
+        (E::Unit { .. }, E::Unit { .. }) => true,
+        (E::Constant(c1), E::Constant(c2)) => c1 == c2,
+        (E::Move { var, .. } | E::Copy { var, .. } | E::BorrowLocal(_, var), other)
+        | (other, E::Move { var, .. } | E::Copy { var, .. } | E::BorrowLocal(_, var)) => {
+            same_local_(var, other)
+        }
+
+        (E::Vector(_, _, _, e1), E::Vector(_, _, _, e2)) => {
+            e1.len() == e2.len()
+                && e1
+                    .iter()
+                    .zip(e2.iter())
+                    .all(|(e1, e2)| same_value_exp(e1, e2))
+        }
+
+        (E::Dereference(e) | E::Freeze(e), other) | (other, E::Dereference(e) | E::Freeze(e)) => {
+            same_value_exp_(&e.exp.value, other)
+        }
+        (E::UnaryExp(op1, e1), E::UnaryExp(op2, e2)) => op1 == op2 && same_value_exp(e1, e2),
+        (E::BinopExp(l1, op1, r1), E::BinopExp(l2, op2, r2)) => {
+            op1 == op2 && same_value_exp(l1, l2) && same_value_exp(r1, r2)
+        }
+
+        (E::Pack(n1, _, fields1), E::Pack(n2, _, fields2)) => {
+            n1 == n2 && same_value_fields(fields1, fields2)
+        }
+        (E::PackVariant(n1, v1, _, fields1), E::PackVariant(n2, v2, _, fields2)) => {
+            n1 == n2 && v1 == v2 && same_value_fields(fields1, fields2)
+        }
+
+        (E::Borrow(_, e1, f1, _), E::Borrow(_, e2, f2, _)) => f1 == f2 && same_value_exp(e1, e2),
+
+        // false for anything effectful
+        (E::ModuleCall(_), _) => false,
+
+        // TODO there is some potential for equality here, but a bit too brittle now
+        (E::Cast(_, _), _) | (E::ErrorConstant { .. }, _) => false,
+
+        // catch all
+        _ => false,
+    }
+}
+
+fn same_value_fields(
+    fields1: &[(Field, H::BaseType, Exp)],
+    fields2: &[(Field, H::BaseType, Exp)],
+) -> bool {
+    fields1.len() == fields2.len()
+        && fields1
+            .iter()
+            .zip(fields2.iter())
+            .all(|((_, _, e1), (_, _, e2))| same_value_exp(e1, e2))
+}
+
+fn same_local_(lhs: &Var, rhs: &H::UnannotatedExp_) -> bool {
+    use H::UnannotatedExp_ as E;
+    match &rhs {
+        E::Copy { var: r, .. } | E::Move { var: r, .. } | E::BorrowLocal(_, r) => lhs == r,
+        _ => false,
+    }
+}

external-crates/move/crates/move-compiler/src/diagnostics/codes.rs

@@ -290,7 +290,7 @@ codes!(
         InvalidTransfer: { msg: "invalid transfer of references", severity: NonblockingError },
         AmbiguousVariableUsage: { msg: "ambiguous usage of variable", severity: NonblockingError },
     ],
-    BytecodeGeneration: [
+    CodeGeneration: [
         UnfoldableConstant: { msg: "cannot compute constant value", severity: NonblockingError },
     ],
     // errors for any unused code or items

external-crates/move/crates/move-compiler/src/diagnostics/warning_filters.rs

@@ -1,3 +1,6 @@
+// Copyright (c) The Move Contributors
+// SPDX-License-Identifier: Apache-2.0
+
 use crate::{
     diagnostics::{
         codes::{Category, DiagnosticInfo, ExternalPrefix, Severity, UnusedItem},