Repository: aptos-labs/aptos-core (language: python)

Issue:
[compiler-v2] Field selection after resource indexing not mutable
Consider code as below:

```
    package fun deposit(account: address, coin: Coin) acquires Account {
        // What I have to write
        //(&mut Account[account].balance).merge(coin)
        // What I wish to write but got error message
        Account[account].balance.merge(coin)
    }

where fun merge(self: &mut Coin, Coin)
```

The produced error message is

```
error: cannot mutably borrow from an immutable ref
   ┌─ /Users/wrwg/move-demo/sources/account.move:20:9
   │
20 │         Account[account].balance.merge(coin)
   │         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

```

It appears that the decision to imm-ref the index `Account[account]` is taken before its known that it needs to be mutable. This might be a difficult problem to fix, but its a bit of annoying.


Discussion / hints:

Fix patch:
diff --git a/third_party/move/move-model/src/builder/exp_builder.rs b/third_party/move/move-model/src/builder/exp_builder.rs
index f47792adfd337..9b6427c19a260 100644
--- a/third_party/move/move-model/src/builder/exp_builder.rs
+++ b/third_party/move/move-model/src/builder/exp_builder.rs
@@ -31,7 +31,7 @@ use codespan_reporting::diagnostic::Severity;
 use itertools::Itertools;
 use move_binary_format::file_format::{self, Ability, AbilitySet};
 use move_compiler::{
-    expansion::ast::{self as EA},
+    expansion::ast::{self as EA, Exp_},
     hlir::ast as HA,
     naming::ast as NA,
     parser::ast::{self as PA, CallKind, Field},
@@ -4136,7 +4136,7 @@ impl<'env, 'translator, 'module_translator> ExpTranslator<'env, 'translator, 'mo
         context: &ErrorMessageContext,
     ) -> ExpData {
         // Translate arguments.
-        let (arg_types, translated_args) = self.translate_exp_list(args);
+        let (mut arg_types, mut translated_args) = self.translate_exp_list(args);
 
         // Special handling of receiver call functions
         if kind == CallKind::Receiver {
@@ -4144,6 +4144,50 @@ impl<'env, 'translator, 'module_translator> ExpTranslator<'env, 'translator, 'mo
                 module.is_none(),
                 "unexpected qualified name in receiver call"
             );
+            debug_assert!(
+                !args.is_empty(),
+                "receiver call needs to have at least one parameter"
+            );
+            let receiver_call_opt = self.get_receiver_function(&arg_types[0], name);
+            if let Some(receiver_call) = receiver_call_opt {
+                if let Exp_::ExpDotted(dotted) = &args[0].value {
+                    // we need a special case for the receiver call S[x].f.fun(&mut...)
+                    // when the first argument is a dotted expression with index notation:
+                    // S[x].y because the reference type is by default set immutable ref
+                    if receiver_call.arg_types[0].is_mutable_reference() {
+                        let first_arg = self.translate_dotted(
+                            dotted,
+                            &arg_types[0],
+                            true,
+                            &ErrorMessageContext::General,
+                        );
+                        translated_args[0] = first_arg.into_exp();
+                    }
+                } else if let Exp_::Index(target, index) = &args[0].value {
+                    // special case for the receiver call S[x].fun(&...), S[x].fun(&mut...)
+                    // so that it behaves the same as (&S[x]).fun(&...), (&mut S[x]).fun(&mut...)
+                    if receiver_call.arg_types[0].is_reference() {
+                        let index_mutate = receiver_call.arg_types[0].is_mutable_reference();
+                        if let Some(first_arg) = self.try_resource_or_vector_index(
+                            loc,
+                            target,
+                            index,
+                            &ErrorMessageContext::General,
+                            &Type::Reference(
+                                ReferenceKind::from_is_mut(index_mutate),
+                                Box::new(arg_types[0].clone()),
+                            ),
+                            index_mutate,
+                        ) {
+                            translated_args[0] = first_arg.into_exp();
+                            arg_types[0] = Type::Reference(
+                                ReferenceKind::from_is_mut(index_mutate),
+                                Box::new(arg_types[0].clone()),
+                            );
+                        }
+                    }
+                }
+            }
             return self.translate_receiver_call(
                 loc,
                 name,


Test patch:
diff --git a/third_party/move/move-compiler-v2/tests/checking/receiver/calls_index.exp b/third_party/move/move-compiler-v2/tests/checking/receiver/calls_index.exp
new file mode 100644
index 0000000000000..643cdd7d3ac08
--- /dev/null
+++ b/third_party/move/move-compiler-v2/tests/checking/receiver/calls_index.exp
@@ -0,0 +1,176 @@
+// -- Model dump before bytecode pipeline
+module 0x42::m {
+    struct T {
+        w: W,
+    }
+    struct S {
+        t: T,
+    }
+    struct W {
+        x: u64,
+    }
+    struct Wrapper<T> {
+        inner: T,
+    }
+    private fun boo(v: vector<S>,w: W) {
+        m::merge(Borrow(Mutable)(select m::T.w<T>(select m::S.t<S>(vector::borrow_mut<S>(Borrow(Mutable)(v), 0)))), w)
+    }
+    private fun boo_(v: vector<S>,w: W) {
+        m::merge(Borrow(Mutable)(select m::T.w<T>(select m::S.t<S>(vector::borrow_mut<S>(Borrow(Mutable)(v), 0)))), w)
+    }
+    private fun boo_greater(v: vector<S>,w: W): bool {
+        m::greater(Borrow(Immutable)(select m::T.w<T>(select m::S.t<S>(vector::borrow<S>(Borrow(Immutable)(v), 0)))), w)
+    }
+    private fun boo_greater_(v: vector<S>,w: W): bool {
+        m::greater(Freeze(false)(Borrow(Mutable)(select m::T.w<T>(select m::S.t<S>(vector::borrow_mut<S>(Borrow(Mutable)(v), 0))))), w)
+    }
+    private fun boo_greater_2(v: vector<W>,w: W): bool {
+        m::greater(vector::borrow<W>(Borrow(Immutable)(v), 0), w)
+    }
+    private fun boo_greater_2_(v: vector<W>,w: W): bool {
+        m::greater(vector::borrow<W>(Borrow(Immutable)(v), 0), w)
+    }
+    private fun dispatch<T>(account: address): T
+        acquires Wrapper(*)
+     {
+        m::unwrap<T>(BorrowGlobal(Immutable)<Wrapper<T>>(account))
+    }
+    private fun foo(account: address,w: W)
+        acquires S(*)
+     {
+        m::merge(Borrow(Mutable)(select m::T.w<T>(select m::S.t<S>(BorrowGlobal(Mutable)<S>(account)))), w)
+    }
+    private fun foo_(account: address,w: W)
+        acquires S(*)
+     {
+        m::merge(Borrow(Mutable)(select m::T.w<T>(select m::S.t<S>(BorrowGlobal(Mutable)<S>(account)))), w)
+    }
+    private fun foo_2(account: address,w: W)
+        acquires W(*)
+     {
+        m::merge(BorrowGlobal(Mutable)<W>(account), w)
+    }
+    private fun foo_3(account: address,w: W)
+        acquires W(*)
+     {
+        m::merge(BorrowGlobal(Mutable)<W>(account), w)
+    }
+    private fun foo_greater(account: address,w: W): bool
+        acquires S(*)
+     {
+        m::greater(Borrow(Immutable)(select m::T.w<T>(select m::S.t<S>(BorrowGlobal(Immutable)<S>(account)))), w)
+    }
+    private fun foo_greater_(account: address,w: W): bool
+        acquires S(*)
+     {
+        m::greater(Freeze(false)(Borrow(Mutable)(select m::T.w<T>(select m::S.t<S>(BorrowGlobal(Mutable)<S>(account))))), w)
+    }
+    private fun foo_greater_2(account: address,w: W): bool
+        acquires S(*)
+     {
+        m::greater(BorrowGlobal(Immutable)<W>(account), w)
+    }
+    private fun greater(self: &W,s: W): bool {
+        Gt<u64>(select m::W.x<&W>(self), select m::W.x<W>(s))
+    }
+    private fun merge(self: &mut W,s: W) {
+        {
+          let $t2: u64 = select m::W.x<W>(s);
+          {
+            let $t1: &mut u64 = Borrow(Mutable)(select m::W.x<&mut W>(self));
+            $t1 = Add<u64>(Deref($t1), $t2)
+          }
+        };
+        Tuple()
+    }
+    private fun unwrap<T>(self: &Wrapper<T>): T {
+        select m::Wrapper.inner<&Wrapper<T>>(self)
+    }
+} // end 0x42::m
+
+// -- Sourcified model before bytecode pipeline
+module 0x42::m {
+    struct T has drop, store, key {
+        w: W,
+    }
+    struct S has drop, key {
+        t: T,
+    }
+    struct W has drop, store, key {
+        x: u64,
+    }
+    struct Wrapper<T: copy> has copy, drop, store, key {
+        inner: T,
+    }
+    fun boo(v: vector<S>, w: W) {
+        merge(&mut 0x1::vector::borrow_mut<S>(&mut v, 0).t.w, w)
+    }
+    fun boo_(v: vector<S>, w: W) {
+        merge(&mut 0x1::vector::borrow_mut<S>(&mut v, 0).t.w, w)
+    }
+    fun boo_greater(v: vector<S>, w: W): bool {
+        greater(&0x1::vector::borrow<S>(&v, 0).t.w, w)
+    }
+    fun boo_greater_(v: vector<S>, w: W): bool {
+        greater(/*freeze*/&mut 0x1::vector::borrow_mut<S>(&mut v, 0).t.w, w)
+    }
+    fun boo_greater_2(v: vector<W>, w: W): bool {
+        greater(0x1::vector::borrow<W>(&v, 0), w)
+    }
+    fun boo_greater_2_(v: vector<W>, w: W): bool {
+        greater(0x1::vector::borrow<W>(&v, 0), w)
+    }
+    fun dispatch<T: copy + store>(account: address): T
+        acquires Wrapper
+    {
+        unwrap<T>(borrow_global<Wrapper<T>>(account))
+    }
+    fun foo(account: address, w: W)
+        acquires S
+    {
+        merge(&mut borrow_global_mut<S>(account).t.w, w)
+    }
+    fun foo_(account: address, w: W)
+        acquires S
+    {
+        merge(&mut borrow_global_mut<S>(account).t.w, w)
+    }
+    fun foo_2(account: address, w: W)
+        acquires W
+    {
+        merge(borrow_global_mut<W>(account), w)
+    }
+    fun foo_3(account: address, w: W)
+        acquires W
+    {
+        merge(borrow_global_mut<W>(account), w)
+    }
+    fun foo_greater(account: address, w: W): bool
+        acquires S
+    {
+        greater(&borrow_global<S>(account).t.w, w)
+    }
+    fun foo_greater_(account: address, w: W): bool
+        acquires S
+    {
+        greater(/*freeze*/&mut borrow_global_mut<S>(account).t.w, w)
+    }
+    fun foo_greater_2(account: address, w: W): bool
+        acquires S
+    {
+        greater(borrow_global<W>(account), w)
+    }
+    fun greater(self: &W, s: W): bool {
+        self.x > s.x
+    }
+    fun merge(self: &mut W, s: W) {
+        {
+            let $t2 = s.x;
+            let $t1 = &mut self.x;
+            *$t1 = *$t1 + $t2
+        };
+    }
+    fun unwrap<T: copy>(self: &Wrapper<T>): T {
+        self.inner
+    }
+}
diff --git a/third_party/move/move-compiler-v2/tests/checking/receiver/calls_index.move b/third_party/move/move-compiler-v2/tests/checking/receiver/calls_index.move
new file mode 100644
index 0000000000000..8f0726ce369a1
--- /dev/null
+++ b/third_party/move/move-compiler-v2/tests/checking/receiver/calls_index.move
@@ -0,0 +1,85 @@
+module 0x42::m {
+
+    struct S has key, drop { t: T }
+
+    struct T has key, store, drop {
+        w: W
+    }
+
+    struct W has key, store, drop {
+        x: u64
+    }
+
+    fun merge(self: &mut W, s: W) {
+        self.x += s.x;
+    }
+
+    fun foo(account: address, w: W) acquires S {
+        S[account].t.w.merge(w)
+    }
+
+    fun foo_2(account: address, w: W) acquires W {
+        W[account].merge(w)
+    }
+
+    fun foo_3(account: address, w: W) acquires W {
+        borrow_global_mut<W>(account).merge(w)
+    }
+
+    fun boo(v: vector<S>, w: W) {
+        v[0].t.w.merge(w)
+    }
+
+    fun foo_(account: address, w: W) acquires S {
+        (&mut S[account].t.w).merge(w)
+    }
+
+    fun boo_(v: vector<S>, w: W) {
+        (&mut v[0].t.w).merge(w)
+    }
+
+    fun greater(self: &W, s: W): bool {
+        self.x > s.x
+    }
+
+    fun foo_greater(account: address, w: W): bool acquires S {
+        S[account].t.w.greater(w)
+    }
+
+    fun foo_greater_2(account: address, w: W): bool acquires S {
+        W[account].greater(w)
+    }
+
+    fun boo_greater(v: vector<S>, w: W): bool {
+        v[0].t.w.greater(w)
+    }
+
+    fun boo_greater_2(v: vector<W>, w: W): bool {
+        v[0].greater(w)
+    }
+
+    fun foo_greater_(account: address, w: W): bool acquires S {
+        (&mut S[account].t.w).greater(w)
+    }
+
+    fun boo_greater_(v: vector<S>, w: W): bool {
+        (&mut v[0].t.w).greater(w)
+    }
+
+    fun boo_greater_2_(v: vector<W>, w: W): bool {
+        (&v[0]).greater(w)
+    }
+
+    struct Wrapper<T: copy> has drop, key, store, copy {
+        inner: T
+    }
+
+    fun unwrap<T: copy>(self: &Wrapper<T>): T {
+        self.inner
+    }
+
+    fun dispatch<T: store + copy>(account: address): T acquires Wrapper {
+        Wrapper<T>[account].unwrap()
+    }
+
+}
diff --git a/third_party/move/move-compiler-v2/tests/checking/receiver/calls_index_errors.exp b/third_party/move/move-compiler-v2/tests/checking/receiver/calls_index_errors.exp
new file mode 100644
index 0000000000000..48bf56fc176e2
--- /dev/null
+++ b/third_party/move/move-compiler-v2/tests/checking/receiver/calls_index_errors.exp
@@ -0,0 +1,19 @@
+
+Diagnostics:
+error: undeclared receiver function `merge_non_receiver` for type `W`
+   ┌─ tests/checking/receiver/calls_index_errors.move:22:9
+   │
+22 │         S[account].t.w.merge_non_receiver(w)
+   │         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: the function takes `&mut` but `&` was provided
+   ┌─ tests/checking/receiver/calls_index_errors.move:26:9
+   │
+26 │         (&S[account].t.w).merge(w)
+   │         ^^^^^^^^^^^^^^^^^
+
+error: the function takes `&mut` but `&` was provided
+   ┌─ tests/checking/receiver/calls_index_errors.move:30:9
+   │
+30 │         (&v[0].t.w).merge(w)
+   │         ^^^^^^^^^^^
diff --git a/third_party/move/move-compiler-v2/tests/checking/receiver/calls_index_errors.move b/third_party/move/move-compiler-v2/tests/checking/receiver/calls_index_errors.move
new file mode 100644
index 0000000000000..bb498baf61707
--- /dev/null
+++ b/third_party/move/move-compiler-v2/tests/checking/receiver/calls_index_errors.move
@@ -0,0 +1,33 @@
+module 0x42::m {
+
+    struct S has key, drop { t: T }
+
+    struct T has store, drop {
+        w: W
+    }
+
+    struct W has store, drop {
+        x: u64
+    }
+
+    fun merge(self: &mut W, s: W) {
+        self.x += s.x;
+    }
+
+    fun merge_non_receiver(self1: &mut W, s: W) {
+        self1.x += s.x;
+    }
+
+    fun foo_non_receiver(account: address, w: W) acquires S {
+        S[account].t.w.merge_non_receiver(w)
+    }
+
+    fun foo_(account: address, w: W) acquires S {
+        (&S[account].t.w).merge(w)
+    }
+
+    fun boo_(v: vector<S>, w: W) {
+        (&v[0].t.w).merge(w)
+    }
+
+}
diff --git a/third_party/move/move-compiler-v2/transactional-tests/tests/no-v1-comparison/index.exp b/third_party/move/move-compiler-v2/transactional-tests/tests/no-v1-comparison/index.exp
index 996ef686af3f8..eb1ba78aa4fa9 100644
--- a/third_party/move/move-compiler-v2/transactional-tests/tests/no-v1-comparison/index.exp
+++ b/third_party/move/move-compiler-v2/transactional-tests/tests/no-v1-comparison/index.exp
@@ -1,1 +1,1 @@
-processed 25 tasks
+processed 29 tasks
diff --git a/third_party/move/move-compiler-v2/transactional-tests/tests/no-v1-comparison/index.move b/third_party/move/move-compiler-v2/transactional-tests/tests/no-v1-comparison/index.move
index c365bd45a1e3a..66093ecfc1e56 100644
--- a/third_party/move/move-compiler-v2/transactional-tests/tests/no-v1-comparison/index.move
+++ b/third_party/move/move-compiler-v2/transactional-tests/tests/no-v1-comparison/index.move
@@ -268,6 +268,99 @@ module 0x42::test {
         assert!(y[0] == 2, 0);
     }
 
+    // Test receiver call
+
+    struct S has key, drop, copy { t: T }
+
+    struct T has key, store, drop, copy {
+        w: W
+    }
+
+    struct W has key, store, drop, copy {
+        x: u64
+    }
+
+    fun init_receiver(signer: &signer) {
+        let w = W {
+            x: 2
+        };
+        let t = T {
+            w
+        };
+        let s = S {
+            t
+        };
+        move_to(signer, w);
+        move_to(signer, s);
+    }
+
+    fun merge(self: &mut W, s: W) {
+        self.x += s.x;
+    }
+
+    fun greater(self: &W, s: W): bool {
+        self.x > s.x
+    }
+
+    fun foo_1(account: address, w: W) acquires S {
+        S[account].t.w.merge(w)
+    }
+
+    fun foo_2(account: address, w: W) acquires W {
+        W[account].merge(w)
+    }
+
+    fun boo_1(v: vector<S>, w: W): u64 {
+        v[0].t.w.merge(w);
+        v[0].t.w.x
+    }
+
+    fun boo_2(v: vector<W>, w: W) {
+        v[0].merge(w);
+        assert!(v[0].x == 8, 0);
+    }
+
+    fun test_receiver() {
+        let w = W {
+            x: 3
+        };
+        assert!(!W[@0x1].greater(w), 0);
+        foo_1(@0x1, w);
+        assert!(S[@0x1].t.w.x == 5, 0);
+        assert!(boo_1(vector[S[@0x1]], w) == 8, 1);
+        foo_2(@0x1, w);
+        assert!(W[@0x1].x == 5, 0);
+        boo_2(vector[W[@0x1]], w);
+    }
+
+    struct Wrapper<T: copy> has drop, key, store, copy {
+        inner: T
+    }
+
+    fun unwrap<T: copy>(self: &Wrapper<T>): T {
+        self.inner
+    }
+
+    fun dispatch<T: store + copy>(account: address): T acquires Wrapper {
+        Wrapper<T>[account].unwrap()
+    }
+
+    fun init_receiver_2(signer: &signer) {
+        let wrapper = Wrapper {
+            inner: 2
+        };
+        move_to(signer, wrapper);
+    }
+
+    fun test_receiver_2() {
+        assert!(dispatch(@0x1) == 2, 0);
+        let wrapper = Wrapper {
+            inner: 2
+        };
+        let v = vector[wrapper];
+        assert!(v[0].unwrap() == 2, 0);
+    }
+
 }
 
 //# run --verbose --signers 0x1 -- 0x42::test::init
@@ -317,3 +410,11 @@ module 0x42::test {
 //# run --verbose -- 0x42::test::test_index_then_field_select_3
 
 //# run --verbose -- 0x42::test::inc_vec_new_test
+
+//# run --verbose --signers 0x1 -- 0x42::test::init_receiver
+
+//# run --verbose -- 0x42::test::test_receiver
+
+//# run --verbose --signers 0x1 -- 0x42::test::init_receiver_2
+
+//# run --verbose -- 0x42::test::test_receiver_2