[Compiler-v2][Feature] Index notation #13695

rahxephon89 · 2024-06-14T01:43:23Z

Description

This PR adds the index notation syntactic sugar to Move 2 for accessing vector elements and resources.

Example use of this feature:

vector (non-spec mode only):

Example:

    struct X<M> has copy, drop, store {
        value: M
    }
    fun test_vector() {
        let x = X {
            value: 2
        };
        let v = vector[x, x];
        assert!(v[0].value == 2, 0);
    }

    fun test_vector_borrow_mut() {
        let x1 = X {
            value: true
        };
        let x2 = X {
            value: false
        };
        let y1 = Y {
            field: x1
        };
        let y2 = Y {
            field: x2
        };
        let v = vector[y1, y2];
        v[0].field.value = false;
        v[1].field.value = true;
        assert!(v[0])field.value == false, 0);
        assert!(v[1].field.value == true, 0);
    }

resource

Example:


    struct X<M> has copy, drop, store {
        value: M
    }
    struct Y<T> has key, drop {
        field: T
    }

    fun test_resource_3() acquires R {
        use 0x42::test;
        assert!(test::Y<X<bool>>[@0x1].field.value == true, 0);
    }

    fun test_resource_4() acquires R {
        let addr = @0x1;
        0x42::test ::Y<X<bool>> [addr].field.value = false;
        spec {
            assert Y<X<bool>>[addr].field.value == false;
        };
        assert!((&Y<X<bool>>[addr]) .field.value == false, 1);
    }

    fun test_resource_5() acquires Y {
        let addr = @0x1;
        0x42::test ::Y<X<bool>> [addr].field.value = false;
        let y_resource = Y<X<bool>>[addr];
        assert!(y_resource.field.value == false, 1);
    }

Type of Change

Which Components or Systems Does This Change Impact?

How Has This Been Tested?

existing tests pass;
added new unit tests to positive and negative use of index notation of V2;
added a unit test to checking-lang-v1 to show the error message when compiled using V1;
added a unit test to the prover to illustrate resource indexing can be used in the prover.

Key Areas to Review

whether the rewrite logic is correctly applied to vector and resource.

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-06-14T01:43:26Z

⏱️ 19h 2m total CI duration on this PR

Job	Cumulative Duration	Recent Runs
rust-targeted-unit-tests	5h 29m	🟥 🟩 🟩 🟩 🟩 (+11 more)
test-fuzzers	4h 14m	🟩 🟩 🟥 🟩 🟩 (+3 more)
rust-move-tests	2h 59m	🟩 🟩 🟩 🟩 🟩 (+10 more)
rust-move-unit-coverage	2h 45m	🟩 🟩 🟩 🟩 🟩 (+10 more)
rust-lints	1h 26m	🟩 🟩 🟩 🟩 🟩 (+10 more)
run-tests-main-branch	1h 7m	🟩 🟩 🟩 🟩 🟩 (+10 more)
general-lints	27m	🟩 🟩 🟩 🟩 🟩 (+10 more)
check-dynamic-deps	22m	🟩 🟩 🟩 🟩 🟩 (+10 more)
semgrep/ci	6m	🟩 🟩 🟩 🟩 🟩 (+10 more)
file_change_determinator	3m	🟩 🟩 🟩 🟩 🟩 (+10 more)
file_change_determinator	3m	🟩 🟩 🟩 🟩 🟩 (+10 more)
permission-check	46s	🟩 🟩 🟩 🟩 🟩 (+10 more)
permission-check	46s	🟩 🟩 🟩 🟩 🟩 (+10 more)
permission-check	42s	🟩 🟩 🟩 🟩 🟩 (+10 more)
permission-check	35s	🟩 🟩 🟩 🟩 🟩 (+10 more)

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

codecov · 2024-06-14T01:54:55Z

Codecov Report

Attention: Patch coverage is 93.38521% with 17 lines in your changes missing coverage. Please review.

Project coverage is 59.0%. Comparing base (20b7f3a) to head (7ad3d51).
Report is 1 commits behind head on main.

Files	Patch %	Lines
...d_party/move/move-model/src/builder/exp_builder.rs	95.3%	11 Missing ⚠️
...hird_party/move/move-compiler/src/expansion/ast.rs	0.0%	3 Missing ⚠️
third_party/move/move-model/src/ast.rs	0.0%	3 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##             main   #13695    +/-   ##
========================================
  Coverage    59.0%    59.0%            
========================================
  Files         821      821            
  Lines      197883   198127   +244     
========================================
+ Hits       116785   117008   +223     
- Misses      81098    81119    +21

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

third_party/move/move-compiler-v2/tests/checking/typing/index_err.exp

third_party/move/move-prover/tests/sources/functional/resource_index.exp

third_party/move/tools/move-package/tests/test_sources/compilation/std-lib-conflicts/Move.exp

brmataptos

Looks mostly good, but we need to gate this feature on language version 2. See uses of check_language_version in exp_builder.rs for examples. Also add a test to tests/checking-lang-v1/.

rahxephon89 · 2024-06-19T00:07:56Z

Looks mostly good, but we need to gate this feature on language version 2. See uses of check_language_version in exp_builder.rs for examples. Also add a test to tests/checking-lang-v1/.

I have been using context.env.flags().v2() in the code to gate the feature but will follow what @wrwg does in the enum type PR to do it. Also, I already added a test in third_party/move/move-compiler/tests/move_check/v2-not-supported for outputting the V1 error and the test cannot be added to tests/checking-lang-v1/ because it is run in V2 context and no error will be generated since they are already rewritten in the expansion phase.

third_party/move/move-compiler-v2/tests/checking/typing/index_err.exp

third_party/move/move-compiler/src/expansion/translate.rs

third_party/move/move-compiler/src/parser/ast.rs

third_party/move/move-compiler/tests/move_check/v2-not-supported/index.exp

third_party/move/tools/move-package/tests/test_sources/compilation/std-lib-conflicts/Move.exp

brmataptos · 2024-06-19T23:15:32Z

Looks mostly good, but we need to gate this feature on language version 2. See uses of check_language_version in exp_builder.rs for examples. Also add a test to tests/checking-lang-v1/.

I have been using context.env.flags().v2() in the code to gate the feature but will follow what @wrwg does in the enum type PR to do it. Also, I already added a test in third_party/move/move-compiler/tests/move_check/v2-not-supported for outputting the V1 error and the test cannot be added to tests/checking-lang-v1/ because it is run in V2 context and no error will be generated since they are already rewritten in the expansion phase.

That means you need to pass language version into the move-compiler. See how Wolfgang uses a modified parser for enums.

wrwg · 2024-06-19T23:33:10Z

Looks mostly good, but we need to gate this feature on language version 2. See uses of check_language_version in exp_builder.rs for examples. Also add a test to tests/checking-lang-v1/.

I have been using context.env.flags().v2() in the code to gate the feature but will follow what @wrwg does in the enum type PR to do it. Also, I already added a test in third_party/move/move-compiler/tests/move_check/v2-not-supported for outputting the V1 error and the test cannot be added to tests/checking-lang-v1/ because it is run in V2 context and no error will be generated since they are already rewritten in the expansion phase.

That means you need to pass language version into the move-compiler. See how Wolfgang uses a modified parser for enums.

I update this further because v2() was actually be used both for v2 compiler and language. Now there are flags.lang_v2() and flags.compiler_v2() (once #56648 landed).

rahxephon89 · 2024-06-29T07:27:51Z

third_party/move/move-model/src/builder/exp_builder.rs

+        if let EA::Exp_::Name(m, _) = &target.value {
+            let (is_struct, is_schema) = self.is_struct_or_schema(m);
+            if is_struct {
+                self.check_language_version(loc, "resource indexing", LanguageVersion::V2_0);


removed check_language_version

rahxephon89 · 2024-06-29T07:28:04Z

third_party/move/move-model/src/builder/exp_builder.rs

@@ -1774,6 +1774,13 @@ impl<'env, 'translator, 'module_translator> ExpTranslator<'env, 'translator, 'mo
                let target_ty = self.fresh_type_var();
                let ty = Type::Reference(ref_kind, Box::new(target_ty.clone()));
                let result_ty = self.check_type(&loc, &ty, expected_type, context);
+                if let EA::Exp_::Index(target, index) = &exp.value {


removed check_language_version

rahxephon89 · 2024-07-02T07:18:23Z

third_party/move/move-model/src/builder/exp_builder.rs

+                call = Some(self.new_error_exp());
+            }
+        }
+        if !self.is_spec_mode() {


The logic here is that, 1) if it is a resource, create resource indexing; 2) if it is a schema, generate error; 3) otherwise, exp builder is in impl mode, we will create vector indexing (call is none for sure).

rahxephon89 · 2024-07-02T07:21:30Z

third_party/move/move-model/src/builder/exp_builder.rs

+            true,
+        );
+        if let Some(call) = index_call_opt {
+            if !self.is_spec_mode() {


rahxephon89 · 2024-07-02T07:23:03Z

third_party/move/move-model/src/builder/exp_builder.rs

+        expected_type: &Type,
+        context: &ErrorMessageContext,
+    ) -> ExpData {
+        fn convert_name_to_type(


Refactored borrow_global as you suggested. I also refactored vector_borrow but due to the issue #13888, I keep vector_borrow as it is for now.

rahxephon89 · 2024-07-02T07:30:27Z

third_party/move/move-compiler-v2/tests/checking-lang-v1/index.exp

+   ┌─ tests/checking-lang-v1/index.move:10:19
+   │
+10 │         use 0x42::test;
+   │                   ^^^^ Unused 'use' of alias 'test'. Consider removing it


rahxephon89 · 2024-07-02T07:36:03Z

third_party/move/move-compiler/src/expansion/translate.rs

-                    .env
-                    .add_diag(diag!(Syntax::SpecContextRestricted, (loc, msg)));
+                context.env.add_diag(diag!(
+                    Syntax::SpecContextRestricted,


rahxephon89 · 2024-07-02T07:36:07Z

third_party/move/move-model/src/builder/exp_builder.rs

@@ -3236,6 +3243,156 @@ impl<'env, 'translator, 'module_translator> ExpTranslator<'env, 'translator, 'mo
        }
    }

+    fn call_to_borrow_global(


rahxephon89 · 2024-07-02T07:36:11Z

third_party/move/move-model/src/builder/exp_builder.rs

+        )
+    }
+
+    fn call_to_vector_borrow(


rahxephon89 · 2024-07-02T07:36:30Z

third_party/move/move-model/src/builder/exp_builder.rs

+    ) -> Option<ExpData> {
+        let mut call = None;
+        if let EA::Exp_::Name(m, _) = &target.value {
+            let (is_struct, is_schema) = self.is_struct_or_schema(m);


refactored the code

mkurnikov · 2024-07-02T17:11:39Z

I haven't read the whole thread, sorry if I missed the discussion before.

Do I understand correctly that with this PR, borrow_global_mut would be translated into the &mut Res[address], and users would have to always use parenteses?

borrow_global_mut<Res>(@0x1).handle

looks not much worse than

(&mut Res[@0x1]).handle

Could be instead somehow get the &mut reference inside the index operator, so the expression prioritisation rules would be more natural?
The Res[&mut @0x1] does not seem much better in terms of readability, but at least it won't require constant wrapping with parens.

rahxephon89 · 2024-07-02T17:15:35Z

I haven't read the whole thread, sorry if I missed the discussion before.

Do I understand correctly that with this PR, borrow_global_mut would be translated into the &mut Res[address], and users would have to always use parenteses?
borrow_global_mut<Res>(@0x1).handle
looks not much worse than
(&mut Res[@0x1]).handle
Could be instead somehow get the &mut reference inside the index operator, so the expression prioritisation rules would be more natural? The Res[&mut @0x1] does not seem much better in terms of readability, but at least it won't require constant wrapping with parens.

Hi @mkurnikov, you can use Res[@0x1] = ... or Res[@0x1].a.b on the left hand side for updating the value. Please let me know if the test case here does not contain test cases that do not cover your case, thanks!

mkurnikov · 2024-07-02T17:24:29Z

third_party/move/move-compiler-v2/transactional-tests/tests/no-v1-comparison/index.move

+        assert!(test::R[@0x1].value == true, 0);
+        R[@0x1] = R{value: false};
+        assert!(test::R[@0x1].value == false, 0);
+    }


@rahxephon89 Are those two tests roughtly equal in the operations performed? Is the R[@0x1] = R{value: false}; is sugar over (&mut R[@0x1]).value = false?

You can use R[@0x1].value = false as well.

Isn't that too much sugar? I honestly thought the R[@0x1] = R { value: false } equals to the move_to(@0x1, R { value: false } ) first, before I remembered that the first parameter is a &signer.

If the goal is to remove boilerplate from the borrow_global and borrow_global_mut calls, I'd suggest instead to go with

borrow_global<Res>(@0x1) -> Res[@0x1] borrow_global_mut<Res>(@0x1) -> Res[mut @0x1]

We're special casing things anyway, and mut without ref never used in Move, so it could be easy distinguished. And it won't mess the prioritisation rules, so you could write

Res[mut @0x1].value = false

without any parens.

You can use R[@0x1].value = false as well.

Oh, so the compiler knows that we're trying to set a value, and automatically replaces borrow_global with borrow_global_mut in the translator?

For standalone R[@0x1], it depends on whether it appears on lhs (to update the value) or rhs (to get the value). If we are going to update the value, R[@0x1] = R {value: 1} is equivalent to *(borrow_global_mut<R>(@0x1)) = R {value: 1}. When appearing on the rhs, R[@0x1] is equivalent to *(borrow_global<R>(@0x1)) to get the actual value.

(borrow_global_mut<R>(@0x1)) = R {value: 1}

I didn't know you can assign resources like that.

Still, isn't it too much sugar though, too many implicit rules? I can understand the magic of vector v[0] considering that it's restricted only to the items with copy, so you can do auto-borrow/auto-deref without issues.
But here, there's a lot of magic where it's been none before.

Not sure whether it is a typo but you need to use *(borrow_global_mut<R>(@0x1)) = R {value: 1} to update the value

Thanks for the feedback @mkurnikov! Any comments/suggestions on this? @wrwg @vineethk

mkurnikov · 2024-07-02T23:42:03Z

I would suggest splitting the PR into two: one for the vector indexing, and another one for the resource access. Those are different features of the compiler v2, and from my POV the design work for them needs to happen separately.

wrwg · 2024-07-02T23:59:10Z

I would suggest splitting the PR into two: one for the vector indexing, and another one for the resource access. Those are different features of the compiler v2, and from my POV the design work for them needs to happen separately.

Lets not overcomplicate things here and create extra work for no value. The features have some relation (both based on index notation). Makes perfect sense to be in one PR.

wrwg · 2024-07-03T04:11:01Z

third_party/move/move-prover/tests/sources/functional/resource_index.exp

@@ -0,0 +1,6 @@
+Move prover returns: exiting with model building errors
+error: not supported before language version `2.0-unstable`: resource indexing


Can you align the error message with those emitted by parser/expansion elsewhere? You have some tests which show how they look like in this PR. It mentions 'Move 2' instead of unstable.

In general it appears though the prover tests should turn on language version 2 to get rid of those errors?

wrwg · 2024-07-03T04:11:37Z

third_party/move/move-prover/tests/testsuite.rs

@@ -168,6 +169,7 @@ fn test_runner_for_feature(path: &Path, feature: &Feature) -> datatest_stable::R

    let mut error_writer = Buffer::no_color();
    let result = if feature.v2 {
+        options.language_version = Some(LanguageVersion::V2_0);


So why do we see the resource index error then?

By adding this, error will only be generated for the newly added test case third_party/move/move-prover/tests/sources/functional/resource_index.move when running prover with V1. No error is generated for V2.

Updated the code so that the error disappears.

0x1 · 2024-07-04T16:48:29Z

lgtm

brmataptos

Generally pretty good, could use a little more abstraction in the code. Helper function for function lookup.

brmataptos · 2024-07-03T23:07:56Z

third_party/move/move-compiler-v2/tests/ability-check/index_ability_err_2.exp

@@ -0,0 +1,7 @@
+
+Diagnostics:
+error: resource indexing can only applied to a resource type


Are we all supposed to know that "resource type" is the same as "struct type which has key"? Or can we add that clarification?

error: resource indexing can only applied to a resource type (a struct type which has key)

brmataptos · 2024-07-05T23:22:18Z

third_party/move/move-compiler/src/expansion/ast.rs

@@ -487,7 +487,7 @@ pub enum Exp_ {

    Borrow(bool, Box<Exp>),
    ExpDotted(Box<ExpDotted>),
-    Index(Box<Exp>, Box<Exp>), // spec only (no mutation needed right now)
+    Index(Box<Exp>, Box<Exp>), // spec only for language version V1


Comment would be clearer as

spec only unless language >= v2

brmataptos · 2024-07-08T18:52:00Z

third_party/move/move-model/src/builder/exp_builder.rs

+        } else {
+            "vector::borrow"
+        };
+        // TODO: comment out because of #13888


Is this TODO obsolete now that #13888 is closed?

brmataptos · 2024-07-08T18:57:31Z

third_party/move/move-model/src/builder/exp_builder.rs

+        if self.had_errors {
+            return self.new_error_exp();
+        }
+        let target_str = if mutable {


Please abstract out a function get_vector_borrow(bool) -> FunctionId or similar to make this code more intelligible. We might want to add a TODO to make a Lazy const for the 2 options so that we don't go searching in case this happens to be common.

brmataptos · 2024-07-08T19:08:34Z

third_party/move/move-compiler-v2/tests/file-format-generator/index_acquires_err.exp

+   │         ^^^^^^^^^^^^^^^^^^^^^^^^^^
+15 │         let addr = @0x1;
+16 │         assert!((&0x42::test::Y<0x42::test::X<bool>>[addr]).field.value == true, 1);
+   │                 ------------------------------------------- called here


It would be useful to be specific about the operation here. global resource test::Y<..> at addr borrowed here. Or even borrow_global<0x42::test::Y<0x42::test::X<bool>>(addr) called here. With the use of [] it's really unclear what could be "called" here.

Currently we don't distinguish different storage operations: https://github.com/aptos-labs/aptos-core/blob/main/third_party/move/move-compiler-v2/src/function_checker.rs#L106. We want to revisit this later but I don't think we need to address it in this PR.

brmataptos · 2024-07-08T19:13:05Z

third_party/move/move-compiler-v2/tests/checking-lang-v1/index.exp

+   ┌─ tests/checking-lang-v1/index.move:11:17
+   │
+11 │         assert!((test::R[@0x1]).value == true, 0);
+   │                 ^^^^^^^^^^^^^^^ `_[_]` index operator in non-specification code only allowed in Move 2


It might be nice if we could stop giving these errors after giving a few (5, maybe). The general idea has been received; the user must have run the wrong compiler or had the wrong idea.

But I guess that's pretty low priority.

Repeated error messages is a general issue which definitely should to be addressed but maybe separately.

third_party/move/move-compiler/src/expansion/translate.rs

github-actions · 2024-07-09T04:47:02Z

✅ Forge suite `compat` success on `1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5` ==> `7ad3d5121a72b931024612614114282362a4a7c5`

Compatibility test results for 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5 ==> 7ad3d5121a72b931024612614114282362a4a7c5 (PR)
1. Check liveness of validators at old version: 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5
compatibility::simple-validator-upgrade::liveness-check : committed: 7966.499651574535 txn/s, latency: 3761.78392007926 ms, (p50: 2700 ms, p90: 6300 ms, p99: 23000 ms), latency samples: 302800
2. Upgrading first Validator to new version: 7ad3d5121a72b931024612614114282362a4a7c5
compatibility::simple-validator-upgrade::single-validator-upgrading : committed: 1383.886726834085 txn/s, latency: 21874.328848870056 ms, (p50: 26600 ms, p90: 31500 ms, p99: 31900 ms), latency samples: 56640
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 3385.140233180243 txn/s, latency: 9116.169707490028 ms, (p50: 10000 ms, p90: 13700 ms, p99: 14300 ms), latency samples: 135380
3. Upgrading rest of first batch to new version: 7ad3d5121a72b931024612614114282362a4a7c5
compatibility::simple-validator-upgrade::half-validator-upgrading : committed: 3342.87226484473 txn/s, latency: 7182.785307641778 ms, (p50: 8400 ms, p90: 9900 ms, p99: 12000 ms), latency samples: 91340
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 3335.522775883092 txn/s, latency: 9312.444782928624 ms, (p50: 9400 ms, p90: 14500 ms, p99: 15000 ms), latency samples: 135900
4. upgrading second batch to new version: 7ad3d5121a72b931024612614114282362a4a7c5
compatibility::simple-validator-upgrade::rest-validator-upgrading : committed: 3930.970805616557 txn/s, latency: 5853.569917050691 ms, (p50: 4900 ms, p90: 12500 ms, p99: 15300 ms), latency samples: 108500
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 6207.220904489957 txn/s, latency: 5235.061959036764 ms, (p50: 4900 ms, p90: 9300 ms, p99: 10300 ms), latency samples: 233380
5. check swarm health
Compatibility test for 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5 ==> 7ad3d5121a72b931024612614114282362a4a7c5 passed
Test Ok

github-actions · 2024-07-09T04:47:28Z

✅ Forge suite `realistic_env_max_load` success on `7ad3d5121a72b931024612614114282362a4a7c5`

two traffics test: inner traffic : committed: 8539.10027174689 txn/s, latency: 4585.719204110057 ms, (p50: 4500 ms, p90: 6000 ms, p99: 10200 ms), latency samples: 3690460
two traffics test : committed: 100.07283888063971 txn/s, latency: 2146.3241573033706 ms, (p50: 2100 ms, p90: 2300 ms, p99: 7600 ms), latency samples: 1780
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.215, avg: 0.212", "QsPosToProposal: max: 0.192, avg: 0.183", "ConsensusProposalToOrdered: max: 0.300, avg: 0.288", "ConsensusOrderedToCommit: max: 0.360, avg: 0.350", "ConsensusProposalToCommit: max: 0.649, avg: 0.638"]
Max round gap was 1 [limit 4] at version 1757920. Max no progress secs was 4.971339 [limit 15] at version 1757920.
Test Ok

github-actions · 2024-07-09T04:49:25Z

✅ Forge suite `framework_upgrade` success on `1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5` ==> `7ad3d5121a72b931024612614114282362a4a7c5`

Compatibility test results for 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5 ==> 7ad3d5121a72b931024612614114282362a4a7c5 (PR)
Upgrade the nodes to version: 7ad3d5121a72b931024612614114282362a4a7c5
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1166.9266599890811 txn/s, submitted: 1169.304261170803 txn/s, failed submission: 2.3776011817218445 txn/s, expired: 2.3776011817218445 txn/s, latency: 2729.9778830480845 ms, (p50: 1800 ms, p90: 5400 ms, p99: 9600 ms), latency samples: 98160
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1058.5065027009487 txn/s, submitted: 1061.2084976025465 txn/s, failed submission: 2.7019949015978266 txn/s, expired: 2.7019949015978266 txn/s, latency: 2803.1366836843226 ms, (p50: 2100 ms, p90: 5300 ms, p99: 10200 ms), latency samples: 94020
5. check swarm health
Compatibility test for 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5 ==> 7ad3d5121a72b931024612614114282362a4a7c5 passed
Upgrade the remaining nodes to version: 7ad3d5121a72b931024612614114282362a4a7c5
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1166.8647052286196 txn/s, submitted: 1168.4863158969936 txn/s, failed submission: 1.621610668374099 txn/s, expired: 1.621610668374099 txn/s, latency: 2657.4659916617034 ms, (p50: 2100 ms, p90: 4800 ms, p99: 8700 ms), latency samples: 100740
Test Ok

rahxephon89 force-pushed the teng/index branch 7 times, most recently from bb7762d to de2c263 Compare June 15, 2024 02:23

rahxephon89 changed the title ~~[WIP][Compiler-v2] Index notation~~ [Compiler-v2][Feature] Index notation Jun 15, 2024

rahxephon89 marked this pull request as ready for review June 15, 2024 02:59

rahxephon89 requested review from brmataptos, wrwg, vineethk and fEst1ck June 15, 2024 03:00

rahxephon89 force-pushed the teng/index branch from de2c263 to 7cfdf3a Compare June 16, 2024 05:20

brmataptos reviewed Jun 18, 2024

View reviewed changes

third_party/move/move-compiler-v2/tests/checking/typing/index_err.exp Outdated Show resolved Hide resolved

third_party/move/move-prover/tests/sources/functional/resource_index.exp Outdated Show resolved Hide resolved

rahxephon89 force-pushed the teng/index branch 3 times, most recently from ced14fb to 2f21675 Compare June 18, 2024 06:50

rahxephon89 commented Jun 18, 2024

View reviewed changes

third_party/move/move-prover/tests/sources/functional/resource_index.exp Outdated Show resolved Hide resolved

third_party/move/tools/move-package/tests/test_sources/compilation/std-lib-conflicts/Move.exp Outdated Show resolved Hide resolved

rahxephon89 requested a review from brmataptos June 18, 2024 15:44

brmataptos reviewed Jun 19, 2024

View reviewed changes

rahxephon89 requested a review from brmataptos June 19, 2024 00:29

wrwg reviewed Jun 19, 2024

View reviewed changes

rahxephon89 force-pushed the teng/index branch from 4ac11b8 to f6de658 Compare June 19, 2024 04:41

rahxephon89 force-pushed the teng/index branch from f6de658 to 1f81f22 Compare June 20, 2024 00:25

rahxephon89 force-pushed the teng/index branch from 1b199fb to 6023afe Compare July 2, 2024 07:29

rahxephon89 requested review from vineethk, wrwg and brmataptos July 2, 2024 08:17

rahxephon89 commented Jul 2, 2024

View reviewed changes

mkurnikov reviewed Jul 2, 2024

View reviewed changes

wrwg approved these changes Jul 3, 2024

View reviewed changes

rahxephon89 force-pushed the teng/index branch from 1f6a03d to 9858c6d Compare July 6, 2024 21:02

brmataptos approved these changes Jul 8, 2024

View reviewed changes

vineethk approved these changes Jul 9, 2024

View reviewed changes

third_party/move/move-compiler/src/expansion/translate.rs Outdated Show resolved Hide resolved

rahxephon89 added 3 commits July 8, 2024 21:09

add indexing operator

371570b

unused alias error

ede2a8f

fix package test

16f59a3

rahxephon89 force-pushed the teng/index branch from de25a66 to 528f7f8 Compare July 9, 2024 04:10

temp

7ad3d51

rahxephon89 force-pushed the teng/index branch from 528f7f8 to 7ad3d51 Compare July 9, 2024 04:13

rahxephon89 enabled auto-merge (squash) July 9, 2024 04:15

This comment has been minimized.

Sign in to view

rahxephon89 merged commit 5b20c16 into main Jul 9, 2024
87 of 91 checks passed

rahxephon89 deleted the teng/index branch July 9, 2024 04:57

		@@ -0,0 +1,6 @@
		Move prover returns: exiting with model building errors
		error: not supported before language version `2.0-unstable`: resource indexing

		@@ -0,0 +1,7 @@

		Diagnostics:
		error: resource indexing can only applied to a resource type

[Compiler-v2][Feature] Index notation #13695

[Compiler-v2][Feature] Index notation #13695

Conversation

rahxephon89 commented Jun 14, 2024 • edited Loading

Description

Type of Change

Which Components or Systems Does This Change Impact?

How Has This Been Tested?

Key Areas to Review

Checklist

trunk-io bot commented Jun 14, 2024 • edited Loading

codecov bot commented Jun 14, 2024 • edited Loading

Codecov Report

brmataptos left a comment

Choose a reason for hiding this comment

rahxephon89 commented Jun 19, 2024 • edited Loading

brmataptos commented Jun 19, 2024

wrwg commented Jun 19, 2024

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

mkurnikov commented Jul 2, 2024

rahxephon89 commented Jul 2, 2024 • edited Loading

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

mkurnikov Jul 2, 2024 • edited Loading

Choose a reason for hiding this comment

rahxephon89 Jul 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rahxephon89 Jul 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkurnikov commented Jul 2, 2024

wrwg commented Jul 2, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rahxephon89 Jul 3, 2024 • edited Loading

Choose a reason for hiding this comment

0x1 commented Jul 4, 2024

brmataptos 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Jul 9, 2024

✅ Forge suite compat success on 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5 ==> 7ad3d5121a72b931024612614114282362a4a7c5

github-actions bot commented Jul 9, 2024

✅ Forge suite realistic_env_max_load success on 7ad3d5121a72b931024612614114282362a4a7c5

github-actions bot commented Jul 9, 2024

✅ Forge suite framework_upgrade success on 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5 ==> 7ad3d5121a72b931024612614114282362a4a7c5

rahxephon89 commented Jun 14, 2024 •

edited

Loading

trunk-io bot commented Jun 14, 2024 •

edited

Loading

codecov bot commented Jun 14, 2024 •

edited

Loading

rahxephon89 commented Jun 19, 2024 •

edited

Loading

rahxephon89 commented Jul 2, 2024 •

edited

Loading

mkurnikov Jul 2, 2024 •

edited

Loading

rahxephon89 Jul 2, 2024 •

edited

Loading

rahxephon89 Jul 2, 2024 •

edited

Loading

rahxephon89 Jul 3, 2024 •

edited

Loading

✅ Forge suite `compat` success on `1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5` ==> `7ad3d5121a72b931024612614114282362a4a7c5`

✅ Forge suite `realistic_env_max_load` success on `7ad3d5121a72b931024612614114282362a4a7c5`

✅ Forge suite `framework_upgrade` success on `1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5` ==> `7ad3d5121a72b931024612614114282362a4a7c5`