[compiler-v2] Set V2 as the default compiler #15408

rahxephon89 · 2024-11-26T20:44:16Z

Description

This PR:

sets v2 as the default compiler;
updates all related tests;
removes CI tests for compiler v2.

close #13869

Note:

will have a separate PR to remove all v2_exp files once exp files in this PR are manually verified.
also note that third_party/move/move-vm still uses V1 compiler. Will switch to V2 in a separate PR.

How Has This Been Tested?

All existing tests pass.

Key Areas to Review

Changes are scattered in the while aptos-core repo, need to carefully check whether these changes are correct and complete

Type of Change

Which Components or Systems Does This Change Impact?

Checklist

I have read and followed the CONTRIBUTING doc
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I identified and added all stakeholders and component owners affected by this change as reviewers
I tested both happy and unhappy path of the functionality
I have made corresponding changes to the documentation

trunk-io · 2024-11-26T20:44:19Z

⏱️ 1h 14m total CI duration on this PR

Job	Cumulative Duration	Recent Runs
rust-unit-tests	23m	🟩
rust-move-tests	13m	🟩
rust-move-tests	12m	🟩
rust-cargo-deny	7m	🟩 🟩 🟩 🟩
rust-move-tests	7m	⬜
rust-doc-tests	5m	🟩
check-dynamic-deps	3m	🟩 🟩 🟩
semgrep/ci	2m	🟩 🟩 🟩 🟩 🟩
general-lints	2m	🟩 🟩 🟩 🟩
file_change_determinator	42s	🟩 🟩 🟩 🟩
permission-check	9s	🟩 🟩 🟩
permission-check	7s	🟩 🟩 🟩
check-branch-prefix	1s	🟩 🟩

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

rahxephon89 · 2024-11-26T20:44:33Z

[compiler-v2] Set V2 as the default compiler #15408 👈 (View in Graphite)
main

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

vineethk

One way to make sure we are using v2 compiler everywhere (expect where we expect to use the v1 compiler, like in v1 compiler tests and v2 compiler transactional tests) is to set MVC_BLOCK_V1 in the CI tests?

vineethk · 2025-01-08T01:53:39Z

api/goldens/aptos_api__tests__state_test__test_get_account_module.json

@@ -1,5 +1,5 @@
 {
-  "bytecode": "0xa11ceb0b0700000a0c010002020208030a30053a23075d7b08d8012006f8010a1082029a010a9c030d0ca9037e0da704060fad040400020003060000040700000500010001000602030001000704050001000804060001000907080001000a04030001000b09050001000c090600010205070301080002050301080101060800010301050206080006080101010106080100076163636f756e74066f626a6563740467756964044755494402494406637265617465096372656174655f69640c6372656174696f6e5f6e756d0f63726561746f725f616464726573730565715f69640269640f69645f6372656174696f6e5f6e756d1269645f63726561746f725f616464726573730461646472000000000000000000000000000000000000000000000000000000000000000103080000000000000000126170746f733a3a6d657461646174615f763185010100000000000000001d45475549445f47454e455241544f525f4e4f545f5055424c49534845445b475549442067656e657261746f72206d757374206265207075626c6973686564206168656164206f66206669727374207573616765206f6620606372656174655f776974685f6361706162696c697479602066756e6374696f6e2e00000002010a080101020207030d0500030000050d0a01140c020a02060100000000000000160b01150b020b001201120002010100000a040b010b00120102020100000a050b00100010011402030100000a050b00100010021402040100000a050b0010000b012102050100000a040b0010001402060100000a040b0010011402070100000a040b00100214020000010001010000000100",
+  "bytecode": "0xa11ceb0b0700000a0b010002020208030a30053a23075d7b08d8012010f801b9010ab1030d0cbe037e0dbc04060fc2040400000001060000030700000600010001000703040001000406020001000806070001000908090001000206040001000a0a020001000b0a0700010205070301080001030205030108010001060800010502060800060801010101060801046775696404475549440269640249440c6372656174696f6e5f6e756d046164647206637265617465096372656174655f69640f63726561746f725f616464726573730565715f69640f69645f6372656174696f6e5f6e756d1269645f63726561746f725f61646472657373076163636f756e74066f626a656374000000000000000000000000000000000000000000000000000000000000000114636f6d70696c6174696f6e5f6d65746164617461090003322e3003322e31126170746f733a3a6d657461646174615f763185010100000000000000001d45475549445f47454e455241544f525f4e4f545f5055424c49534845445b475549442067656e657261746f72206d757374206265207075626c6973686564206168656164206f66206669727374207573616765206f6620606372656174655f776974685f6361706162696c697479602066756e6374696f6e2e00000002010208010102020403050500030000020d0a01140c020a02060100000000000000160b01150b020b0012011200020101000005040b010b001201020201000005050b001000100114020301000005050b001000100214020401000005050b0010000b0121020501000005040b00100014020601000005040b00100114020701000005040b0010021402000001000101000c000d00",


Perhaps this resolves #13869?

vineethk · 2025-01-08T02:06:21Z

aptos-move/aptos-transactional-test-harness/tests/aptos_test_harness/diamond_clicker.exp

@@ -1,76 +1,62 @@
 processed 4 tasks

 task 1 'print-bytecode'. lines 4-35:
-Error: extended checks failed:
+// Move bytecode v7


Could you provide a brief comment about this change in output due to compiler v2? I see that this change pre-exists (as seen from .v2_exp).

yes, we have a V1-only bug for this: #14722

vineethk · 2025-01-08T02:17:08Z

aptos-move/framework/aptos-framework/doc/aggregator_v2.md

@@ -82,23 +68,10 @@ read, read_snapshot, read_derived_string
    -  [Function `derive_string_concat`](#@Specification_1_derive_string_concat)
    -  [Function `copy_snapshot`](#@Specification_1_copy_snapshot)
    -  [Function `string_concat`](#@Specification_1_string_concat)
-    -  [Function `verify_aggregator_try_add_sub`](#@Specification_1_verify_aggregator_try_add_sub)


Do we have a bug open for doc generator, where there is a change in the behavior, when using v2, verify_only functions are skipped? Or perhaps this is intended behavior?

Good catch! It is not intended. Filed a bug here: #15687

vineethk · 2025-01-08T02:20:57Z

aptos-move/framework/tests/gate_v2_features.rs

@@ -28,26 +28,31 @@ fn compile_pkg_with_v1(path_to_pkg: impl Into<String>) {
 }

 #[test]
+#[ignore]


Do we want to remove this file altogether, instead of ignoring the tests?

vineethk · 2025-01-08T02:26:12Z

third_party/move/move-model/src/metadata.rs

-        } else {
-            Self::V1
-        }
+        Self::latest_stable()


I wonder if it is still useful to allow setting the compiler version to 1 via env variable (to handle those scenarios where --move-1 is not applicable).

vineethk · 2025-01-08T02:32:20Z

third_party/move/testing-infra/transactional-test-runner/tests/vm_test_harness/example.exp

+   │         ^^^^^^^^^^^^^^
+
+
+
 task 7 'view'. lines 47-49:


Could you check if these differences in output are acceptable and why? Seems like the v2 output was pre-existing.

There are two "A::N" published in this test. The testing infra puts these two modules into two separate temp files and when publishing the latter one, the previous one is added as a dependency. Compiler V2 then generates an compilation error of duplicated modules while V1 does not.
I commented out the first publish command and the updated exp file matches with the V1 version.

rahxephon89 force-pushed the teng/use-v2-as-default-in-rust-unit-tests branch 2 times, most recently from db094c9 to 2c9bf22 Compare December 2, 2024 19:09

rahxephon89 force-pushed the teng/use-v2-as-default-in-rust-unit-tests branch 2 times, most recently from 9eaa4ab to 4826561 Compare December 16, 2024 00:02

rahxephon89 changed the title ~~[test-only] set v2 flag for unit tests~~ [test-only] Set V2 as the default compiler Dec 16, 2024

rahxephon89 force-pushed the teng/use-v2-as-default-in-rust-unit-tests branch 2 times, most recently from 94b157a to d6cfb28 Compare December 16, 2024 17:30

rahxephon89 force-pushed the teng/use-v2-as-default-in-rust-unit-tests branch 10 times, most recently from e51881b to 353e020 Compare January 7, 2025 05:06

rahxephon89 changed the title ~~[test-only] Set V2 as the default compiler~~ [compiler-v2] Set V2 as the default compiler Jan 7, 2025

rahxephon89 force-pushed the teng/use-v2-as-default-in-rust-unit-tests branch 3 times, most recently from f7d532a to 9cfa04d Compare January 7, 2025 06:20

rahxephon89 marked this pull request as ready for review January 7, 2025 18:18

rahxephon89 requested review from davidiw, movekevin, wrwg, banool, gregnazario and 0xmaayan as code owners January 7, 2025 18:18

rahxephon89 requested a review from a team as a code owner January 7, 2025 18:18

rahxephon89 requested review from vineethk, igor-aptos and runtian-zhou January 7, 2025 18:18

vineethk reviewed Jan 8, 2025

View reviewed changes

rahxephon89 added 5 commits January 7, 2025 20:14

[test-only] set v2 flag for unit tests

4dfba2b

remove use of v2_exp and v2 environment variable

939f4d5

update move-abi and doc-gen test using v2

b50ee82

remove ci test for compiler v2

d7fdad6

remove gate_v2_features.rs

ef46774

rahxephon89 force-pushed the teng/use-v2-as-default-in-rust-unit-tests branch from 9cfa04d to ef46774 Compare January 8, 2025 04:35

fix test

ad1a49e

rahxephon89 force-pushed the teng/use-v2-as-default-in-rust-unit-tests branch 2 times, most recently from 70a7828 to 4fb0116 Compare January 8, 2025 07:29

vineethk approved these changes Jan 8, 2025

View reviewed changes

test v1

75d1065

rahxephon89 force-pushed the teng/use-v2-as-default-in-rust-unit-tests branch from 4fb0116 to 75d1065 Compare January 8, 2025 16:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[compiler-v2] Set V2 as the default compiler #15408

[compiler-v2] Set V2 as the default compiler #15408

rahxephon89 commented Nov 26, 2024 •

edited

Loading

trunk-io bot commented Nov 26, 2024 •

edited

Loading

rahxephon89 commented Nov 26, 2024 •

edited

Loading

vineethk left a comment

vineethk Jan 8, 2025

vineethk Jan 8, 2025

rahxephon89 Jan 8, 2025

vineethk Jan 8, 2025

rahxephon89 Jan 8, 2025

vineethk Jan 8, 2025

vineethk Jan 8, 2025

vineethk Jan 8, 2025

rahxephon89 Jan 8, 2025

[compiler-v2] Set V2 as the default compiler #15408

Are you sure you want to change the base?

[compiler-v2] Set V2 as the default compiler #15408

Conversation

rahxephon89 commented Nov 26, 2024 • edited Loading

Description

How Has This Been Tested?

Key Areas to Review

Type of Change

Which Components or Systems Does This Change Impact?

Checklist

trunk-io bot commented Nov 26, 2024 • edited Loading

rahxephon89 commented Nov 26, 2024 • edited Loading

vineethk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rahxephon89 commented Nov 26, 2024 •

edited

Loading

trunk-io bot commented Nov 26, 2024 •

edited

Loading

rahxephon89 commented Nov 26, 2024 •

edited

Loading