[4/n][enums] Move enums all of the Move tests #17246

tzakian · 2024-04-19T22:24:58Z

Description

This PR consists of tests only! (and updates to exp files of existing tests).

Stack:

Test Plan

File format

Status: Done

Bounds checking and serialization proptests updated.

Deserializer

Status: Done

Added test to verify that we are properly checking for versioning issues
-- e.g., if someone manually changed the version of the serialized module
so the version was pre-enums, but contained enum-data inside of it.

Serializer

Status: Done

Ensured that serializing a version 7 module at version 6 works as long as
no version 7 features are used, and that the resulting module can be
deserialized into version 6 and version 7 modules.
Proptests for serializer/deserializer round-trips
Tested round-trip properties:
- Version 7 module with no eums can:
  - Be serialized under version 6
  - Result can be deserialized under version 6 and 7 and reserialized at version 6 and 7.
Updated serializer, and added tests to make sure we use the module's version when serializing, unless it is specified as an override.

Compatibility

Status: Done

Add config to allow adding new variants
Existing variants must be in the same order (have the same tag)
Cannot remove a variant
Cannot change a field or fields of an existing variant
Cannot rename existing variant

Bytecode Verifier

Status: Done

Abilities

Enum can have fewer abilities than the type that it contains [external-crates/move/crates/bytecode-verifier-transactional-tests/tests/ability_field_requirements/unrestricted_enum_has_resource_field.exp]
Enum cannot have abilities that a type that it contains does not satisfy [external-crates/move/crates/bytecode-verifier-transactional-tests/tests/ability_field_requirements/unrestricted_enum_has_resource_field.mvir]

check_bounds

Generic enum cannot have too few type arguments [external-crates/move/crates/bytecode-verifier-transactional-tests/tests/check_bounds/too_few_type_actuals_enum.mvir]
Generic enum cannot have too many type arguments [external-crates/move/crates/bytecode-verifier-transactional-tests/tests/check_bounds/too_many_type_actuals_enum.mvir]

check_duplication

Cannot have enum and struct in the same module with the same name [external-crates/move/crates/bytecode-verifier-transactional-tests/tests/check_duplication/duplicate_enum_and_struct_names.mvir]
Cannot have two enums in the same module with the same name [external-crates/move/crates/bytecode-verifier-transactional-tests/tests/check_duplication/duplicate_enum_name.mvir]
Cannot have an enum variant with duplicate field names [external-crates/move/crates/bytecode-verifier-transactional-tests/tests/check_duplication/duplicate_field_name_enum.mvir]
Cannot have an enum with duplicate variant names [external-crates/move/crates/bytecode-verifier-transactional-tests/tests/check_duplication/duplicate_variant_name.mvir]
Cannot have an empty enum [external-crates/move/crates/bytecode-verifier-transactional-tests/tests/check_duplication/empty_enums.mvir]

Check for duplicated variant handles [external-crates/move/crates/bytecode-verifier-tests/src/unit_tests/duplication_tests.rs]
Check for duplicated datatype names [external-crates/move/crates/bytecode-verifier-transactional-tests/tests/enum_defs/module_enum_struct_shared_name.mvir]

enum_defs

Can have an enum with the same name as the module [external-crates/move/crates/bytecode-verifier-transactional-tests/tests/enum_defs/module_enum_shared_name.mvir]
Cannot have mutually recursive enums (simple test) [external-crates/move/crates/bytecode-verifier-transactional-tests/tests/enum_defs/mutual_recursive_enum.mvir]
More complex recursive enum definition checks [external-crates/move/crates/bytecode-verifier-transactional-tests/tests/enum_defs/recursive_enum.mvir]
Cannot declare an enum with references inside of it [external-crates/move/crates/bytecode-verifier-transactional-tests/tests/enum_defs/ref_in_enum.mvir]

instantiaion_loops [each test is an existing test that we have for structs but ported over to enums]

Complex type instantiation loop using enums [external-crates/move/crates/bytecode-verifier-transactional-tests/tests/instantiation_loops/complex_1_enum.mvir]
Mutually recursive enum type instantiated with a type is OK [external-crates/move/crates/bytecode-verifier-transactional-tests/tests/instantiation_loops/mutually_recursive_non_generic_type_ok_enum.mvir]
Mutually recursive enum type instantiation with a type is OK since the types eventually shuffle to ground out [external-crates/move/crates/bytecode-verifier-transactional-tests/tests/instantiation_loops/mutually_recursive_three_args_type_con_non_generic_types_ok_enum.mvir]
...

locals_safety

Re-assign an enum that doesn't have drop [external-crates/move/crates/bytecode-verifier-transactional-tests/tests/locals_safety/assign_enum_resource.mvir]

stack_usage_verifier

Unpack an enum variant with too many fields [external-crates/move/crates/bytecode-verifier-transactional-tests/tests/stack_usage_verifier/unpack_extra_binding_enum.mvir]
Unpack an enum variant with too few fields [external-crates/move/crates/bytecode-verifier-transactional-tests/tests/stack_usage_verifier/unpack_missing_binding_enum.mvir]

type_safety

Fail to handle an undroppable value within a variant switch [external-crates/move/crates/bytecode-verifier-transactional-tests/tests/control_flow/variant_switch.mvir]
Try to use an unassigned value within a variant switch arm [external-crates/move/crates/bytecode-verifier-transactional-tests/tests/control_flow/variant_switch.mvir]
Fail to unpack an undroppable enum within a variant switch [external-crates/move/crates/bytecode-verifier-transactional-tests/tests/control_flow/variant_switch_successor.mvir]
Make sure we properly construct the control flow graph for a variant switch as an unconditional branch [external-crates/move/crates/bytecode-verifier-transactional-tests/tests/control_flow/variant_switch_unconditional_branch.mvir]
Added low-level tests to make sure we properly construct the control flow graph [external-crates/move/crates/move-binary-format/src/unit_tests/control_flow_graph_tests.rs] and in [external-crates/move/crates/move-ir-compiler/src/unit_tests/cfg_tests.rs]
Added test to make sure successors are always sorted/stable, and that the control-flow graph generated is the same regardless of the order of the jumps within a variant switch [external-crates/move/crates/move-binary-format/src/unit_tests/control_flow_graph_tests.rs:out_of_order_blocks_variant_switch]

VM

Test trying to unpack the wrong variant (generic, non-generic)
Test that execution proceeds normally, and that we get expected results for computations that use enums
Test that mutable references taken from an enum variant can be modified, and that these modifications are reflected correctly
Test that we can mutate the variant inside of an enum type.

Added tests to make sure we can't infinitie loop within a variant switch [external-crates/move/crates/move-vm-transactional-tests/tests/enums/variant_switch_loop.mvir]

Values

Test that enum Option is backwards compatible with vector Option.

vercel · 2024-04-19T22:25:06Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
sui-core	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 24, 2024 5:53pm
sui-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 24, 2024 5:53pm

3 Ignored Deployments

Name	Status	Preview	Updated (UTC)
multisig-toolkit	⬜️ Ignored (Inspect)	Visit Preview	May 24, 2024 5:53pm
sui-kiosk	⬜️ Ignored (Inspect)	Visit Preview	May 24, 2024 5:53pm
sui-typescript-docs	⬜️ Ignored (Inspect)	Visit Preview	May 24, 2024 5:53pm

cgswords · 2024-05-02T06:54:13Z

Should we preserve / move the development/enums/matching tests to move_2024/ for the compiler suite as well? It is somewhat redundant, but I often rely on the compiler tests during development.

cgswords

Reviewed the new transactonal tests with some feedback

external-crates/move/crates/move-compiler-transactional-tests/tests/matching/abc_match_ref.move

...l-crates/move/crates/move-compiler-transactional-tests/tests/matching/abc_match_mut_ref.move

...nal-crates/move/crates/move-compiler-transactional-tests/tests/matching/abc_match_twice.move

...nal-crates/move/crates/move-compiler-transactional-tests/tests/matching/at_pattern_copy.move

...l-crates/move/crates/move-compiler-transactional-tests/tests/matching/guard_backtrack_5.move

external-crates/move/crates/move-compiler-transactional-tests/tests/matching/multi_or.move

...rates/move/crates/move-compiler-transactional-tests/tests/matching/nested_either_borrow.move

...-crates/move/crates/move-compiler-transactional-tests/tests/matching/struct_named_value.move

external-crates/move/crates/move-compiler-transactional-tests/tests/matching/struct_named.move

cgswords

The changes look good to me but I think bad_guard_2.move should be disallowed by the compiler outright due to how match is compiled. I'm going to put a diff up soon to address this.

## Description This is the top-level (and final!) PR in the stack for Move enums. This PR * Updates the protocol config to support the new execution version * Adds new e2e adapter tests to make sure we handle enums properly * Updates snapshots for tests (and protocol config snapshots and the like). This also then tests all other tests that have been added below this in the stack (e.g., the package resolver tests, graphql tests, all the Move tests, other misc tests that were added). **NB:** Before landing this we need to determine the best way of turning on the correct binary-version for testing only. So I would encourage mainly looking at the tests (and the updated snapshots for the replay tool). Protocol-config changes, and snapshots for them will need to be updated as this gets closer to landing, and we further concretize the testing/pre-rollout/rollout plan. ## Stack: * #17245 * #17246 * #17247 * #17248 * #17249 * #17250 * #17251 **<< You are here** 🎉 ## Test plan It's pretty much all tests.

## Description Adds support for Move enums to both graphql and json-rpc. This adds a couple different things. At a high level: 1. Adds an interface `IMoveDatatype` that allows for access to common fields between both Move structs and enums (e.g., name, type parameters, abilities). 2. Adds methods `datatype(name: String)` and `datatypes` to `MovePackage` that returns datatypes. Note that datatype names are still sorted in the same way as before in pagination (in particular: it will not be paginated as all structs, then all enums or vis-versa, but paginated in sorted order on the datatype names). This is due to the way `datatypes` is implemented in the package resolver. 3. Adds `MoveEnum` as a GraphQL type and associated machinery. This PR is meant to be a logically-reviewable portion but is not land-able on its own. It must be merged in with the changes in the rest of this stack to be landed. ## Stack: * #17245 * #17246 * #17247 * #17248 * #17249 * #17250 **<< You are here** * #17251 ## Test plan Added new tests in this PR for the new features. --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [X] JSON-RPC: - [X] GraphQL: - [ ] CLI: - [ ] Rust SDK:

## Description Adds support for Move enums to the Sui package resolver crate along with tests for it. This PR is meant to be a logically-reviewable portion but is not land-able on its own. It must be merged in with the changes in the rest of this stack to be landed. ## Stack: * #17245 * #17246 * #17247 * #17248 * #17249 **<< You are here** * #17250 * #17251 ## Test plan Added tests in the PR to make sure we can properly resolve enum types. --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK:

## Description Changes for Move enums in `sui-execution`. Generally nothing surprising. Only thing worth note is that enums are not allowed to be OTWs (see tests in top commit in this stack). This PR is meant to be a logically-reviewable portion but is not land-able on its own. It must be merged in with the changes in the rest of this stack to be landed. ## Stack: * #17245 * #17246 * #17247 * #17248 **<< You are here** * #17249 * #17250 * #17251 ## Test plan Tests in the top commit on this stack.

…ms (#17247) ## Description This PR contains the changes needed for Move enums in sui-types, the protocol config changes (except for bumping the bytecode version), along with other misc/smaller changes that are not easily categorized. This PR is meant to be a logically-reviewable portion but is not land-able on its own. It must be merged in with the changes in the rest of this stack to be landed. ## Stack: * #17245 * #17246 * #17247 **<< You are here** * #17248 * #17249 * #17250 * #17251 ## Test plan Tests are in the top commit in this stack here:

## Description This adds the enums implementation for Move. This PR only contains the changes for enums within `external-crates/move` and nowhere else. This PR _does not_ contain any tests. The tests for these changes can be found in the PR above in the stack. Note that individual commits are cut out within this PR to (hopefully) make the review process a bit easier. ## Stack: * #17245 **<< You are here** * #17246 * #17247 * #17248 * #17249 * #17250 * #17251 ## Test plan Tested in the PR above this. --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [X] Protocol: Adds a new protocol version, and enables Move enums on devnet. - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [X] JSON-RPC: Adds support for Move enum values in returned json-rpc results. - [X] GraphQL: Adds support for Move enum values and types to GraphQL. - [ ] CLI: - [ ] Rust SDK:

tzakian requested review from cgswords, tnowacki, dariorussi and a team April 19, 2024 22:24

tzakian force-pushed the tzakian/move-enums-implementation branch from 96fab27 to 060990f Compare April 22, 2024 19:57

tzakian force-pushed the tzakian/move-enums-move-tests branch from 2ce2dab to e8d8c5d Compare April 22, 2024 19:57

vercel bot deployed to Preview – sui-core April 22, 2024 20:01 View deployment

tzakian force-pushed the tzakian/move-enums-implementation branch from 060990f to 6c5ab65 Compare April 22, 2024 22:39

tzakian force-pushed the tzakian/move-enums-move-tests branch from e8d8c5d to 3ac0bb8 Compare April 22, 2024 22:39

tzakian marked this pull request as ready for review April 22, 2024 22:40

vercel bot deployed to Preview – sui-core April 22, 2024 22:41 View deployment

tzakian force-pushed the tzakian/move-enums-implementation branch from 6c5ab65 to 0b0e5bd Compare April 25, 2024 23:27

tzakian force-pushed the tzakian/move-enums-move-tests branch from 3ac0bb8 to f7c5349 Compare April 25, 2024 23:27

vercel bot deployed to Preview – sui-docs April 25, 2024 23:35 View deployment

tzakian force-pushed the tzakian/move-enums-implementation branch from 0b0e5bd to bcb9f29 Compare May 2, 2024 19:35

tzakian force-pushed the tzakian/move-enums-move-tests branch from f7c5349 to 9382454 Compare May 2, 2024 19:35

vercel bot deployed to Preview – sui-docs May 2, 2024 19:44 View deployment

cgswords requested changes May 8, 2024

View reviewed changes

tzakian force-pushed the tzakian/move-enums-implementation branch from bcb9f29 to 6a646a2 Compare May 13, 2024 18:28

tzakian force-pushed the tzakian/move-enums-move-tests branch from 9382454 to eff9b81 Compare May 13, 2024 18:28

vercel bot deployed to Preview – sui-docs May 13, 2024 18:31 View deployment

tzakian force-pushed the tzakian/move-enums-implementation branch from 6a646a2 to 1f87170 Compare May 14, 2024 22:09

tzakian force-pushed the tzakian/move-enums-move-tests branch from f5c830f to e25cf32 Compare May 22, 2024 18:04

vercel bot deployed to Preview – sui-docs May 22, 2024 18:06 View deployment

cgswords approved these changes May 22, 2024

View reviewed changes

tzakian force-pushed the tzakian/move-enums-implementation branch from 8f1cb35 to 0caf49f Compare May 23, 2024 17:17

tzakian force-pushed the tzakian/move-enums-move-tests branch from e25cf32 to d34a051 Compare May 23, 2024 17:17

vercel bot deployed to Preview – sui-docs May 23, 2024 17:20 View deployment

tzakian force-pushed the tzakian/move-enums-implementation branch from 0caf49f to 9a3dd52 Compare May 23, 2024 18:49

tzakian force-pushed the tzakian/move-enums-move-tests branch from d34a051 to 5cf287d Compare May 23, 2024 18:50

vercel bot deployed to Preview – sui-docs May 23, 2024 18:52 View deployment

tzakian added 2 commits May 23, 2024 17:27

[4/n][enums] Move compiler enums tests

a882acd

[1][5/n][enums] All Move tests except source lang tests

f076f43

tzakian force-pushed the tzakian/move-enums-implementation branch from 9a3dd52 to 5c315e9 Compare May 24, 2024 00:42

tzakian force-pushed the tzakian/move-enums-move-tests branch from 5cf287d to f076f43 Compare May 24, 2024 00:42

vercel bot deployed to Preview – sui-docs May 24, 2024 00:44 View deployment

tzakian requested review from amnn, wlmyng, stefan-mysten, emmazzz, suiwombat, mystenmark and joyqvq as code owners May 24, 2024 17:46

tzakian merged commit d0a9c1b into tzakian/move-enums-implementation May 24, 2024
14 of 18 checks passed

tzakian deleted the tzakian/move-enums-move-tests branch May 24, 2024 17:46

vercel bot deployed to Preview – sui-docs May 24, 2024 17:53 View deployment

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[4/n][enums] Move enums all of the Move tests #17246

[4/n][enums] Move enums all of the Move tests #17246

tzakian commented Apr 19, 2024 •

edited

Loading

vercel bot commented Apr 19, 2024 •

edited

Loading

cgswords commented May 2, 2024

cgswords left a comment

cgswords left a comment

[4/n][enums] Move enums all of the Move tests #17246

[4/n][enums] Move enums all of the Move tests #17246

Conversation

tzakian commented Apr 19, 2024 • edited Loading

Description

Stack:

Test Plan

File format

Deserializer

Serializer

Compatibility

Bytecode Verifier

Abilities

check_bounds

check_duplication

enum_defs

instantiaion_loops [each test is an existing test that we have for structs but ported over to enums]

locals_safety

stack_usage_verifier

type_safety

VM

Values

vercel bot commented Apr 19, 2024 • edited Loading

cgswords commented May 2, 2024

cgswords left a comment

Choose a reason for hiding this comment

cgswords left a comment

Choose a reason for hiding this comment

tzakian commented Apr 19, 2024 •

edited

Loading

vercel bot commented Apr 19, 2024 •

edited

Loading