[compiler-v2][lint] Needless mutable reference checker

[vineethk]

Description

This PR implements a new lint checker (at the stackless bytecode level) that looks for &mut and borrow_global_mut that are needless, and suggests to instead use & and borrow_global instead.

This lint finds several violations on the aptos framework, and I have verified them to be true positives.

Type of Change

• New feature

Which Components or Systems Does This Change Impact?

• Move/Aptos Virtual Machine

How Has This Been Tested?

• Added manual tests
• Existing tests are updated as needed

Key Areas to Review

• The lint checker logic.

[trunk-io]

⏱️ 8h 31m total CI duration on this PR

Job	Cumulative Duration	Recent Runs
execution-performance / single-node-performance	1h 6m	🟩 🟩 🟩
forge-compat-test / forge	59m	🟩 🟩 🟩
forge-e2e-test / forge	41m	🟩 🟩 🟩
forge-framework-upgrade-test / forge	41m	🟥 🟩 🟩
rust-move-unit-coverage	19m	🟩
general-lints	18m	🟩 ⬜ 🟩 🟩 🟩 (+5 more)
rust-cargo-deny	16m	🟩 ⬜ 🟩 🟩 🟩 (+5 more)
rust-move-unit-coverage	16m	🟩
rust-move-unit-coverage	16m	🟩
rust-move-unit-coverage	16m	🟩
test-target-determinator	15m	🟩 🟩 🟩
rust-move-unit-coverage	14m	🟩
rust-move-unit-coverage	14m	🟩
rust-move-unit-coverage	14m	🟩
execution-performance / test-target-determinator	13m	🟩 🟩 🟩
rust-move-unit-coverage	11m	🟩
check	11m	🟩 🟩 🟩
rust-move-tests	10m	🟩
rust-move-tests	10m	🟩
rust-move-tests	10m	🟩
rust-move-tests	10m	🟩
rust-move-tests	9m	🟩
rust-move-tests	9m	🟩
rust-move-tests	9m	🟩
rust-move-tests	9m	🟩
check-dynamic-deps	8m	🟩 🟩 🟩 🟩 🟩 (+5 more)
rust-doc-tests	5m	🟩
rust-doc-tests	5m	🟩
rust-doc-tests	5m	🟩
semgrep/ci	4m	🟩 🟩 🟩 🟩 🟩 (+5 more)
file_change_determinator	2m	🟩 🟩 🟩 🟩 🟩 (+5 more)
file_change_determinator	2m	🟩 🟩 🟩 🟩 🟩 (+5 more)
permission-check	38s	🟩 🟩 🟩 🟩 🟩 (+5 more)
file_change_determinator	36s	🟩 🟩 🟩
permission-check	32s	🟩 🟩 🟩 🟩 🟩 (+5 more)
permission-check	28s	🟩 🟩 🟩 🟩 🟩 (+5 more)
permission-check	27s	🟩 🟩 🟩 🟩 🟩 (+5 more)
rust-move-tests	16s	⬜
permission-check	7s	🟩 🟩 🟩
determine-docker-build-metadata	7s	🟩 🟩 🟩
rust-move-tests	6s	⬜
Backport PR	4s	🟥
permission-check	2s	🟩
rust-move-unit-coverage	1s	⬜

🚨 1 job on the last run was significantly faster/slower than expected

Job	Duration	vs 7d avg	Delta
forge-compat-test / forge	23m	17m

_{settings ⋅ feedback ⋅ docs ⋅ learn more about trunk.io}

[vineethk]

• [aptos-framework] Fix needless mutable references and borrows #14656
• [compiler-v2][lint] Needless mutable reference checker #14651 👈
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @vineethk and the rest of your teammates on Graphite

[codecov]

Codecov Report

Attention: Patch coverage is 99.27536% with 1 line in your changes missing coverage. Please review.

Project coverage is 59.8%. Comparing base (0a9d654) to head (e39b231).
Report is 8 commits behind head on main.

Files with missing lines	Patch %	Lines
...eline/lint_processor/needless_mutable_reference.rs	99.2%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##             main   #14651    +/-   ##
========================================
  Coverage    59.7%    59.8%            
========================================
  Files         850      851     +1     
  Lines      207109   207246   +137     
========================================
+ Hits       123845   123982   +137     
  Misses      83264    83264            

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[brmataptos]

Looks generally good, but could use a few more tests of other data types than u64.

[vineethk]

• [aptos-framework] Fix needless mutable references and borrows #14656
• [compiler-v2][lint] Needless mutable reference checker #14651 👈
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @vineethk and the rest of your teammates on Graphite

[vineethk]

• [aptos-framework] Fix needless mutable references and borrows #14656
• [compiler-v2][lint] Needless mutable reference checker #14651 👈
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @vineethk and the rest of your teammates on Graphite

[brmataptos]

Generally looks good, except that lint warnings should be sorted so that they match line order. That may be just in the test framework, but for real use cases you need to fix it.

[vineethk]

• [aptos-framework] Fix needless mutable references and borrows #14656
• [compiler-v2][lint] Needless mutable reference checker #14651 👈
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @vineethk and the rest of your teammates on Graphite

[github-actions]

✅ Forge suite realistic_env_max_load success on e39b231b55a4be05e95a05323de4d4271a6d8481

two traffics test: inner traffic : committed: 14545.17 txn/s, latency: 2730.79 ms, (p50: 2700 ms, p70: 2700, p90: 2900 ms, p99: 3000 ms), latency samples: 5530460
two traffics test : committed: 100.03 txn/s, latency: 1770.69 ms, (p50: 1500 ms, p70: 1600, p90: 1700 ms, p99: 11900 ms), latency samples: 1800
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.252, avg: 0.221", "QsPosToProposal: max: 1.076, avg: 1.053", "ConsensusProposalToOrdered: max: 0.321, avg: 0.290", "ConsensusOrderedToCommit: max: 0.418, avg: 0.403", "ConsensusProposalToCommit: max: 0.706, avg: 0.693"]
Max non-epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 0.94s no progress at version 2445137 (avg 0.20s) [limit 15].
Max epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 8.85s no progress at version 2445135 (avg 8.85s) [limit 15].
Test Ok

• Grafana dashboard
• Humio Logs
• Axiom Logs
• Validator CPU Profile
• Fullnode CPU Profile
• Test runner output
• Test run is land-blocking

[github-actions]

✅ Forge suite framework_upgrade success on 25a081116546670e62ca927ba90478de78557056 ==> e39b231b55a4be05e95a05323de4d4271a6d8481

Compatibility test results for 25a081116546670e62ca927ba90478de78557056 ==> e39b231b55a4be05e95a05323de4d4271a6d8481 (PR)
Upgrade the nodes to version: e39b231b55a4be05e95a05323de4d4271a6d8481
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 978.06 txn/s, submitted: 979.88 txn/s, failed submission: 1.82 txn/s, expired: 1.82 txn/s, latency: 3284.56 ms, (p50: 2400 ms, p70: 3900, p90: 6400 ms, p99: 8200 ms), latency samples: 86060
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 869.57 txn/s, submitted: 871.80 txn/s, failed submission: 2.23 txn/s, expired: 2.23 txn/s, latency: 3624.22 ms, (p50: 2700 ms, p70: 4500, p90: 7200 ms, p99: 9000 ms), latency samples: 78080
5. check swarm health
Compatibility test for 25a081116546670e62ca927ba90478de78557056 ==> e39b231b55a4be05e95a05323de4d4271a6d8481 passed
Upgrade the remaining nodes to version: e39b231b55a4be05e95a05323de4d4271a6d8481
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 294.90 txn/s, submitted: 295.12 txn/s, failed submission: 0.22 txn/s, expired: 0.22 txn/s, latency: 8417.87 ms, (p50: 2700 ms, p70: 15400, p90: 20800 ms, p99: 23500 ms), latency samples: 26520
Test Ok

• Grafana dashboard
• Humio Logs
• Axiom Logs
• Validator CPU Profile
• Fullnode CPU Profile
• Test runner output
• Test run is land-blocking

[github-actions]

✅ Forge suite compat success on 25a081116546670e62ca927ba90478de78557056 ==> e39b231b55a4be05e95a05323de4d4271a6d8481

Compatibility test results for 25a081116546670e62ca927ba90478de78557056 ==> e39b231b55a4be05e95a05323de4d4271a6d8481 (PR)
1. Check liveness of validators at old version: 25a081116546670e62ca927ba90478de78557056
compatibility::simple-validator-upgrade::liveness-check : committed: 13916.16 txn/s, latency: 2157.50 ms, (p50: 1900 ms, p70: 2100, p90: 2500 ms, p99: 7800 ms), latency samples: 535620
2. Upgrading first Validator to new version: e39b231b55a4be05e95a05323de4d4271a6d8481
compatibility::simple-validator-upgrade::single-validator-upgrading : committed: 7436.47 txn/s, latency: 3817.08 ms, (p50: 4300 ms, p70: 4500, p90: 4600 ms, p99: 4700 ms), latency samples: 139340
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 6614.77 txn/s, latency: 4543.27 ms, (p50: 4600 ms, p70: 4700, p90: 4800 ms, p99: 6600 ms), latency samples: 251200
3. Upgrading rest of first batch to new version: e39b231b55a4be05e95a05323de4d4271a6d8481
compatibility::simple-validator-upgrade::half-validator-upgrading : committed: 7230.11 txn/s, latency: 3753.38 ms, (p50: 4100 ms, p70: 4500, p90: 5100 ms, p99: 5300 ms), latency samples: 129520
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 7582.97 txn/s, latency: 4212.09 ms, (p50: 4300 ms, p70: 4500, p90: 6300 ms, p99: 6500 ms), latency samples: 251620
4. upgrading second batch to new version: e39b231b55a4be05e95a05323de4d4271a6d8481
compatibility::simple-validator-upgrade::rest-validator-upgrading : committed: 10201.04 txn/s, latency: 2721.73 ms, (p50: 2700 ms, p70: 2800, p90: 4300 ms, p99: 4600 ms), latency samples: 198820
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 5145.51 txn/s, submitted: 5145.85 txn/s, expired: 0.33 txn/s, latency: 2947.39 ms, (p50: 2700 ms, p70: 2800, p90: 5600 ms, p99: 6900 ms), latency samples: 340558
5. check swarm health
Compatibility test for 25a081116546670e62ca927ba90478de78557056 ==> e39b231b55a4be05e95a05323de4d4271a6d8481 passed
Test Ok

• Grafana dashboard
• Humio Logs
• Axiom Logs
• Validator CPU Profile
• Fullnode CPU Profile
• Test runner output
• Test run is land-blocking