[Bug][compiler-v2] Compiler erroneously reports unused assignments in test

[alnoki]

@brmataptos @fEst1ck @georgemitenkov @gregnazario @rahxephon89 @runtian-zhou @vineethk @wrwg

Steps to reproduce

See this file snippet:
https://github.com/econia-labs/emojicoin-dot-fun/blob/4bae268f/src/move/emojicoin_arena/tests/tests.move#L1739-L1752

Then from in src/move/emojicoin_arena:

git ls-files | entr -c sh -c " \
       aptos move test --coverage --dev --move-2  &&
       aptos move fmt
   "

If I don't call the variables called out in the snippet, then the compiler erroneously reports an unused assignment even though it is used in the test:

warning: Unused assignment to `covered_unequal_market_ids`. Consider removing or prefixing with an underscore: `_covered_unequal_market_ids`
     ┌─ /Users/yours-truly/repos/emojicoin-dot-fun/src/move/emojicoin_arena/tests/tests.move:1741:42
     │
1741 │         let covered_unequal_market_ids = false;
     │   

However if I then delete the line, or prefix with _ as suggested, the compiler then reports:

error: undeclared `covered_unequal_market_ids`
     ┌─ /Users/yours-truly/repos/emojicoin-dot-fun/src/move/emojicoin_arena/tests/tests.move:1758:17
     │
1758 │                 covered_unequal_market_ids = true;
     │                 ^^^^^^^^^^^^^^^^^^^^^^^^^^

error: undeclared `covered_unequal_market_ids`
     ┌─ /Users/yours-truly/repos/emojicoin-dot-fun/src/move/emojicoin_arena/tests/tests.move:1783:17
     │
1783 │         assert!(covered_unequal_market_ids);
     │

[vineethk]

@alnoki

The compiler is indeed correct in reporting the unused assignment here.

Below is a much more simplified version of the code in question, having deleted a lot of irrelevant code and added comments.

let covered_unequal_market_ids = false; // unused assignment

loop {
    // Get two random market IDs.
    let market_id_0 = random_market_id();
    let market_id_1 = random_market_id();

    // Check if they are equal, restarting loop as needed, setting coverage condition.
    if (market_id_0 == market_id_1) {
        covered_equal_market_ids = true;
        continue;
    } else {
        covered_unequal_market_ids = true;
    };

    // Some other code that eventually gets us out of the loop.
    // To get here, we must have executed the `else` block above.
};

assert!(covered_unequal_market_ids); // only `= true` reaches here.

Note that the only way to get out of the loop is to have executed the else block, which sets covered_unequal_market_ids to true. The only use of covered_unequal_market_ids is in the assert!, and only the true assignment can reach it, thus, the false assignment at the start is an "unused assignment".

The way to fix the warning correctly is to remove the assignment, but keep the declaration, because it is indeed used (i.e., let covered_unequal_market_ids; instead of let covered_unequal_market_ids = false;).

I'll follow up with a minor fix to the warning message that says "Consider removing the assignment, or prefixing with an underscore: ...". The existing "consider removing" may be perceived as suggesting removing the declaration and the assignment altogether.

[alnoki]

@alnoki

The compiler is indeed correct in reporting the unused assignment here.

Below is a much more simplified version of the code in question, having deleted a lot of irrelevant code and added comments.

let covered_unequal_market_ids = false; // unused assignment

loop {
    // Get two random market IDs.
    let market_id_0 = random_market_id();
    let market_id_1 = random_market_id();

    // Check if they are equal, restarting loop as needed, setting coverage condition.
    if (market_id_0 == market_id_1) {
        covered_equal_market_ids = true;
        continue;
    } else {
        covered_unequal_market_ids = true;
    };

    // Some other code that eventually gets us out of the loop.
    // To get here, we must have executed the `else` block above.
};

assert!(covered_unequal_market_ids); // only `= true` reaches here.

Note that the only way to get out of the loop is to have executed the else block, which sets covered_unequal_market_ids to true. The only use of covered_unequal_market_ids is in the assert!, and only the true assignment can reach it, thus, the false assignment at the start is an "unused assignment".

The way to fix the warning correctly is to remove the assignment, but keep the declaration, because it is indeed used (i.e., let covered_unequal_market_ids; instead of let covered_unequal_market_ids = false;).

I'll follow up with a minor fix to the warning message that says "Consider removing the assignment, or prefixing with an underscore: ...". The existing "consider removing" may be perceived as suggesting removing the declaration and the assignment altogether.

@vineethk Thanks! Addressed in econia-labs/emojicoin-dot-fun#632