[move-2024][easy] Disallow macro variables to be used as exp dotted b…

…ase expressions (#16313)

## Description 

What it says on the tin -- this is just to reduce programmer confusion

## Test Plan 

New test case plus updated expectations

---
If your changes are not user-facing and do not break anything, you can
skip the following section. Otherwise, please briefly describe what has
changed under the Release Notes section.

### Type of Change (Check all that apply)

- [ ] protocol change
- [ ] user-visible impact
- [ ] breaking change for a client SDKs
- [ ] breaking change for FNs (FN binary must upgrade)
- [ ] breaking change for validators or node operators (must upgrade
binaries)
- [ ] breaking change for on-chain data layout
- [ ] necessitate either a data wipe or data migration

### Release notes

external-crates/move/crates/move-compiler/src/naming/ast.rs

@@ -844,6 +844,10 @@ impl Var_ {
         P::Var::starts_with_underscore_name(self.name)
     }
 
+    pub fn is_syntax_identifier(&self) -> bool {
+        P::Var::is_syntax_identifier_name(self.name)
+    }
+
     pub fn is_valid(&self) -> bool {
         P::Var::is_valid_name(self.name)
     }

external-crates/move/crates/move-compiler/src/naming/translate.rs

@@ -1826,6 +1826,24 @@ fn dotted(context: &mut Context, edot: E::ExpDotted) -> Option<N::ExpDotted> {
             let ne = exp(context, e);
             match &ne.value {
                 N::Exp_::UnresolvedError => return None,
+                N::Exp_::Var(n) if n.value.is_syntax_identifier() => {
+                    let mut diag = diag!(
+                        NameResolution::NamePositionMismatch,
+                        (n.loc, "Macro parameters are not allowed to appear in paths")
+                    );
+                    diag.add_note(format!(
+                        "To use a macro parameter as a value in a path expression, first bind \
+                            it to a local variable, e.g. 'let {0} = ${0};'",
+                        &n.value.name.to_string()[1..]
+                    ));
+                    diag.add_note(
+                        "Macro parameters are always treated as value expressions, and are not \
+                        modified by path operations.\n\
+                        Path operations include 'move', 'copy', '&', '&mut', and field references",
+                    );
+                    context.env.add_diag(diag);
+                    N::ExpDotted_::Exp(Box::new(sp(ne.loc, N::Exp_::UnresolvedError)))
+                }
                 _ => N::ExpDotted_::Exp(ne),
             }
         }

external-crates/move/crates/move-compiler/tests/move_2024/typing/macro_arg_by_name_invalid_usage_value.exp

@@ -1,12 +1,20 @@
-error[E04026]: invalid 'copy' usage
-  ┌─ tests/move_2024/typing/macro_arg_by_name_invalid_usage_value.move:4:9
+error[E03006]: unexpected name in this position
+  ┌─ tests/move_2024/typing/macro_arg_by_name_invalid_usage_value.move:4:14
   │
 4 │         copy $x;
-  │         ^^^^ Invalid 'copy'. Expected a variable or path.
+  │              ^^ Macro parameters are not allowed to appear in paths
+  │
+  = To use a macro parameter as a value in a path expression, first bind it to a local variable, e.g. 'let x = $x;'
+  = Macro parameters are always treated as value expressions, and are not modified by path operations.
+    Path operations include 'move', 'copy', '&', '&mut', and field references
 
-error[E04027]: invalid 'move' usage
-  ┌─ tests/move_2024/typing/macro_arg_by_name_invalid_usage_value.move:5:9
+error[E03006]: unexpected name in this position
+  ┌─ tests/move_2024/typing/macro_arg_by_name_invalid_usage_value.move:5:14
   │
 5 │         move $x;
-  │         ^^^^ Invalid 'move'. Expected a variable or path.
+  │              ^^ Macro parameters are not allowed to appear in paths
+  │
+  = To use a macro parameter as a value in a path expression, first bind it to a local variable, e.g. 'let x = $x;'
+  = Macro parameters are always treated as value expressions, and are not modified by path operations.
+    Path operations include 'move', 'copy', '&', '&mut', and field references
 

external-crates/move/crates/move-compiler/tests/move_2024/typing/macro_arg_by_name_invalid_usage_value.move

@@ -1,8 +1,9 @@
 module a::m {
     // macro args are call-by-name, not all usages are valid
-    macro fun foo<$T>($x: $T) {
+    macro fun foo<$T>($x: $T): $T {
         copy $x;
         move $x;
+        $x
     }
 
     fun t() {

external-crates/move/crates/move-compiler/tests/move_2024/typing/macro_arg_by_name_invalid_usage_var.exp

@@ -1,12 +1,20 @@
-error[E04026]: invalid 'copy' usage
-  ┌─ tests/move_2024/typing/macro_arg_by_name_invalid_usage_var.move:4:9
+error[E03006]: unexpected name in this position
+  ┌─ tests/move_2024/typing/macro_arg_by_name_invalid_usage_var.move:4:14
   │
 4 │         copy $x;
-  │         ^^^^ Invalid 'copy'. Expected a variable or path.
+  │              ^^ Macro parameters are not allowed to appear in paths
+  │
+  = To use a macro parameter as a value in a path expression, first bind it to a local variable, e.g. 'let x = $x;'
+  = Macro parameters are always treated as value expressions, and are not modified by path operations.
+    Path operations include 'move', 'copy', '&', '&mut', and field references
 
-error[E04027]: invalid 'move' usage
-  ┌─ tests/move_2024/typing/macro_arg_by_name_invalid_usage_var.move:5:9
+error[E03006]: unexpected name in this position
+  ┌─ tests/move_2024/typing/macro_arg_by_name_invalid_usage_var.move:5:14
   │
 5 │         move $x;
-  │         ^^^^ Invalid 'move'. Expected a variable or path.
+  │              ^^ Macro parameters are not allowed to appear in paths
+  │
+  = To use a macro parameter as a value in a path expression, first bind it to a local variable, e.g. 'let x = $x;'
+  = Macro parameters are always treated as value expressions, and are not modified by path operations.
+    Path operations include 'move', 'copy', '&', '&mut', and field references
 

external-crates/move/crates/move-compiler/tests/move_2024/typing/macro_arg_by_name_invalid_usage_var.move

@@ -1,8 +1,9 @@
 module a::m {
     // macro args are call-by-name, not all usages are valid
-    macro fun foo<$T>($x: $T) {
+    macro fun foo<$T>($x: $T): $T {
         copy $x;
         move $x;
+        $x
     }
 
     fun t() {

external-crates/move/crates/move-compiler/tests/move_2024/typing/macro_arg_by_name_strange_usage.exp

@@ -1,21 +1,11 @@
-error[E06002]: use of unassigned variable
-   ┌─ tests/move_2024/typing/macro_arg_by_name_strange_usage.move:14:14
-   │
-14 │         foo!(x); // TODO improve this error message
-   │              ^
-   │              │
-   │              Invalid usage of previously moved variable 'x'.
-   │              Suggestion: use 'copy x' to avoid the move.
-   │              In a loop, this typically means it was moved in the first iteration, and is not available by the second iteration.
-
 error[E05001]: ability constraint not satisfied
-   ┌─ tests/move_2024/typing/macro_arg_by_name_strange_usage.move:17:14
+   ┌─ tests/move_2024/typing/macro_arg_by_name_strange_usage.move:18:14
    │
  2 │     public struct X() has drop;
    │                   - To satisfy the constraint, the 'copy' ability would need to be added here
  3 │     public struct S { f: X } has drop;
    │                          - The type 'a::m::X' does not have the ability 'copy'
    ·
-17 │         foo!(s.f); // TODO improve this error message
+18 │         foo!(s.f); // TODO improve this error message
    │              ^^^ Invalid implicit copy of field 'f' without the 'copy' ability
 

external-crates/move/crates/move-compiler/tests/move_2024/typing/macro_arg_by_name_strange_usage.move

@@ -5,8 +5,9 @@ module a::m {
     // some usages of the expression won't do what they would do if they werent used as a value
     // In other words, we don't re-interpret the expression as a pth
     macro fun foo<$T>($x: $T) {
-        &$x;
-        &mut $x;
+        let mut x = $x;
+        &x;
+        &mut x;
     }
 
     fun t() {

external-crates/move/crates/move-compiler/tests/move_2024/typing/macro_arg_by_name_strange_usage_fields.move

@@ -2,15 +2,16 @@ module a::m {
     public struct X() has drop;
     public struct S { f: X } has drop;
 
-    // some usages of the expression won't do what they would do if they werent used as a value
-    // In other words, we don't re-interpret the expression as a pth
+    // this is all technically correct due to the binding. If we allowed `&$s.f` this would break,
+    // but we explicitly prevent this.
     macro fun foo<$T>($s: $T) {
-        &$s.f;
-        &mut $s.f;
+        let mut s = $s;
+        &s.f;
+        &mut s.f;
     }
 
     fun t() {
         let s = S { f: X() };
-        foo!(s); // TODO improve this error message
+        foo!(s);
     }
 }

external-crates/move/crates/move-compiler/tests/move_2024/typing/macro_duck_typing_constraint.move

@@ -8,7 +8,8 @@ module a::m {
     macro fun needs_copy<$T, $U, $V>($x: X<$T>, $u: $U, $v: $V): X<$U> {
         $x;
         $u;
-        mycopy(&$v);
+        let v = $v;
+        mycopy(&v);
         X()
     }
 

external-crates/move/crates/move-compiler/tests/move_2024/typing/macro_duck_typing_constraint_invalid.exp

@@ -9,7 +9,7 @@ error[E05001]: ability constraint not satisfied
  9 │     macro fun needs_copy<$T, $U, $V>(_: X<$T>, _: $U, $v: $V): X<$U> {
    │                                         ^^^^^ 'copy' constraint not satisifed
    ·
-16 │         needs_copy!<None, None, None>(X(), None(), None());
+17 │         needs_copy!<None, None, None>(X(), None(), None());
    │                     ---- The type 'a::m::None' does not have the ability 'copy'
 
 error[E05001]: ability constraint not satisfied
@@ -23,35 +23,35 @@ error[E05001]: ability constraint not satisfied
  9 │     macro fun needs_copy<$T, $U, $V>(_: X<$T>, _: $U, $v: $V): X<$U> {
    │                                                                ^^^^^ 'copy' constraint not satisifed
    ·
-16 │         needs_copy!<None, None, None>(X(), None(), None());
+17 │         needs_copy!<None, None, None>(X(), None(), None());
    │                           ---- The type 'a::m::None' does not have the ability 'copy'
 
 error[E05001]: ability constraint not satisfied
-   ┌─ tests/move_2024/typing/macro_duck_typing_constraint_invalid.move:10:9
+   ┌─ tests/move_2024/typing/macro_duck_typing_constraint_invalid.move:11:9
    │
  3 │     public struct None() has drop;
    │                   ---- To satisfy the constraint, the 'copy' ability would need to be added here
  4 │ 
  5 │     fun mycopy<T: copy>(t: &T): T {
    │                   ---- 'copy' constraint declared here
    ·
-10 │         mycopy(&$v);
-   │         ^^^^^^^^^^^ 'copy' constraint not satisifed
+11 │         mycopy(&v);
+   │         ^^^^^^^^^^ 'copy' constraint not satisifed
    ·
-16 │         needs_copy!<None, None, None>(X(), None(), None());
+17 │         needs_copy!<None, None, None>(X(), None(), None());
    │                                 ---- The type 'a::m::None' does not have the ability 'copy'
 
 error[E05001]: ability constraint not satisfied
-   ┌─ tests/move_2024/typing/macro_duck_typing_constraint_invalid.move:11:9
+   ┌─ tests/move_2024/typing/macro_duck_typing_constraint_invalid.move:12:9
    │
  2 │     public struct X<phantom T: copy>() has copy, drop;
    │                                ---- 'copy' constraint declared here
  3 │     public struct None() has drop;
    │                   ---- To satisfy the constraint, the 'copy' ability would need to be added here
    ·
-11 │         X()
+12 │         X()
    │         ^^^ 'copy' constraint not satisifed
    ·
-16 │         needs_copy!<None, None, None>(X(), None(), None());
+17 │         needs_copy!<None, None, None>(X(), None(), None());
    │                           ---- The type 'a::m::None' does not have the ability 'copy'
 

external-crates/move/crates/move-compiler/tests/move_2024/typing/macro_duck_typing_constraint_invalid.move

@@ -7,7 +7,8 @@ module a::m {
     }
 
     macro fun needs_copy<$T, $U, $V>(_: X<$T>, _: $U, $v: $V): X<$U> {
-        mycopy(&$v);
+        let v = $v;
+        mycopy(&v);
         X()
     }
 

external-crates/move/crates/move-compiler/tests/move_2024/typing/macro_duck_typing_method.move

@@ -15,7 +15,8 @@ module a::m {
     // this is "duck typing" in the sense that this macro can be called only by those
     // types that "walk and talk like a duck"
     macro fun call_foo<$T>($x: $T) {
-        $x.foo()
+        let x = $x;
+        x.foo()
     }
 
     fun t() {

external-crates/move/crates/move-compiler/tests/move_2024/typing/macro_duck_typing_method_invalid.exp

@@ -1,9 +1,9 @@
 error[E04023]: invalid method call
-  ┌─ tests/move_2024/typing/macro_duck_typing_method_invalid.move:4:9
+  ┌─ tests/move_2024/typing/macro_duck_typing_method_invalid.move:5:9
   │
-4 │         $x.foo()
-  │         ^^^^^^^^
-  │         │  │
-  │         │  No local 'use fun' alias was found for 'a::m::X.foo', and no function 'foo' was found in the defining module 'a::m'
+5 │         x.foo()
+  │         ^^^^^^^
+  │         │ │
+  │         │ No local 'use fun' alias was found for 'a::m::X.foo', and no function 'foo' was found in the defining module 'a::m'
   │         Invalid method call. No known method 'foo' on type 'a::m::X'
 

external-crates/move/crates/move-compiler/tests/move_2024/typing/macro_duck_typing_method_invalid.move

@@ -1,7 +1,8 @@
 module a::m {
     public struct X() has copy, drop;
     macro fun call_foo<$T>($x: $T) {
-        $x.foo()
+        let x= $x;
+        x.foo()
     }
 
     fun t() {

external-crates/move/crates/move-compiler/tests/move_2024/typing/macro_unique_use_fun_scopes_captured_lambda.move

@@ -4,7 +4,8 @@ module a::m {
     use fun into as u64.into;
 
     public macro fun apply($f: |u64| -> u64, $x: u64): u64 {
-        $f($x.into())
+        let x = $x;
+        $f(x.into())
     }
 }
 

external-crates/move/crates/move-compiler/tests/move_2024/typing/macro_unique_use_fun_scopes_in_lambda_arg.move

@@ -4,7 +4,8 @@ module a::m {
     use fun id as u64.id;
 
     public macro fun apply($f: |u64| -> u64, $x: u64): u64 {
-        $f($x.id())
+        let x = $x;
+        $f(x.id())
     }
 }
 

external-crates/move/crates/move-compiler/tests/move_2024/typing/macros_dont_allow_paths.exp

@@ -0,0 +1,50 @@
+error[E03006]: unexpected name in this position
+  ┌─ tests/move_2024/typing/macros_dont_allow_paths.move:6:18
+  │
+6 │         let _m = $s.x; // disallowed
+  │                  ^^ Macro parameters are not allowed to appear in paths
+  │
+  = To use a macro parameter as a value in a path expression, first bind it to a local variable, e.g. 'let s = $s;'
+  = Macro parameters are always treated as value expressions, and are not modified by path operations.
+    Path operations include 'move', 'copy', '&', '&mut', and field references
+
+error[E03006]: unexpected name in this position
+  ┌─ tests/move_2024/typing/macros_dont_allow_paths.move:7:24
+  │
+7 │         let _vs = &mut $y; // disallowed
+  │                        ^^ Macro parameters are not allowed to appear in paths
+  │
+  = To use a macro parameter as a value in a path expression, first bind it to a local variable, e.g. 'let y = $y;'
+  = Macro parameters are always treated as value expressions, and are not modified by path operations.
+    Path operations include 'move', 'copy', '&', '&mut', and field references
+
+error[E03006]: unexpected name in this position
+  ┌─ tests/move_2024/typing/macros_dont_allow_paths.move:8:19
+  │
+8 │         let _q = &$n; // disallowed
+  │                   ^^ Macro parameters are not allowed to appear in paths
+  │
+  = To use a macro parameter as a value in a path expression, first bind it to a local variable, e.g. 'let n = $n;'
+  = Macro parameters are always treated as value expressions, and are not modified by path operations.
+    Path operations include 'move', 'copy', '&', '&mut', and field references
+
+error[E03006]: unexpected name in this position
+  ┌─ tests/move_2024/typing/macros_dont_allow_paths.move:9:23
+  │
+9 │         let _q = copy $n; // disallowed
+  │                       ^^ Macro parameters are not allowed to appear in paths
+  │
+  = To use a macro parameter as a value in a path expression, first bind it to a local variable, e.g. 'let n = $n;'
+  = Macro parameters are always treated as value expressions, and are not modified by path operations.
+    Path operations include 'move', 'copy', '&', '&mut', and field references
+
+error[E03006]: unexpected name in this position
+   ┌─ tests/move_2024/typing/macros_dont_allow_paths.move:10:23
+   │
+10 │         let _q = move $n; // disallowed
+   │                       ^^ Macro parameters are not allowed to appear in paths
+   │
+   = To use a macro parameter as a value in a path expression, first bind it to a local variable, e.g. 'let n = $n;'
+   = Macro parameters are always treated as value expressions, and are not modified by path operations.
+     Path operations include 'move', 'copy', '&', '&mut', and field references
+

external-crates/move/crates/move-compiler/tests/move_2024/typing/macros_dont_allow_paths.move

@@ -0,0 +1,20 @@
+module 0x42::m {
+
+    public struct S has drop { x: u64 }
+
+    macro fun bad_paths($s: S, $y: vector<u64>, $n: u64): (S, vector<u64>, u64) {
+        let _m = $s.x; // disallowed
+        let _vs = &mut $y; // disallowed
+        let _q = &$n; // disallowed
+        let _q = copy $n; // disallowed
+        let _q = move $n; // disallowed
+        ($s, $y, $n)
+    }
+
+    fun call_macro() {
+        let s = S { x: 10 };
+        let n = 0;
+        let vs = vector[];
+        let (_, _, _) = bad_paths!(s, vs, n);
+    }
+}