Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[compiler-v2] Pattern matching for struct variants (aka enum types) #13725

Merged
merged 2 commits into from
Jun 24, 2024

Conversation

wrwg
Copy link
Contributor

@wrwg wrwg commented Jun 17, 2024

Description

This implements the Move 2 match expression on stackless bytecode level. The PR consists of the following parts:

  1. Extending the Bytecode

There are a few new bytecode operations:

dest := TestVariant(struct_id, variant, src)
dest := PackVariant(struct_id, variant, srcs)
dests := UnpackVariant(struct_id, variant, src)
dest := BorrowFieldVariant(struct_id, variant, field, src)

The TestVariant and BorrowFieldVariant operations expect references to the variant struct. The UnpackVariant and BorrowFieldVariant operations abort if the src value is not a value of the given variant.

Notably, there are no new control flow operations, and the remaining part of the stackless bytecode framework should operate without changes. This may change in the future if we introduce a new branch instruction for switching over variants.

  1. Translating Matches

Matches are translated into cascades of test & branch instructions. The translation is complicated by the need for 'probing' whether a value can be matched before it is consumed. See for discussion in the code how this problem is solved.

The current translation is manually reviewed via the baseline output, but it will not be fully tested before we are able to run e2e tests with VM execution. (Future PRs.)

  1. Checking Match Coverage

Even though we generate robust match code which is resilient against addition of new match variants by explicit abort if there is no match, at compile time complete match coverage is required. Checking for this is non-trivial because of the sequential, imperative semantics of matching. See documentation in the code.

  1. Other

There are some only indirectly related changes in this PR. To implement non-destructive matches with conditions, it appeared handy to have *&x to be equivalent to x. This seems to be general useful and is AFAICT also in Rust.

Also, the new pattern translation is a bit more efficient also in the let case, leading to some changes in baseline files.

Type of Change

  • New feature
  • Bug fix
  • Breaking change
  • Performance improvement
  • Refactoring
  • Dependency update
  • Documentation update
  • Tests

Which Components or Systems Does This Change Impact?

  • Validator Node
  • Full Node (API, Indexer, etc.)
  • Move/Aptos Virtual Machine
  • Aptos Framework
  • Aptos CLI/SDK
  • Developer Infrastructure
  • Other (specify)

Copy link

trunk-io bot commented Jun 17, 2024

⏱️ 12h 59m total CI duration on this PR
Job Cumulative Duration Recent Runs
rust-targeted-unit-tests 3h 🟥🟥🟥🟩 (+3 more)
test-fuzzers 2h 24m 🟩🟩🟩🟩
rust-move-tests 1h 44m 🟩🟩🟩🟩🟩 (+3 more)
rust-move-unit-coverage 1h 22m 🟩🟩🟩🟩🟩 (+2 more)
rust-lints 49m 🟩🟩🟩🟩🟩 (+3 more)
run-tests-main-branch 40m 🟩🟩🟩🟩🟩 (+3 more)
rust-smoke-tests 33m 🟩
forge-e2e-test / forge 27m 🟥🟩
execution-performance / single-node-performance 22m 🟩
general-lints 15m 🟩🟩🟩🟩🟩 (+3 more)
forge-framework-upgrade-test / forge 14m 🟩
rust-images / rust-all 12m 🟩
forge-compat-test / forge 11m 🟩
check-dynamic-deps 8m 🟩🟩🟩🟩🟩 (+3 more)
rust-build-cached-packages 8m 🟩
cli-e2e-tests / run-cli-tests 7m 🟩
test-target-determinator 7m 🟩
execution-performance / test-target-determinator 5m 🟩
check 4m 🟩
semgrep/ci 3m 🟩🟩🟩🟩🟩 (+3 more)
file_change_determinator 1m 🟩🟩🟩🟩🟩 (+3 more)
file_change_determinator 1m 🟩🟩🟩🟩🟩 (+3 more)
node-api-compatibility-tests / node-api-compatibility-tests 1m 🟩
permission-check 23s 🟩🟩🟩🟩🟩 (+3 more)
permission-check 22s 🟩🟩🟩🟩🟩 (+3 more)
permission-check 21s 🟩🟩🟩🟩🟩 (+3 more)
permission-check 20s 🟩🟩🟩🟩🟩 (+3 more)
file_change_determinator 11s 🟩
permission-check 3s 🟩
determine-docker-build-metadata 2s 🟩

🚨 4 jobs on the last run were significantly faster/slower than expected

Job Duration vs 7d avg Delta
rust-targeted-unit-tests 27m 13m +112%
rust-build-cached-packages 8m 5m +56%
test-target-determinator 7m 4m +51%
rust-move-unit-coverage 14m 12m +22%

settingsfeedbackdocs ⋅ learn more about trunk.io

Copy link

codecov bot commented Jun 17, 2024

Codecov Report

Attention: Patch coverage is 95.02924% with 34 lines in your changes missing coverage. Please review.

Project coverage is 59.0%. Comparing base (153133b) to head (1adf5cb).
Report is 1 commits behind head on main.

Files Patch % Lines
...ty/move/move-compiler-v2/src/bytecode_generator.rs 94.8% 31 Missing ⚠️
...e-prover/boogie-backend/src/bytecode_translator.rs 0.0% 2 Missing ⚠️
third_party/move/evm/move-to-yul/src/functions.rs 0.0% 1 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (153133b) and HEAD (1adf5cb). Click for more details.

HEAD has 1 upload less than BASE | Flag | BASE (153133b) | HEAD (1adf5cb) | |------|------|------| ||2|1|
Additional details and impacted files
@@             Coverage Diff             @@
##             main   #13725       +/-   ##
===========================================
- Coverage    71.1%    59.0%    -12.2%     
===========================================
  Files        2314      817     -1497     
  Lines      454222   197108   -257114     
===========================================
- Hits       323278   116316   -206962     
+ Misses     130944    80792    -50152     

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

@wrwg wrwg force-pushed the wrwg/enum-match branch 2 times, most recently from 5202f3b to 91e3ff3 Compare June 18, 2024 05:10
@wrwg wrwg force-pushed the wrwg/enum branch 3 times, most recently from 236b742 to 2aa5ac5 Compare June 19, 2024 22:11
Base automatically changed from wrwg/enum to main June 19, 2024 23:44
@wrwg wrwg force-pushed the wrwg/enum-match branch from 91e3ff3 to 1cb5772 Compare June 20, 2024 04:38
@wrwg wrwg force-pushed the wrwg/enum-match branch from 1cb5772 to 5ba2912 Compare June 20, 2024 19:05
@wrwg wrwg requested a review from rahxephon89 June 20, 2024 19:14
x: u64
}

enum Outer {
Copy link
Contributor

@rahxephon89 rahxephon89 Jun 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add tests where enum types are attached with abilities?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also some with generic types.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both done in matching_ok, specifically the case where we match over instantiated generics themselves. Notice that there are already negative ability related tests in matching_ability_err, but I also added some positive ones.

/// Consider `let s = S{x: C{y}}; match (s) { S{C{y}}} if p(&y) => y, t => t }`. In order
/// to check whether the match is true, the value in `s` must not be moved until the match
/// is decided, while still being able to look at sub-fields and evaluate predicates.
/// Therefore, the match need to evaluated first using a reference to `s`. We call this
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: "need to evaluated" => "needs to be evaluated"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

// the expression is not a tuple, a singleton vector will be returned. This
// reflects the current special semantics of tuples in Move which cannot be
// nested.
fn gen_tuple(&mut self, exp: &Exp, with_force_temp: bool) -> Vec<TempIndex> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fn gen_tuple(&mut self, exp: &Exp, with_force_temp: bool) -> Vec<TempIndex> {
fn gen_tuple(&mut self, exp: &Exp, with_forced_temp: bool) -> Vec<TempIndex> {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to keep it consistent with other similar uses in this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@vineethk vineethk self-requested a review June 21, 2024 15:26
Copy link
Contributor

@vineethk vineethk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good. Given that my comments are minor, I am approving the PR assuming the comments will be considered.

exit_path: Label,
/// Set if this is a probing match, and if so, the set of vars which need to
/// be bound for the match condition. In probing mode, we only need to
/// bind certain variables. For example, in `S{x, y} if p(y)` we do not need
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would y be a probing var in both these cases: p(y) and p(&y)? I ask because the example at the top is about p(&y) and the one here is p(y).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This depends, if y is not copyable, one better uses &y, but the logic described here is independent of this.

enum ValueShape {
Any,
Tuple(Vec<ValueShape>),
Struct(QualifiedId<StructId>, Option<Symbol>, Vec<ValueShape>),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be helpful to add some notes about this variant's fields.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

/// which show the counterexamples of missed patterns, but only up to the shape as the user
/// has inspected values via patterns. For example, if the user has written `R{S{_}, _}`
/// the counter example should not contain irrelevant other parts of the value (as e.g.
/// in `R{T{x:_}, S}`). Moreover, missing patterns which can never be matched should
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"missing patterns which can never be matched" is a bit confusing => if there is a missing pattern (there exists atleast one value being matched that does not fit any match arm), it is an error, but there is no dead code; but if one pattern represents the superset of the other and appears first, then the other arm can never be matched (and therefore we have dead code). Would be helpful to clarify this in comments.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

/// by lowest 6 bytes of a sha256 of the string "Move 2 Abort Code",
/// appended with two bytes for the error type.
pub const WELL_KNOWN_ABORT_CODE_BASE: u64 = 0xD8CA26CBD9BE << 16;
pub const INCOMPLETE_MATCH_ABORT_CODE: u64 = WELL_KNOWN_ABORT_CODE_BASE | 0x1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: We allow user to also directly abort with this code (using abort, or assert)? Can this cause any issues (by using the abort codes in the user space)? As far as I understand, arithmetic errors, for example, are always distinguishable as an outcome from user aborts; should we have the same here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@well, I would consider the probability of a clash extremely unlikely. In any case we need this or extend the VM and storage to add more info for abort code (storage breaking change for on-chain state). We can detect though at runtime if user issues reserved abort codes. I added a TODO.

@@ -159,6 +160,12 @@ pub enum Operation {
MoveFrom(ModuleId, StructId, Vec<Type>),
Exists(ModuleId, StructId, Vec<Type>),

// Variants
TestVariant(ModuleId, StructId, Symbol, Vec<Type>),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be helpful to have documentation for the fields.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

x: u64
}

enum Outer {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also some with generic types.

wrwg added 2 commits June 23, 2024 19:03
…evel. The PR consists of the following parts:

1. *Extending the Bytecode*

There are a few new bytecode operations:

```
dest := TestVariant(struct_id, variant, src)
dest := PackVariant(struct_id, variant, srcs)
dests := UnpackVariant(struct_id, variant, src)
dest := BorrowFieldVariant(struct_id, variant, field, src)
```

The `TestVariant` and `BorrowFieldVariant` operations  expect references to the variant struct. The `UnpackVariant` and `BorrowFieldVariant` operations abort if the `src` value is not a value of the given variant.

Notably, there are no new control flow operations, and the remaining part of the stackless bytecode framework should operate without changes. This may change in the future if we introduce a new branch instruction for switching over variants.

2. *Translating Matches*

Matches are translated into cascades of test & branch instructions. The translation is complicated by the need for 'probing' whether a value can be matched before it is consumed. See for discussion in the code how this problem is solved.

The current translation is manually reviewed via the baseline output, but it will not be fully tested before we are able to run e2e tests with VM execution. (Future PRs.)

3. *Checking Match Coverage*

Even though we generate robust match code which is resilient against addition of new match variants by explicit `abort` if there is no match, at compile time complete match coverage is required. Checking for this is non-trivial because of the sequential, imperative semantics of matching. See documentation in the code.

4. *Other*

There are some only indirectly related changes in this PR. To implement non-destructive matches with conditions, it appeared handy to have `*&x` to be equivalent to `x`. This seems to be general useful and is AFAICT also in Rust.
Also, the new pattern translation is a bit more efficient also in the `let` case, leading to some changes in baseline files.
@wrwg wrwg force-pushed the wrwg/enum-match branch from 5ba2912 to 1adf5cb Compare June 24, 2024 02:03
Copy link
Contributor Author

@wrwg wrwg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the reviews, @rahxephon89 PTAL

// the expression is not a tuple, a singleton vector will be returned. This
// reflects the current special semantics of tuples in Move which cannot be
// nested.
fn gen_tuple(&mut self, exp: &Exp, with_force_temp: bool) -> Vec<TempIndex> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

/// Consider `let s = S{x: C{y}}; match (s) { S{C{y}}} if p(&y) => y, t => t }`. In order
/// to check whether the match is true, the value in `s` must not be moved until the match
/// is decided, while still being able to look at sub-fields and evaluate predicates.
/// Therefore, the match need to evaluated first using a reference to `s`. We call this
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

exit_path: Label,
/// Set if this is a probing match, and if so, the set of vars which need to
/// be bound for the match condition. In probing mode, we only need to
/// bind certain variables. For example, in `S{x, y} if p(y)` we do not need
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This depends, if y is not copyable, one better uses &y, but the logic described here is independent of this.

/// which show the counterexamples of missed patterns, but only up to the shape as the user
/// has inspected values via patterns. For example, if the user has written `R{S{_}, _}`
/// the counter example should not contain irrelevant other parts of the value (as e.g.
/// in `R{T{x:_}, S}`). Moreover, missing patterns which can never be matched should
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

enum ValueShape {
Any,
Tuple(Vec<ValueShape>),
Struct(QualifiedId<StructId>, Option<Symbol>, Vec<ValueShape>),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

x: u64
}

enum Outer {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both done in matching_ok, specifically the case where we match over instantiated generics themselves. Notice that there are already negative ability related tests in matching_ability_err, but I also added some positive ones.

@@ -159,6 +160,12 @@ pub enum Operation {
MoveFrom(ModuleId, StructId, Vec<Type>),
Exists(ModuleId, StructId, Vec<Type>),

// Variants
TestVariant(ModuleId, StructId, Symbol, Vec<Type>),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

/// by lowest 6 bytes of a sha256 of the string "Move 2 Abort Code",
/// appended with two bytes for the error type.
pub const WELL_KNOWN_ABORT_CODE_BASE: u64 = 0xD8CA26CBD9BE << 16;
pub const INCOMPLETE_MATCH_ABORT_CODE: u64 = WELL_KNOWN_ABORT_CODE_BASE | 0x1;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@well, I would consider the probability of a clash extremely unlikely. In any case we need this or extend the VM and storage to add more info for abort code (storage breaking change for on-chain state). We can detect though at runtime if user issues reserved abort codes. I added a TODO.

@wrwg wrwg requested a review from rahxephon89 June 24, 2024 02:04
@wrwg wrwg enabled auto-merge (squash) June 24, 2024 02:30

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

✅ Forge suite compat success on f648076a280621dbfd4e73b1ca83e3a3f52878ed ==> 1adf5cb829e788723ecbfdd51033a2334e903217

Compatibility test results for f648076a280621dbfd4e73b1ca83e3a3f52878ed ==> 1adf5cb829e788723ecbfdd51033a2334e903217 (PR)
1. Check liveness of validators at old version: f648076a280621dbfd4e73b1ca83e3a3f52878ed
compatibility::simple-validator-upgrade::liveness-check : committed: 9573.176967616942 txn/s, latency: 3046.8457742782152 ms, (p50: 2500 ms, p90: 3300 ms, p99: 14000 ms), latency samples: 381000
2. Upgrading first Validator to new version: 1adf5cb829e788723ecbfdd51033a2334e903217
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 3386.2749095660133 txn/s, latency: 9142.673458778738 ms, (p50: 9400 ms, p90: 14100 ms, p99: 14500 ms), latency samples: 136580
3. Upgrading rest of first batch to new version: 1adf5cb829e788723ecbfdd51033a2334e903217
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 2979.085860489408 txn/s, latency: 10385.021324363754 ms, (p50: 10900 ms, p90: 13900 ms, p99: 14200 ms), latency samples: 123380
4. upgrading second batch to new version: 1adf5cb829e788723ecbfdd51033a2334e903217
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 3044.318980373989 txn/s, latency: 10478.936306205871 ms, (p50: 7000 ms, p90: 19200 ms, p99: 20500 ms), latency samples: 107640
5. check swarm health
Compatibility test for f648076a280621dbfd4e73b1ca83e3a3f52878ed ==> 1adf5cb829e788723ecbfdd51033a2334e903217 passed
Test Ok

This comment has been minimized.

Copy link
Contributor

✅ Forge suite framework_upgrade success on f648076a280621dbfd4e73b1ca83e3a3f52878ed ==> 1adf5cb829e788723ecbfdd51033a2334e903217

Compatibility test results for f648076a280621dbfd4e73b1ca83e3a3f52878ed ==> 1adf5cb829e788723ecbfdd51033a2334e903217 (PR)
Upgrade the nodes to version: 1adf5cb829e788723ecbfdd51033a2334e903217
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1197.8095452100856 txn/s, submitted: 1200.8069511942495 txn/s, failed submission: 2.997405984163833 txn/s, expired: 2.997405984163833 txn/s, latency: 2533.6346102021175 ms, (p50: 1800 ms, p90: 4500 ms, p99: 9000 ms), latency samples: 103900
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1162.3454518760805 txn/s, submitted: 1165.5544513488205 txn/s, failed submission: 3.208999472740116 txn/s, expired: 3.208999472740116 txn/s, latency: 2609.382804180635 ms, (p50: 2100 ms, p90: 4800 ms, p99: 7500 ms), latency samples: 101420
5. check swarm health
Compatibility test for f648076a280621dbfd4e73b1ca83e3a3f52878ed ==> 1adf5cb829e788723ecbfdd51033a2334e903217 passed
Upgrade the remaining nodes to version: 1adf5cb829e788723ecbfdd51033a2334e903217
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1164.8984653887396 txn/s, submitted: 1167.3490071977426 txn/s, failed submission: 2.4505418090028948 txn/s, expired: 2.4505418090028948 txn/s, latency: 2698.1215528781795 ms, (p50: 2100 ms, p90: 4800 ms, p99: 8100 ms), latency samples: 104580
Test Ok

Copy link
Contributor

@rahxephon89 rahxephon89 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

This comment has been minimized.

Copy link
Contributor

✅ Forge suite realistic_env_max_load success on 1adf5cb829e788723ecbfdd51033a2334e903217

two traffics test: inner traffic : committed: 8665.945995056645 txn/s, latency: 4522.513128476694 ms, (p50: 4400 ms, p90: 5400 ms, p99: 10400 ms), latency samples: 3742780
two traffics test : committed: 100.02272898131058 txn/s, latency: 2101.5897727272727 ms, (p50: 2000 ms, p90: 2300 ms, p99: 7100 ms), latency samples: 1760
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.221, avg: 0.214", "QsPosToProposal: max: 0.271, avg: 0.244", "ConsensusProposalToOrdered: max: 0.302, avg: 0.288", "ConsensusOrderedToCommit: max: 0.374, avg: 0.360", "ConsensusProposalToCommit: max: 0.661, avg: 0.649"]
Max round gap was 1 [limit 4] at version 1768802. Max no progress secs was 4.972362 [limit 15] at version 1768802.
Test Ok

@wrwg wrwg merged commit ebcdb74 into main Jun 24, 2024
83 of 87 checks passed
@wrwg wrwg deleted the wrwg/enum-match branch June 24, 2024 14:38
wrwg added a commit that referenced this pull request Jun 25, 2024
In #13725 support for enums was added to the stackless bytecode IR. This PR extends the Move VM's representation of bytecode ('file format') by struct variants. It also implements

- representation in file format
- serialization/deserialization of file format
- bytecode verifier
- code generation in compiler v2

The runtime semantics (intepreter, runtime types, compatibility checking), as well as certain other features (as marked by #13806 in the code), are not yet implemented by this PR.

On Move bytecode level, there are `5 * 2` new instructions (each instruction has a dual generic version):

```
TestVariant(StructVariantHandleIndex)
	and TestVariantGeneric(StructVariantInstantiationIndex)
PackVariant(StructVariantHandleIndex)
	and PackVariantGeneric(StructVariantInstantiationIndex)
UnpackVariant(StructVariantHandleIndex)
	and UnpackVariantGeneric(StructVariantInstantiationIndex)
ImmBorrowVariantField(VariantFieldHandleIndex)
	and ImmBorrowVariantFieldGeneric(VariantFieldInstantiationIndex)
MutBorrowVariantField(VariantFieldHandleIndex)
	and MutBorrowVariantFieldGeneric(VariantFieldInstantiationIndex)
```

For the indices used in those instructions, 4 new tables have been added to the file format holding the associated data.

There is a lot of boilerplate code to support the new instructions and tables. Some refactoring of existing code has been done to avoid too much copy and paste, specifically in the serializers and in the bytecode verifier.

Apart of passing existing tests, there is a new test in `move-compiler-v2/tests/file-format-generator/struct_variants.move` which shows the disassembeled output of various match expressions. To add negative tests for the bytecode verifier and serializers, we first need a better way to build code from scratch (like a `CompiledModuleBuilder` or similar) which is planned to be done in a followup PR. Unfortunately, `.mvir` files cannot be used for this purpose, as there is no plan to support struct variants in this language right now.
wrwg added a commit that referenced this pull request Jun 25, 2024
In #13725 support for enums was added to the stackless bytecode IR. This PR extends the Move VM's representation of bytecode ('file format') by struct variants. It also implements

- representation in file format
- serialization/deserialization of file format
- bytecode verifier
- code generation in compiler v2

The runtime semantics (intepreter, runtime types, compatibility checking), as well as certain other features (as marked by #13806 in the code), are not yet implemented by this PR.

On Move bytecode level, there are `5 * 2` new instructions (each instruction has a dual generic version):

```
TestVariant(StructVariantHandleIndex)
	and TestVariantGeneric(StructVariantInstantiationIndex)
PackVariant(StructVariantHandleIndex)
	and PackVariantGeneric(StructVariantInstantiationIndex)
UnpackVariant(StructVariantHandleIndex)
	and UnpackVariantGeneric(StructVariantInstantiationIndex)
ImmBorrowVariantField(VariantFieldHandleIndex)
	and ImmBorrowVariantFieldGeneric(VariantFieldInstantiationIndex)
MutBorrowVariantField(VariantFieldHandleIndex)
	and MutBorrowVariantFieldGeneric(VariantFieldInstantiationIndex)
```

For the indices used in those instructions, 4 new tables have been added to the file format holding the associated data.

There is a lot of boilerplate code to support the new instructions and tables. Some refactoring of existing code has been done to avoid too much copy and paste, specifically in the serializers and in the bytecode verifier.

Apart of passing existing tests, there is a new test in `move-compiler-v2/tests/file-format-generator/struct_variants.move` which shows the disassembeled output of various match expressions. To add negative tests for the bytecode verifier and serializers, we first need a better way to build code from scratch (like a `CompiledModuleBuilder` or similar) which is planned to be done in a followup PR. Unfortunately, `.mvir` files cannot be used for this purpose, as there is no plan to support struct variants in this language right now.
@wrwg wrwg mentioned this pull request Jun 25, 2024
15 tasks
wrwg added a commit that referenced this pull request Jun 26, 2024
In #13725 support for enums was added to the stackless bytecode IR. This PR extends the Move VM's representation of bytecode ('file format') by struct variants. It also implements

- representation in file format
- serialization/deserialization of file format
- bytecode verifier
- code generation in compiler v2

The runtime semantics (intepreter, runtime types, compatibility checking), as well as certain other features (as marked by #13806 in the code), are not yet implemented by this PR.

On Move bytecode level, there are `5 * 2` new instructions (each instruction has a dual generic version):

```
TestVariant(StructVariantHandleIndex)
	and TestVariantGeneric(StructVariantInstantiationIndex)
PackVariant(StructVariantHandleIndex)
	and PackVariantGeneric(StructVariantInstantiationIndex)
UnpackVariant(StructVariantHandleIndex)
	and UnpackVariantGeneric(StructVariantInstantiationIndex)
ImmBorrowVariantField(VariantFieldHandleIndex)
	and ImmBorrowVariantFieldGeneric(VariantFieldInstantiationIndex)
MutBorrowVariantField(VariantFieldHandleIndex)
	and MutBorrowVariantFieldGeneric(VariantFieldInstantiationIndex)
```

For the indices used in those instructions, 4 new tables have been added to the file format holding the associated data.

There is a lot of boilerplate code to support the new instructions and tables. Some refactoring of existing code has been done to avoid too much copy and paste, specifically in the serializers and in the bytecode verifier.

Apart of passing existing tests, there is a new test in `move-compiler-v2/tests/file-format-generator/struct_variants.move` which shows the disassembeled output of various match expressions. To add negative tests for the bytecode verifier and serializers, we first need a better way to build code from scratch (like a `CompiledModuleBuilder` or similar) which is planned to be done in a followup PR. Unfortunately, `.mvir` files cannot be used for this purpose, as there is no plan to support struct variants in this language right now.
wrwg added a commit that referenced this pull request Jun 26, 2024
In #13725 support for enums was added to the stackless bytecode IR. This PR extends the Move VM's representation of bytecode ('file format') by struct variants. It also implements

- representation in file format
- serialization/deserialization of file format
- bytecode verifier
- code generation in compiler v2

The runtime semantics (intepreter, runtime types, compatibility checking), as well as certain other features (as marked by #13806 in the code), are not yet implemented by this PR.

On Move bytecode level, there are `5 * 2` new instructions (each instruction has a dual generic version):

```
TestVariant(StructVariantHandleIndex)
	and TestVariantGeneric(StructVariantInstantiationIndex)
PackVariant(StructVariantHandleIndex)
	and PackVariantGeneric(StructVariantInstantiationIndex)
UnpackVariant(StructVariantHandleIndex)
	and UnpackVariantGeneric(StructVariantInstantiationIndex)
ImmBorrowVariantField(VariantFieldHandleIndex)
	and ImmBorrowVariantFieldGeneric(VariantFieldInstantiationIndex)
MutBorrowVariantField(VariantFieldHandleIndex)
	and MutBorrowVariantFieldGeneric(VariantFieldInstantiationIndex)
```

For the indices used in those instructions, 4 new tables have been added to the file format holding the associated data.

There is a lot of boilerplate code to support the new instructions and tables. Some refactoring of existing code has been done to avoid too much copy and paste, specifically in the serializers and in the bytecode verifier.

Apart of passing existing tests, there is a new test in `move-compiler-v2/tests/file-format-generator/struct_variants.move` which shows the disassembeled output of various match expressions. To add negative tests for the bytecode verifier and serializers, we first need a better way to build code from scratch (like a `CompiledModuleBuilder` or similar) which is planned to be done in a followup PR. Unfortunately, `.mvir` files cannot be used for this purpose, as there is no plan to support struct variants in this language right now.
wrwg added a commit that referenced this pull request Jul 23, 2024
In #13725 support for enums was added to the stackless bytecode IR. This PR extends the Move VM's representation of bytecode ('file format') by struct variants. It also implements

- representation in file format
- serialization/deserialization of file format
- bytecode verifier
- code generation in compiler v2
- intepreter and paranoid mode

The runtime semantics (intepreter, runtime types, compatibility checking), as well as certain other features (as marked by #13806 in the code), are not yet implemented by this PR.

On Move bytecode level, there are `5 * 2` new instructions (each instruction has a dual generic version):

```
TestVariant(StructVariantHandleIndex)
	and TestVariantGeneric(StructVariantInstantiationIndex)
PackVariant(StructVariantHandleIndex)
	and PackVariantGeneric(StructVariantInstantiationIndex)
UnpackVariant(StructVariantHandleIndex)
	and UnpackVariantGeneric(StructVariantInstantiationIndex)
ImmBorrowVariantField(VariantFieldHandleIndex)
	and ImmBorrowVariantFieldGeneric(VariantFieldInstantiationIndex)
MutBorrowVariantField(VariantFieldHandleIndex)
	and MutBorrowVariantFieldGeneric(VariantFieldInstantiationIndex)
```

For the indices used in those instructions, 4 new tables have been added to the file format holding the associated data.

There is a lot of boilerplate code to support the new instructions and tables. Some refactoring of existing code has been done to avoid too much copy and paste, specifically in the serializers and in the bytecode verifier.

Apart of passing existing tests, there is a new test in move-compiler-v2/tests/file-format-generator/struct_variants.move which shows the disassembeled output of various match expressions.

There are also new e2e transactional tests in move-compiler-v2/transactional-tests/tests/enun. To add negative tests for the bytecode verifier and serializers, we first need a better way to build code with injected faults. See also #14074 and #13812.
wrwg added a commit that referenced this pull request Jul 23, 2024
In #13725 support for enums was added to the stackless bytecode IR. This PR extends the Move VM's representation of bytecode ('file format') by struct variants. It also implements

- representation in file format
- serialization/deserialization of file format
- bytecode verifier
- code generation in compiler v2
- intepreter and paranoid mode

The runtime semantics (intepreter, runtime types, compatibility checking), as well as certain other features (as marked by #13806 in the code), are not yet implemented by this PR.

On Move bytecode level, there are `5 * 2` new instructions (each instruction has a dual generic version):

```
TestVariant(StructVariantHandleIndex)
	and TestVariantGeneric(StructVariantInstantiationIndex)
PackVariant(StructVariantHandleIndex)
	and PackVariantGeneric(StructVariantInstantiationIndex)
UnpackVariant(StructVariantHandleIndex)
	and UnpackVariantGeneric(StructVariantInstantiationIndex)
ImmBorrowVariantField(VariantFieldHandleIndex)
	and ImmBorrowVariantFieldGeneric(VariantFieldInstantiationIndex)
MutBorrowVariantField(VariantFieldHandleIndex)
	and MutBorrowVariantFieldGeneric(VariantFieldInstantiationIndex)
```

For the indices used in those instructions, 4 new tables have been added to the file format holding the associated data.

There is a lot of boilerplate code to support the new instructions and tables. Some refactoring of existing code has been done to avoid too much copy and paste, specifically in the serializers and in the bytecode verifier.

Apart of passing existing tests, there is a new test in move-compiler-v2/tests/file-format-generator/struct_variants.move which shows the disassembeled output of various match expressions.

There are also new e2e transactional tests in move-compiler-v2/transactional-tests/tests/enun. To add negative tests for the bytecode verifier and serializers, we first need a better way to build code with injected faults. See also #14074 and #13812.
wrwg added a commit that referenced this pull request Jul 23, 2024
In #13725 support for enums was added to the stackless bytecode IR. This PR extends the Move VM's representation of bytecode ('file format') by struct variants. It also implements

- representation in file format
- serialization/deserialization of file format
- bytecode verifier
- code generation in compiler v2
- intepreter and paranoid mode

The runtime semantics (intepreter, runtime types, compatibility checking), as well as certain other features (as marked by #13806 in the code), are not yet implemented by this PR.

On Move bytecode level, there are `5 * 2` new instructions (each instruction has a dual generic version):

```
TestVariant(StructVariantHandleIndex)
	and TestVariantGeneric(StructVariantInstantiationIndex)
PackVariant(StructVariantHandleIndex)
	and PackVariantGeneric(StructVariantInstantiationIndex)
UnpackVariant(StructVariantHandleIndex)
	and UnpackVariantGeneric(StructVariantInstantiationIndex)
ImmBorrowVariantField(VariantFieldHandleIndex)
	and ImmBorrowVariantFieldGeneric(VariantFieldInstantiationIndex)
MutBorrowVariantField(VariantFieldHandleIndex)
	and MutBorrowVariantFieldGeneric(VariantFieldInstantiationIndex)
```

For the indices used in those instructions, 4 new tables have been added to the file format holding the associated data.

There is a lot of boilerplate code to support the new instructions and tables. Some refactoring of existing code has been done to avoid too much copy and paste, specifically in the serializers and in the bytecode verifier.

Apart of passing existing tests, there is a new test in move-compiler-v2/tests/file-format-generator/struct_variants.move which shows the disassembeled output of various match expressions.

There are also new e2e transactional tests in move-compiler-v2/transactional-tests/tests/enun. To add negative tests for the bytecode verifier and serializers, we first need a better way to build code with injected faults. See also #14074 and #13812.
wrwg added a commit that referenced this pull request Jul 23, 2024
In #13725 support for enums was added to the stackless bytecode IR. This PR extends the Move VM's representation of bytecode ('file format') by struct variants. It also implements

- representation in file format
- serialization/deserialization of file format
- bytecode verifier
- code generation in compiler v2
- intepreter and paranoid mode

The runtime semantics (intepreter, runtime types, compatibility checking), as well as certain other features (as marked by #13806 in the code), are not yet implemented by this PR.

On Move bytecode level, there are `5 * 2` new instructions (each instruction has a dual generic version):

```
TestVariant(StructVariantHandleIndex)
	and TestVariantGeneric(StructVariantInstantiationIndex)
PackVariant(StructVariantHandleIndex)
	and PackVariantGeneric(StructVariantInstantiationIndex)
UnpackVariant(StructVariantHandleIndex)
	and UnpackVariantGeneric(StructVariantInstantiationIndex)
ImmBorrowVariantField(VariantFieldHandleIndex)
	and ImmBorrowVariantFieldGeneric(VariantFieldInstantiationIndex)
MutBorrowVariantField(VariantFieldHandleIndex)
	and MutBorrowVariantFieldGeneric(VariantFieldInstantiationIndex)
```

For the indices used in those instructions, 4 new tables have been added to the file format holding the associated data.

There is a lot of boilerplate code to support the new instructions and tables. Some refactoring of existing code has been done to avoid too much copy and paste, specifically in the serializers and in the bytecode verifier.

Apart of passing existing tests, there is a new test in move-compiler-v2/tests/file-format-generator/struct_variants.move which shows the disassembeled output of various match expressions.

There are also new e2e transactional tests in move-compiler-v2/transactional-tests/tests/enun. To add negative tests for the bytecode verifier and serializers, we first need a better way to build code with injected faults. See also #14074 and #13812.
wrwg added a commit that referenced this pull request Jul 23, 2024
In #13725 support for enums was added to the stackless bytecode IR. This PR extends the Move VM's representation of bytecode ('file format') by struct variants. It also implements

- representation in file format
- serialization/deserialization of file format
- bytecode verifier
- code generation in compiler v2
- intepreter and paranoid mode

The runtime semantics (intepreter, runtime types, compatibility checking), as well as certain other features (as marked by #13806 in the code), are not yet implemented by this PR.

On Move bytecode level, there are `5 * 2` new instructions (each instruction has a dual generic version):

```
TestVariant(StructVariantHandleIndex)
	and TestVariantGeneric(StructVariantInstantiationIndex)
PackVariant(StructVariantHandleIndex)
	and PackVariantGeneric(StructVariantInstantiationIndex)
UnpackVariant(StructVariantHandleIndex)
	and UnpackVariantGeneric(StructVariantInstantiationIndex)
ImmBorrowVariantField(VariantFieldHandleIndex)
	and ImmBorrowVariantFieldGeneric(VariantFieldInstantiationIndex)
MutBorrowVariantField(VariantFieldHandleIndex)
	and MutBorrowVariantFieldGeneric(VariantFieldInstantiationIndex)
```

For the indices used in those instructions, 4 new tables have been added to the file format holding the associated data.

There is a lot of boilerplate code to support the new instructions and tables. Some refactoring of existing code has been done to avoid too much copy and paste, specifically in the serializers and in the bytecode verifier.

Apart of passing existing tests, there is a new test in move-compiler-v2/tests/file-format-generator/struct_variants.move which shows the disassembeled output of various match expressions.

There are also new e2e transactional tests in move-compiler-v2/transactional-tests/tests/enun. To add negative tests for the bytecode verifier and serializers, we first need a better way to build code with injected faults. See also #14074 and #13812.
wrwg added a commit that referenced this pull request Jul 23, 2024
In #13725 support for enums was added to the stackless bytecode IR. This PR extends the Move VM's representation of bytecode ('file format') by struct variants. It also implements

- representation in file format
- serialization/deserialization of file format
- bytecode verifier
- code generation in compiler v2
- intepreter and paranoid mode

The runtime semantics (intepreter, runtime types, compatibility checking), as well as certain other features (as marked by #13806 in the code), are not yet implemented by this PR.

On Move bytecode level, there are `5 * 2` new instructions (each instruction has a dual generic version):

```
TestVariant(StructVariantHandleIndex)
	and TestVariantGeneric(StructVariantInstantiationIndex)
PackVariant(StructVariantHandleIndex)
	and PackVariantGeneric(StructVariantInstantiationIndex)
UnpackVariant(StructVariantHandleIndex)
	and UnpackVariantGeneric(StructVariantInstantiationIndex)
ImmBorrowVariantField(VariantFieldHandleIndex)
	and ImmBorrowVariantFieldGeneric(VariantFieldInstantiationIndex)
MutBorrowVariantField(VariantFieldHandleIndex)
	and MutBorrowVariantFieldGeneric(VariantFieldInstantiationIndex)
```

For the indices used in those instructions, 4 new tables have been added to the file format holding the associated data.

There is a lot of boilerplate code to support the new instructions and tables. Some refactoring of existing code has been done to avoid too much copy and paste, specifically in the serializers and in the bytecode verifier.

Apart of passing existing tests, there is a new test in move-compiler-v2/tests/file-format-generator/struct_variants.move which shows the disassembeled output of various match expressions.

There are also new e2e transactional tests in move-compiler-v2/transactional-tests/tests/enun. To add negative tests for the bytecode verifier and serializers, we first need a better way to build code with injected faults. See also #14074 and #13812.
wrwg added a commit that referenced this pull request Jul 23, 2024
In #13725 support for enums was added to the stackless bytecode IR. This PR extends the Move VM's representation of bytecode ('file format') by struct variants. It also implements

- representation in file format
- serialization/deserialization of file format
- bytecode verifier
- code generation in compiler v2
- intepreter and paranoid mode

The runtime semantics (intepreter, runtime types, compatibility checking), as well as certain other features (as marked by #13806 in the code), are not yet implemented by this PR.

On Move bytecode level, there are `5 * 2` new instructions (each instruction has a dual generic version):

```
TestVariant(StructVariantHandleIndex)
	and TestVariantGeneric(StructVariantInstantiationIndex)
PackVariant(StructVariantHandleIndex)
	and PackVariantGeneric(StructVariantInstantiationIndex)
UnpackVariant(StructVariantHandleIndex)
	and UnpackVariantGeneric(StructVariantInstantiationIndex)
ImmBorrowVariantField(VariantFieldHandleIndex)
	and ImmBorrowVariantFieldGeneric(VariantFieldInstantiationIndex)
MutBorrowVariantField(VariantFieldHandleIndex)
	and MutBorrowVariantFieldGeneric(VariantFieldInstantiationIndex)
```

For the indices used in those instructions, 4 new tables have been added to the file format holding the associated data.

There is a lot of boilerplate code to support the new instructions and tables. Some refactoring of existing code has been done to avoid too much copy and paste, specifically in the serializers and in the bytecode verifier.

Apart of passing existing tests, there is a new test in move-compiler-v2/tests/file-format-generator/struct_variants.move which shows the disassembeled output of various match expressions.

There are also new e2e transactional tests in move-compiler-v2/transactional-tests/tests/enun. To add negative tests for the bytecode verifier and serializers, we first need a better way to build code with injected faults. See also #14074 and #13812.
wrwg added a commit that referenced this pull request Jul 23, 2024
In #13725 support for enums was added to the stackless bytecode IR. This PR extends the Move VM's representation of bytecode ('file format') by struct variants. It also implements

- representation in file format
- serialization/deserialization of file format
- bytecode verifier
- code generation in compiler v2
- intepreter and paranoid mode

The runtime semantics (intepreter, runtime types, compatibility checking), as well as certain other features (as marked by #13806 in the code), are not yet implemented by this PR.

On Move bytecode level, there are `5 * 2` new instructions (each instruction has a dual generic version):

```
TestVariant(StructVariantHandleIndex)
	and TestVariantGeneric(StructVariantInstantiationIndex)
PackVariant(StructVariantHandleIndex)
	and PackVariantGeneric(StructVariantInstantiationIndex)
UnpackVariant(StructVariantHandleIndex)
	and UnpackVariantGeneric(StructVariantInstantiationIndex)
ImmBorrowVariantField(VariantFieldHandleIndex)
	and ImmBorrowVariantFieldGeneric(VariantFieldInstantiationIndex)
MutBorrowVariantField(VariantFieldHandleIndex)
	and MutBorrowVariantFieldGeneric(VariantFieldInstantiationIndex)
```

For the indices used in those instructions, 4 new tables have been added to the file format holding the associated data.

There is a lot of boilerplate code to support the new instructions and tables. Some refactoring of existing code has been done to avoid too much copy and paste, specifically in the serializers and in the bytecode verifier.

Apart of passing existing tests, there is a new test in move-compiler-v2/tests/file-format-generator/struct_variants.move which shows the disassembeled output of various match expressions.

There are also new e2e transactional tests in move-compiler-v2/transactional-tests/tests/enun. To add negative tests for the bytecode verifier and serializers, we first need a better way to build code with injected faults. See also #14074 and #13812.
wrwg added a commit that referenced this pull request Jul 23, 2024
In #13725 support for enums was added to the stackless bytecode IR. This PR extends the Move VM's representation of bytecode ('file format') by struct variants. It also implements

- representation in file format
- serialization/deserialization of file format
- bytecode verifier
- code generation in compiler v2
- intepreter and paranoid mode

The runtime semantics (intepreter, runtime types, compatibility checking), as well as certain other features (as marked by #13806 in the code), are not yet implemented by this PR.

On Move bytecode level, there are `5 * 2` new instructions (each instruction has a dual generic version):

```
TestVariant(StructVariantHandleIndex)
	and TestVariantGeneric(StructVariantInstantiationIndex)
PackVariant(StructVariantHandleIndex)
	and PackVariantGeneric(StructVariantInstantiationIndex)
UnpackVariant(StructVariantHandleIndex)
	and UnpackVariantGeneric(StructVariantInstantiationIndex)
ImmBorrowVariantField(VariantFieldHandleIndex)
	and ImmBorrowVariantFieldGeneric(VariantFieldInstantiationIndex)
MutBorrowVariantField(VariantFieldHandleIndex)
	and MutBorrowVariantFieldGeneric(VariantFieldInstantiationIndex)
```

For the indices used in those instructions, 4 new tables have been added to the file format holding the associated data.

There is a lot of boilerplate code to support the new instructions and tables. Some refactoring of existing code has been done to avoid too much copy and paste, specifically in the serializers and in the bytecode verifier.

Apart of passing existing tests, there is a new test in move-compiler-v2/tests/file-format-generator/struct_variants.move which shows the disassembeled output of various match expressions.

There are also new e2e transactional tests in move-compiler-v2/transactional-tests/tests/enun. To add negative tests for the bytecode verifier and serializers, we first need a better way to build code with injected faults. See also #14074 and #13812.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants