[compiler-v2][lint] Complex boolean expression checker #16721

jgcrosta · 2025-05-29T22:14:47Z

Description

This PR introduces a new lint detector for the move-linter: simplifiable_boolean_expression. This detector identifies overly complex boolean expressions that can be simplified using fundamental boolean algebra laws.

Supported Simplification Patterns:

Absorption Laws:

a && b || a → a
a || a && b → a

Idempotence Laws:

a && a → a
a || a → a

Contradiction Laws:

a && !a → false
!a && a → false

Tautology Laws:

a || !a → true
!a || a → true

Distributive Laws:

(a && b) || (a && c) → a && (b || c)
(a || b) && (a || c) → a || (b && c)

The detector analyzes boolean expressions in Move code and suggests simplified equivalents.

How Has This Been Tested?

Added manual tests (/tests/model_ast_lints/simplifiable_boolean_expression.move)
This lint do not triggers in any file in aptos-move/framework/*

Key Areas to Review

Pattern detection logic:
- check_absorption_law: Detects when OR expressions contain redundant AND sub-expressions
- check_idempotence: Identifies duplicate operands in AND/OR expressions
- check_contradiction_tautology: Finds expressions with negated operands
- check_distributive_law: Complex pattern matching for distributive simplifications
Expression equality checking:
- is_expression_equal: This method handles structural comparison of AST nodes by value/symbol
Suggestions:
- Added SimplifiablePattern to handle suggestions.

Type of Change

New feature

Which Components or Systems Does This Change Impact?

Other (move-linter)

third_party/move/tools/move-linter/src/model_ast_lints.rs

third_party/move/tools/move-linter/tests/model_ast_lints/simplifiable_boolean_expression.move

third_party/move/tools/move-linter/src/model_ast_lints/simplifiable_boolean_expression.rs

jgcrosta · 2025-06-04T14:49:11Z

@vineethk PTAL

vineethk

Looking generally good, please address the new comments.

third_party/move/tools/move-linter/src/model_ast_lints/simpler_bool_expression.rs

vineethk · 2025-06-10T14:49:18Z

third_party/move/tools/move-linter/src/model_ast_lints/simpler_bool_expression.rs

+            (ExpData::LocalVar(_, s1), ExpData::LocalVar(_, s2)) => s1 == s2,
+            (ExpData::Value(_, v1), ExpData::Value(_, v2)) => v1 == v2,
+            (ExpData::Temporary(_, t1), ExpData::Temporary(_, t2)) => t1 == t2,
+            (ExpData::Call(_, op1, args1), ExpData::Call(_, op2, args2)) => {


This is not true if the operation can be side-effecting (which means calling it twice is different from calling it once). We should assume that all user-defined functions are side-effecting.

You may be able to use Operation::is_ok_to_remove_from_code to approximate your usage here (but please verify and double check carefully what that function does, and add corresponding test cases).

The idea is to use operation.is_ok_to_remove_from_code()? I could modify it to fit this scenario, but I'm not sure this would work since functions like the following still trigger the lint, even though helper_function() could be side-effecting. This function (is_ok_to_remove_from_code) seems to be defined for ExpData too, but it still triggers the lint..

public fun test_distributive_law_mixed() { let x = helper_function(); let y = 5; if ((x && TRUE_CONST) || (x && (y > 10))) (); if ((x || FALSE_CONST) && (x || (y > 10))) (); }

When you have:

let x = helper_function(); x && x && x;

x is only evaluated once (so it does not matter if it is side-effecting), as opposed to:

helper_function() && helper_function() && helper_function().

Please check whether the function suggested is appropriate (if not, do not use it): you may want to only disallow user defined functions here.

done.
I have added checks on is_expression_equal to ignore operations of type MoveFunction on the expressions itself or any of the arguments. Let me know if that covers it. I could add some sort of logic to check wether it was only used once, I'm not sure this adds value to the end user though, right now if a user defined function is used, the check gets ignored.

There are other Operation which can return different results or carry side effects when Called twice, e.g., Closure and MoveFrom.

Good point, we need to consider some of the other things that can be side-effecting as well!

I think we also need to handle Invoke (similar to Call, but instead of a fixed entity to call, it can call whatever an expression evaluates to). This needs tests.

Not completely sure if we need to include MoveFrom (or even Closure) etc in these contexts. For example, it is an error to move from a global value twice, and the lint can assume that. Either way, looks like we need some tests to showcase these.

Nevermind, Invoke is already handled (I was confusing it with the stackless bytecode's Invoke).

Might still be worth it to think through each Operation and add some tests (e.g., (|| true)() && (|| true).

On the other hand, it might be easier to allow specific Operations (write a function that matches over Operation and returns true or false on whether it is appropriate for consideration here, in this way we only explicitly allow stuff we want to, rather than inadvertently allow something). It is okay to have some false negatives (we always will) instead of false positives (which we should strive to minimize).

Yeah, calling MoveFrom twice with the same arg will be an error, but I felt we might consider detecting that separately. Otherwise, the user will see a suggestion like MoveFrom<ty1>(addr1) && MoveFrom<ty1>(addr1) can be simplified to MoveFrom<ty1>(addr1).

I am concerned about Closure because it can call MoveFunction inside.

Yeah, reasonable point about MoveFrom: I don't remember whether we err out by the time we reach the lint phase for this or not - a test case would tell.

Note: Closure cannot "call" MoveFunction inside: it only "creates" the closure. It is Invoke that calls the closure, which in turn calls the function inside (the terms referred to here are all the model AST terms, to remove ambiguity).

Given all these discussions, I am more of the opinion that we should have a function that matches over various Operation that true or false on whether such operations should participate in the structural equality.

oh, good to learn about Closure v.s. Invoke! Yes, I would also support a match over various Operation. Something like is_ok_to_remove_from_code but less restrictive.

third_party/move/tools/move-linter/tests/model_ast_lints/simpler_bool_expression.exp

third_party/move/tools/move-linter/tests/model_ast_lints/simpler_bool_expression.move

third_party/move/tools/move-linter/src/model_ast_lints/simpler_bool_expression.rs

jgcrosta · 2025-06-12T17:21:29Z

@vineethk PTAL

vineethk

Please post results of running on certain projects like aptos-framework.

third_party/move/tools/move-linter/tests/model_ast_lints/simpler_bool_expression.move

jgcrosta · 2025-06-17T15:15:30Z

Results of running the compiled linter on:

aptos-framework:

./../../../target/release/aptos move lint         
                                                                                                                                     ─
Compiling, may take a little while to download git dependencies...
INCLUDING DEPENDENCY AptosStdlib
INCLUDING DEPENDENCY MoveStdlib
BUILDING AptosFramework
warning: [lint] This is an unnecessary self assignment. Consider removing it.
    ┌─ /Users/josegarcia/Desktop/aptos-core/aptos-move/framework/aptos-framework/sources/aptos_coin.move:116:17
    │
116 │             let element: &DelegatedMintCapability = element;
    │                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    │
    = To suppress this warning, annotate the function/module with the attribute `#[lint::skip(self_assignment)]`.
    = For more information, see https://aptos.dev/en/build/smart-contracts/linter#self_assignment.

warning: [lint] This is an unnecessary self assignment. Consider removing it.
    ┌─ /Users/josegarcia/Desktop/aptos-core/aptos-move/framework/aptos-framework/sources/code.move:194:17
    │
194 │             let old: &PackageMetadata = old;
    │                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^
    │
    = To suppress this warning, annotate the function/module with the attribute `#[lint::skip(self_assignment)]`.
    = For more information, see https://aptos.dev/en/build/smart-contracts/linter#self_assignment.

warning: [lint] This is an unnecessary self assignment. Consider removing it.
    ┌─ /Users/josegarcia/Desktop/aptos-core/aptos-move/framework/aptos-framework/sources/code.move:285:17
    │
285 │             let old_mod: &ModuleMetadata = old_mod;
    │                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    │
    = To suppress this warning, annotate the function/module with the attribute `#[lint::skip(self_assignment)]`.
    = For more information, see https://aptos.dev/en/build/smart-contracts/linter#self_assignment.

warning: [lint] This is an unnecessary self assignment. Consider removing it.
    ┌─ /Users/josegarcia/Desktop/aptos-core/aptos-move/framework/aptos-framework/sources/code.move:303:17
    │
303 │             let dep: &PackageDep = dep;
    │                 ^^^^^^^^^^^^^^^^^^^^^^
    │
    = To suppress this warning, annotate the function/module with the attribute `#[lint::skip(self_assignment)]`.
    = For more information, see https://aptos.dev/en/build/smart-contracts/linter#self_assignment.

warning: [lint] This is an unnecessary self assignment. Consider removing it.
    ┌─ /Users/josegarcia/Desktop/aptos-core/aptos-move/framework/aptos-framework/sources/code.move:313:25
    │
313 │                     let dep_pack: &PackageMetadata = dep_pack;
    │                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    │
    = To suppress this warning, annotate the function/module with the attribute `#[lint::skip(self_assignment)]`.
    = For more information, see https://aptos.dev/en/build/smart-contracts/linter#self_assignment.

warning: [lint] This is an unnecessary self assignment. Consider removing it.
    ┌─ /Users/josegarcia/Desktop/aptos-core/aptos-move/framework/aptos-framework/sources/code.move:358:17
    │
358 │             let pack_module: &ModuleMetadata = pack_module;
    │                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    │
    = To suppress this warning, annotate the function/module with the attribute `#[lint::skip(self_assignment)]`.
    = For more information, see https://aptos.dev/en/build/smart-contracts/linter#self_assignment.

warning: [lint] This binary operation can be simplified to just the left-hand side
     ┌─ /Users/josegarcia/Desktop/aptos-core/aptos-move/framework/aptos-framework/sources/coin.move:1165:39
     │
1165 │             update supply<CoinType> = supply<CoinType> + 0;
     │                                       ^^^^^^^^^^^^^^^^^^^^
     │
     = To suppress this warning, annotate the function/module with the attribute `#[lint::skip(simpler_numeric_expression)]`.
     = For more information, see https://aptos.dev/en/build/smart-contracts/linter#simpler_numeric_expression.

warning: [lint] This is an unnecessary self assignment. Consider removing it.
   ┌─ /Users/josegarcia/Desktop/aptos-core/aptos-move/framework/aptos-framework/sources/configs/jwk_consensus_config.move:93:17
   │
93 │             let provider: &OIDCProvider = provider;
   │                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   │
   = To suppress this warning, annotate the function/module with the attribute `#[lint::skip(self_assignment)]`.
   = For more information, see https://aptos.dev/en/build/smart-contracts/linter#self_assignment.

warning: [lint] This if-else can be replaced with just the condition
    ┌─ /Users/josegarcia/Desktop/aptos-core/aptos-move/framework/aptos-framework/sources/datastructures/big_ordered_map.move:385:16
    │
385 │           } else if (&lower_bound.key == key) {
    │ ╭────────────────^
386 │ │             true
387 │ │         } else {
388 │ │             false
389 │ │         }
    │ ╰─────────^
    │
    = To suppress this warning, annotate the function/module with the attribute `#[lint::skip(needless_bool)]`.
    = For more information, see https://aptos.dev/en/build/smart-contracts/linter#needless_bool.

warning: [lint] This is an unnecessary self assignment. Consider removing it.
    ┌─ /Users/josegarcia/Desktop/aptos-core/aptos-move/framework/aptos-framework/sources/datastructures/ordered_map.move:511:17
    │
511 │             let e: &Entry<K, V> = e;
    │                 ^^^^^^^^^^^^^^^^^^^
    │
    = To suppress this warning, annotate the function/module with the attribute `#[lint::skip(self_assignment)]`.
    = For more information, see https://aptos.dev/en/build/smart-contracts/linter#self_assignment.

warning: [lint] This is an unnecessary self assignment. Consider removing it.
    ┌─ /Users/josegarcia/Desktop/aptos-core/aptos-move/framework/aptos-framework/sources/datastructures/ordered_map.move:519:17
    │
519 │             let e: &Entry<K, V> = e;
    │                 ^^^^^^^^^^^^^^^^^^^
    │
    = To suppress this warning, annotate the function/module with the attribute `#[lint::skip(self_assignment)]`.
    = For more information, see https://aptos.dev/en/build/smart-contracts/linter#self_assignment.

warning: [lint] This is an unnecessary self assignment. Consider removing it.
    ┌─ /Users/josegarcia/Desktop/aptos-core/aptos-move/framework/aptos-framework/sources/genesis.move:177:17
    │
177 │             let account_map: &AccountMap = account_map;
    │                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    │
    = To suppress this warning, annotate the function/module with the attribute `#[lint::skip(self_assignment)]`.
    = For more information, see https://aptos.dev/en/build/smart-contracts/linter#self_assignment.

warning: [lint] This is an unnecessary self assignment. Consider removing it.
    ┌─ /Users/josegarcia/Desktop/aptos-core/aptos-move/framework/aptos-framework/sources/genesis.move:217:17
    │
217 │             let employee_group: &EmployeeAccountMap = employee_group;
    │                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    │
    = To suppress this warning, annotate the function/module with the attribute `#[lint::skip(self_assignment)]`.
    = For more information, see https://aptos.dev/en/build/smart-contracts/linter#self_assignment.

warning: [lint] This is an unnecessary self assignment. Consider removing it.
    ┌─ /Users/josegarcia/Desktop/aptos-core/aptos-move/framework/aptos-framework/sources/genesis.move:302:17
    │
302 │             let validator: &ValidatorConfigurationWithCommission = validator;
    │                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    │
    = To suppress this warning, annotate the function/module with the attribute `#[lint::skip(self_assignment)]`.
    = For more information, see https://aptos.dev/en/build/smart-contracts/linter#self_assignment.

warning: [lint] Comparison is always true, consider rewriting the code to remove the redundant comparison
    ┌─ /Users/josegarcia/Desktop/aptos-core/aptos-move/framework/aptos-framework/sources/randomness.move:281:27
    │
281 │                 invariant sample >= 0 && sample < max_excl - min_incl;
    │                           ^^^^^^^^^^^
    │
    = To suppress this warning, annotate the function/module with the attribute `#[lint::skip(unnecessary_numerical_extreme_comparison)]`.
    = For more information, see https://aptos.dev/en/build/smart-contracts/linter#unnecessary_numerical_extreme_comparison.

warning: [lint] Comparison is always true, consider rewriting the code to remove the redundant comparison
    ┌─ /Users/josegarcia/Desktop/aptos-core/aptos-move/framework/aptos-framework/sources/randomness.move:291:20
    │
291 │             assert sample >= 0 && sample < max_excl - min_incl;
    │                    ^^^^^^^^^^^
    │
    = To suppress this warning, annotate the function/module with the attribute `#[lint::skip(unnecessary_numerical_extreme_comparison)]`.
    = For more information, see https://aptos.dev/en/build/smart-contracts/linter#unnecessary_numerical_extreme_comparison.

warning: [lint] Comparison is always true, consider rewriting the code to remove the redundant comparison
    ┌─ /Users/josegarcia/Desktop/aptos-core/aptos-move/framework/aptos-framework/sources/randomness.move:330:27
    │
330 │                 invariant tail >= 0 && tail < len(values);
    │                           ^^^^^^^^^
    │
    = To suppress this warning, annotate the function/module with the attribute `#[lint::skip(unnecessary_numerical_extreme_comparison)]`.
    = For more information, see https://aptos.dev/en/build/smart-contracts/linter#unnecessary_numerical_extreme_comparison.

warning: [lint] This is an unnecessary self assignment. Consider removing it.
     ┌─ /Users/josegarcia/Desktop/aptos-core/aptos-move/framework/aptos-framework/sources/stake.move:1262:17
     │
1262 │             let validator: &ValidatorInfo = validator;
     │                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     │
     = To suppress this warning, annotate the function/module with the attribute `#[lint::skip(self_assignment)]`.
     = For more information, see https://aptos.dev/en/build/smart-contracts/linter#self_assignment.

warning: [lint] This is an unnecessary self assignment. Consider removing it.
     ┌─ /Users/josegarcia/Desktop/aptos-core/aptos-move/framework/aptos-framework/sources/stake.move:1269:17
     │
1269 │             let validator: &ValidatorInfo = validator;
     │                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     │
     = To suppress this warning, annotate the function/module with the attribute `#[lint::skip(self_assignment)]`.
     = For more information, see https://aptos.dev/en/build/smart-contracts/linter#self_assignment.

warning: [lint] Comparison is always true, consider rewriting the code to remove the redundant comparison
     ┌─ /Users/josegarcia/Desktop/aptos-core/aptos-move/framework/aptos-framework/sources/stake.move:1325:27
     │
1325 │                 invariant 0 <= validator_index && validator_index <= vlen;
     │                           ^^^^^^^^^^^^^^^^^^^^
     │
     = To suppress this warning, annotate the function/module with the attribute `#[lint::skip(unnecessary_numerical_extreme_comparison)]`.
     = For more information, see https://aptos.dev/en/build/smart-contracts/linter#unnecessary_numerical_extreme_comparison.

warning: [lint] Comparison is always true, consider rewriting the code to remove the redundant comparison
    ┌─ /Users/josegarcia/Desktop/aptos-core/aptos-move/framework/aptos-framework/sources/staking_contract.move:374:13
    │
374 │             commission_percentage >= 0 && commission_percentage <= 100,
    │             ^^^^^^^^^^^^^^^^^^^^^^^^^^
    │
    = To suppress this warning, annotate the function/module with the attribute `#[lint::skip(unnecessary_numerical_extreme_comparison)]`.
    = For more information, see https://aptos.dev/en/build/smart-contracts/linter#unnecessary_numerical_extreme_comparison.

warning: [lint] Comparison is always true, consider rewriting the code to remove the redundant comparison
    ┌─ /Users/josegarcia/Desktop/aptos-core/aptos-move/framework/aptos-framework/sources/staking_contract.move:500:13
    │
500 │             new_commission_percentage >= 0 && new_commission_percentage <= 100,
    │             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    │
    = To suppress this warning, annotate the function/module with the attribute `#[lint::skip(unnecessary_numerical_extreme_comparison)]`.
    = For more information, see https://aptos.dev/en/build/smart-contracts/linter#unnecessary_numerical_extreme_comparison.

warning: [lint] This binary operation can be simplified to just the right-hand side
    ┌─ /Users/josegarcia/Desktop/aptos-core/aptos-move/framework/aptos-framework/sources/storage_gas.move:406:27
    │
406 │             target_usage: 1 * m * m, // 1TB
    │                           ^^^^^
    │
    = To suppress this warning, annotate the function/module with the attribute `#[lint::skip(simpler_numeric_expression)]`.
    = For more information, see https://aptos.dev/en/build/smart-contracts/linter#simpler_numeric_expression.

warning: [lint] Needless mutable reference or borrow: consider using immutable reference or borrow instead
    ┌─ /Users/josegarcia/Desktop/aptos-core/aptos-move/framework/aptos-framework/sources/account/account.move:524:32
    │
524 │         let account_resource = &mut Account[addr];
    │                                ^^^^^^^^^^^^^^^^^^
    │
    = To suppress this warning, annotate the function/module with the attribute `#[lint::skip(needless_mutable_reference)]`.
    = For more information, see https://aptos.dev/en/build/smart-contracts/linter#needless_mutable_reference.

warning: [lint] Compare using references of these values instead (i.e., place `&` on both the operands), to avoid unnecessary copies.
    ┌─ /Users/josegarcia/Desktop/aptos-core/aptos-move/framework/aptos-framework/sources/coin.move:442:13
    │
442 │             fungible_asset::mint_ref_metadata(&mint_ref) == metadata,
    │             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    │
    = To suppress this warning, annotate the function/module with the attribute `#[lint::skip(avoid_copy_on_identity_comparison)]`.
    = For more information, see https://aptos.dev/en/build/smart-contracts/linter#avoid_copy_on_identity_comparison.

warning: [lint] Compare using references of these values instead (i.e., place `&` on both the operands), to avoid unnecessary copies.
    ┌─ /Users/josegarcia/Desktop/aptos-core/aptos-move/framework/aptos-framework/sources/coin.move:478:13
    │
478 │             fungible_asset::transfer_ref_metadata(&transfer_ref) == metadata,
    │             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    │
    = To suppress this warning, annotate the function/module with the attribute `#[lint::skip(avoid_copy_on_identity_comparison)]`.
    = For more information, see https://aptos.dev/en/build/smart-contracts/linter#avoid_copy_on_identity_comparison.

warning: [lint] Compare using references of these values instead (i.e., place `&` on both the operands), to avoid unnecessary copies.
    ┌─ /Users/josegarcia/Desktop/aptos-core/aptos-move/framework/aptos-framework/sources/coin.move:528:13
    │
528 │             fungible_asset::burn_ref_metadata(&burn_ref) == metadata,
    │             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    │
    = To suppress this warning, annotate the function/module with the attribute `#[lint::skip(avoid_copy_on_identity_comparison)]`.
    = For more information, see https://aptos.dev/en/build/smart-contracts/linter#avoid_copy_on_identity_comparison.

warning: [lint] Needless mutable reference or borrow: consider using immutable reference or borrow instead
     ┌─ /Users/josegarcia/Desktop/aptos-core/aptos-move/framework/aptos-framework/sources/datastructures/big_ordered_map.move:1070:30
     │
1070 │         let left_prev = &mut left_node.prev;
     │                              ^^^^^^^^^
     │
     = To suppress this warning, annotate the function/module with the attribute `#[lint::skip(needless_mutable_reference)]`.
     = For more information, see https://aptos.dev/en/build/smart-contracts/linter#needless_mutable_reference.

warning: [lint] Compare using references of these values instead (i.e., place `&` on both the operands), to avoid unnecessary copies.
    ┌─ /Users/josegarcia/Desktop/aptos-core/aptos-move/framework/aptos-framework/sources/jwks.move:447:13
    │
447 │             provider.name == name
    │             ^^^^^^^^^^^^^^^^^^^^^
    │
    = To suppress this warning, annotate the function/module with the attribute `#[lint::skip(avoid_copy_on_identity_comparison)]`.
    = For more information, see https://aptos.dev/en/build/smart-contracts/linter#avoid_copy_on_identity_comparison.

warning: [lint] Compare using references of these values instead (i.e., place `&` on both the operands), to avoid unnecessary copies.
    ┌─ /Users/josegarcia/Desktop/aptos-core/aptos-move/framework/aptos-framework/sources/jwks.move:613:13
    │
613 │             provider_jwk_set.issuer == issuer
    │             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    │
    = To suppress this warning, annotate the function/module with the attribute `#[lint::skip(avoid_copy_on_identity_comparison)]`.
    = For more information, see https://aptos.dev/en/build/smart-contracts/linter#avoid_copy_on_identity_comparison.

aptos-stdlib:

./../../../target/release/aptos move lint                                                                                                                                             
Compiling, may take a little while to download git dependencies...
INCLUDING DEPENDENCY MoveStdlib
BUILDING AptosStdlib
warning: [lint] Having blocks in conditions make code harder to read. Consider rewriting this code.
   ┌─ /Users/josegarcia/Desktop/aptos-core/aptos-move/framework/aptos-stdlib/sources/data_structures/smart_table.move:138:17
   │
138 │           assert!(bucket.all(| entry | {
   │ ╭─────────────────^
139 │ │             let e: &Entry<K, V> = entry;
140 │ │             &e.key != &key
141 │ │         }), error::invalid_argument(EALREADY_EXIST));
   │ ╰──────────^
   │
   = To suppress this warning, annotate the function/module with the attribute `#[lint::skip(blocks_in_conditions)]`.
   = For more information, see https://aptos.dev/en/build/smart-contracts/linter#blocks_in_conditions.

warning: [lint] Needless mutable reference or borrow: consider using immutable reference or borrow instead
  ┌─ /Users/josegarcia/Desktop/aptos-core/aptos-move/framework/aptos-stdlib/sources/bcs_stream.move:39:30
  │
39 │     public fun has_remaining(stream: &mut BCSStream): bool {
  │                              ^^^^^^
  │
  = To suppress this warning, annotate the function/module with the attribute `#[lint::skip(needless_mutable_reference)]`.
  = For more information, see https://aptos.dev/en/build/smart-contracts/linter#needless_mutable_reference.

aptos-token:

./../../../target/release/aptos move lint                                                                                                                                             
Compiling, may take a little while to download git dependencies...
INCLUDING DEPENDENCY AptosFramework
INCLUDING DEPENDENCY AptosStdlib
INCLUDING DEPENDENCY MoveStdlib
BUILDING AptosToken
warning: [lint] This is an unnecessary self assignment. Consider removing it.
    ┌─ /Users/josegarcia/Desktop/aptos-core/aptos-move/framework/aptos-token/sources/token.move:1863:17
    │
1863 │             let key: &String = key;
    │                 ^^^^^^^^^^^^^^^^^^
    │
    = To suppress this warning, annotate the function/module with the attribute `#[lint::skip(self_assignment)]`.
    = For more information, see https://aptos.dev/en/build/smart-contracts/linter#self_assignment.

aptos-token-objects:

./../../../target/release/aptos move lint                                                                                                                                             
Compiling, may take a little while to download git dependencies...
INCLUDING DEPENDENCY AptosFramework
INCLUDING DEPENDENCY AptosStdlib
INCLUDING DEPENDENCY MoveStdlib
BUILDING AptosTokenObjects

-> This new lint simpler_bool_expression does not seem to be triggered by any of these projects

jgcrosta · 2025-06-17T15:16:42Z

@vineethk PTAL

third_party/move/tools/move-linter/tests/model_ast_lints/simpler_bool_expression.move

github-actions · 2025-07-01T22:16:36Z

❌ Forge suite `compat` failure on `20fcb95ffc2aedc2fd7fce2d955e6823b3a4e585` ==> `5b328b258b26dd6a3a9a3dd9f0454d4e270332cb`

Forge test runner terminated:
Trailing Log Lines:
             at /rustc/05f9846f893b09a1be1fc8560e33fc3c815cfecb/library/std/src/panicking.rs:587:40
  34: std::panicking::try
             at /rustc/05f9846f893b09a1be1fc8560e33fc3c815cfecb/library/std/src/panicking.rs:550:19
  35: std::panic::catch_unwind
             at /rustc/05f9846f893b09a1be1fc8560e33fc3c815cfecb/library/std/src/panic.rs:358:14
  36: std::rt::lang_start_internal
             at /rustc/05f9846f893b09a1be1fc8560e33fc3c815cfecb/library/std/src/rt.rs:164:5
  37: main
  38: __libc_start_main
  39: _start
Debugging output:
NAME                                    READY   STATUS     RESTARTS   AGE
aptos-node-0-validator-0                0/1     Init:0/1   0          20m
aptos-node-1-validator-0                0/1     Init:0/1   0          20m
aptos-node-2-validator-0                0/1     Init:0/1   0          20m
aptos-node-3-validator-0                0/1     Init:0/1   0          20m
forge-testnet-deployer-rl25x            0/1     Error      0          21m
genesis-aptos-genesis-eforge105-29cdk   0/1     Error      0          7m56s
genesis-aptos-genesis-eforge105-7kgzx   0/1     Error      0          13m
genesis-aptos-genesis-eforge105-7npxl   0/1     Error      0          20m
genesis-aptos-genesis-eforge105-cdnd2   0/1     Error      0          19m
genesis-aptos-genesis-eforge105-m7fkx   0/1     Error      0          16m
genesis-aptos-genesis-eforge105-q48qk   0/1     Error      0          18m
genesis-aptos-genesis-eforge105-wmzzs   0/1     Error      0          20m

junxzm1990

Looks good to me!

github-actions · 2025-09-03T17:04:21Z

✅ Forge suite `realistic_env_max_load` success on `d80ea2a6d1fc7f5e6b1b62906621882aa1fc25f1`

two traffics test: inner traffic : committed: 11870.14 txn/s, submitted: 11870.36 txn/s, expired: 0.21 txn/s, latency: 3176.86 ms, (p50: 3000 ms, p70: 3100, p90: 3300 ms, p99: 5100 ms), latency samples: 4513280
two traffics test : committed: 99.99 txn/s, latency: 1118.11 ms, (p50: 900 ms, p70: 1300, p90: 1600 ms, p99: 2600 ms), latency samples: 1740
Latency breakdown for phase 0: ["MempoolToBlockCreation: max: 2.714, avg: 2.266", "ConsensusProposalToOrdered: max: 0.166, avg: 0.163", "ConsensusOrderedToCommit: max: 0.604, avg: 0.299", "ConsensusProposalToCommit: max: 0.770, avg: 0.462"]
Max non-epoch-change gap was: 1 rounds at version 3655588 (avg 0.00) [limit 4], 1.16s no progress at version 5187192 (avg 0.07s) [limit 15].
Max epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 0.35s no progress at version 2104636 (avg 0.35s) [limit 16].
Test Ok

… are known constants. Only evaluate side-effect free equality on expressions (remove `move-functions`)

…er_bool_expression.move Co-authored-by: Vineeth Kashyap <[email protected]>

vineethk self-requested a review May 30, 2025 14:26

vineethk reviewed Jun 2, 2025

View reviewed changes

jgcrosta requested a review from vineethk June 4, 2025 14:48

vineethk reviewed Jun 10, 2025

View reviewed changes

jgcrosta requested a review from vineethk June 12, 2025 17:21

vineethk reviewed Jun 16, 2025

View reviewed changes

third_party/move/tools/move-linter/tests/model_ast_lints/simpler_bool_expression.move Show resolved Hide resolved

vineethk requested a review from junxzm1990 June 16, 2025 14:42

jgcrosta requested a review from vineethk June 17, 2025 15:16

vineethk reviewed Jun 18, 2025

View reviewed changes

third_party/move/tools/move-linter/tests/model_ast_lints/simpler_bool_expression.move Outdated Show resolved Hide resolved

matiascabello mentioned this pull request Jun 19, 2025

Find needlessly complex boolean expressions that can be simplified CoinFabrik/aptos-core#5

Open

jgcrosta requested a review from vineethk June 20, 2025 15:19

vineethk approved these changes Jul 1, 2025

View reviewed changes

vineethk added the CICD:run-e2e-tests when this label is present github actions will run all land-blocking e2e tests from the PR label Jul 1, 2025

This comment has been minimized.

Sign in to view

jgcrosta force-pushed the 5-find-needlessly-complex-boolean-expressions-that-can-be-simplified branch from 5b328b2 to 2d4b191 Compare July 8, 2025 19:42

Helios-vmg force-pushed the 5-find-needlessly-complex-boolean-expressions-that-can-be-simplified branch from 2d4b191 to f40a908 Compare July 10, 2025 14:08

junxzm1990 approved these changes Aug 25, 2025

View reviewed changes

jgcrosta force-pushed the 5-find-needlessly-complex-boolean-expressions-that-can-be-simplified branch 2 times, most recently from bee3c80 to d80ea2a Compare September 2, 2025 14:41

vineethk enabled auto-merge (squash) September 3, 2025 16:28

This comment has been minimized.

Sign in to view

auto-merge was automatically disabled September 4, 2025 17:28
Head branch was pushed to by a user without write access

jgcrosta and others added 16 commits September 10, 2025 12:05

Add redundant boolean expression detector

ee0704d

Add tests

374792b

Add detector to base

db2d2ca

Add distributive law and remove already existing checks

e3017d4

Update naming and prevent panic on unwrap

a08a899

Rename detector for more accurate representation

fb6e290

Rename files

1626155

Add several tests and support ExpData::Call type

167c9d2

Update linter suggestions

b6672c1

Disable idempotence, contradiction and tautology when the expressions…

72b71f4

… are known constants. Only evaluate side-effect free equality on expressions (remove `move-functions`)

Updated comments

c9569bc

Add double negation support

74c887d

Add vector tests

443b89d

Update third_party/move/tools/move-linter/tests/model_ast_lints/simpl…

5eda4ac

…er_bool_expression.move Co-authored-by: Vineeth Kashyap <[email protected]>

Update detector

9ec61d1

Update tests baseline

0034d08

jgcrosta force-pushed the 5-find-needlessly-complex-boolean-expressions-that-can-be-simplified branch from dd52118 to 0034d08 Compare September 10, 2025 15:05

[compiler-v2][lint] Complex boolean expression checker #16721

Are you sure you want to change the base?

[compiler-v2][lint] Complex boolean expression checker #16721

Conversation

jgcrosta commented May 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Supported Simplification Patterns:

How Has This Been Tested?

Key Areas to Review

Type of Change

Which Components or Systems Does This Change Impact?

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jgcrosta commented Jun 4, 2025

Uh oh!

vineethk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

junxzm1990 Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jgcrosta commented Jun 12, 2025

Uh oh!

vineethk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jgcrosta commented Jun 17, 2025

Uh oh!

jgcrosta commented Jun 17, 2025

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Jul 1, 2025

❌ Forge suite compat failure on 20fcb95ffc2aedc2fd7fce2d955e6823b3a4e585 ==> 5b328b258b26dd6a3a9a3dd9f0454d4e270332cb

Uh oh!

This comment has been minimized.

junxzm1990 left a comment

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

github-actions bot commented Sep 3, 2025

✅ Forge suite realistic_env_max_load success on d80ea2a6d1fc7f5e6b1b62906621882aa1fc25f1

Uh oh!

Uh oh!

jgcrosta commented May 29, 2025 •

edited

Loading

junxzm1990 Jun 18, 2025 •

edited

Loading

❌ Forge suite `compat` failure on `20fcb95ffc2aedc2fd7fce2d955e6823b3a4e585` ==> `5b328b258b26dd6a3a9a3dd9f0454d4e270332cb`

✅ Forge suite `realistic_env_max_load` success on `d80ea2a6d1fc7f5e6b1b62906621882aa1fc25f1`