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

Issue:
[Bug][Compiler-V2] `variants not allowed in this context` is generated when the struct has the same name as the module where it is defined
# 🐛 Bug

When compiling the following code using V2:

```
module 0x42::M {
    struct M has drop {
        v: u64
    }
}

module 0x42::M1 {
    use 0x42::M::{Self, M};

    fun test(_m: M::M): u64 {
        3
    }
}
```

The following error is generated:

```
error: variants not allowed in this context
   ┌─ tests/sources/functional/schemas_ok.move:15:18
   │
15 │     fun test(_m: M::M): u64 {
   │                  ^^^^
```

While V1 can compile the code.






Discussion / hints:
Here is a smaller example:

```
module 0x42::M1 {
    struct M1 has drop {
        v: u64
    }
    fun test(_m: M1::M1): u64 {
        3
    }
}
```

which leads to the error:

```
error: variants not allowed in this context
  ┌─ tests/checking/naming/module_struct_same_name.move:8:18
  │
8 │     fun test(_m: M1::M1): u64 {
  │                  ^^^^^^

```



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 871bfcda81727..651680c20c8d1 100644
--- a/third_party/move/move-model/src/builder/exp_builder.rs
+++ b/third_party/move/move-model/src/builder/exp_builder.rs
@@ -3054,10 +3054,11 @@ impl<'env, 'translator, 'module_translator> ExpTranslator<'env, 'translator, 'mo
         }
 
         // Treat this as a call to a global function.
-        if !self.parent.check_no_variant(maccess) {
+        let (no_variant, maccess) = self.parent.check_no_variant_and_convert_maccess(maccess);
+        if !no_variant {
             return self.new_error_exp();
         }
-        let (module_name, name, _) = self.parent.module_access_to_parts(maccess);
+        let (module_name, name, _) = self.parent.module_access_to_parts(&maccess);
 
         // Process `old(E)` scoping
         let is_old =
diff --git a/third_party/move/move-model/src/builder/module_builder.rs b/third_party/move/move-model/src/builder/module_builder.rs
index 5b5cdaeae904c..9f04fe84d1237 100644
--- a/third_party/move/move-model/src/builder/module_builder.rs
+++ b/third_party/move/move-model/src/builder/module_builder.rs
@@ -48,7 +48,10 @@ use move_compiler::{
     parser::ast as PA,
     shared::{unique_map::UniqueMap, Identifier, Name},
 };
-use move_ir_types::{ast::ConstantName, location::Spanned};
+use move_ir_types::{
+    ast::ConstantName,
+    location::{sp, Spanned},
+};
 use regex::Regex;
 use std::{
     cell::RefCell,
@@ -250,8 +253,8 @@ impl<'env, 'translator> ModuleBuilder<'env, 'translator> {
     /// Converts a ModuleAccess into a qualified symbol which can be used for lookup of
     /// types or functions. If the access has a struct variant, an error is produced.
     pub fn module_access_to_qualified(&self, access: &EA::ModuleAccess) -> QualifiedSymbol {
-        self.check_no_variant(access);
-        let (qsym, _) = self.module_access_to_qualified_with_variant(access);
+        let (_, access) = self.check_no_variant_and_convert_maccess(access);
+        let (qsym, _) = self.module_access_to_qualified_with_variant(&access);
         qsym
     }
 
@@ -262,15 +265,46 @@ impl<'env, 'translator> ModuleBuilder<'env, 'translator> {
         )
     }
 
-    pub fn check_no_variant(&self, maccess: &EA::ModuleAccess) -> bool {
-        if Self::is_variant(maccess) {
-            self.parent.env.error(
-                &self.parent.to_loc(&maccess.loc),
-                "variants not allowed in this context",
-            );
-            false
+    /// If `maccess` takes the form `ModuleAccess(M, _, Some(V))`,
+    /// check `M::V` is a struct/enum, constant or schema,
+    /// if so, return the form `ModuleAccess(M, V, None)`,
+    /// see how `maccess` is created by
+    /// function `name_access_chain` in `expansion/translate.rs`
+    pub fn check_no_variant_and_convert_maccess(
+        &self,
+        maccess: &EA::ModuleAccess,
+    ) -> (bool, EA::ModuleAccess) {
+        if let EA::ModuleAccess_::ModuleAccess(mident, _, Some(var_name)) = &maccess.value {
+            let addr = self
+                .parent
+                .resolve_address(&self.parent.to_loc(&mident.loc), &mident.value.address);
+            let name = self
+                .symbol_pool()
+                .make(mident.value.module.0.value.as_str());
+            let module_name = ModuleName::from_address_bytes_and_name(addr, name);
+            let var_name_sym = self.symbol_pool().make(var_name.value.as_str());
+            let qualitifed_name = QualifiedSymbol {
+                module_name,
+                symbol: var_name_sym,
+            };
+            if self.parent.struct_table.contains_key(&qualitifed_name)
+                || self.parent.spec_schema_table.contains_key(&qualitifed_name)
+                || self.parent.const_table.contains_key(&qualitifed_name)
+            {
+                let new_maccess = sp(
+                    maccess.loc,
+                    EA::ModuleAccess_::ModuleAccess(*mident, *var_name, None),
+                );
+                (true, new_maccess)
+            } else {
+                self.parent.env.error(
+                    &self.parent.to_loc(&maccess.loc),
+                    "variants not allowed in this context",
+                );
+                (false, maccess.clone())
+            }
         } else {
-            true
+            (true, maccess.clone())
         }
     }
 
@@ -410,7 +444,7 @@ impl<'env, 'translator> ModuleBuilder<'env, 'translator> {
                             self.symbol_pool().make(n.value.as_str()),
                         ),
                         EA::ModuleAccess_::ModuleAccess(mident, n, _) => {
-                            self.check_no_variant(macc);
+                            let (_, macc) = self.check_no_variant_and_convert_maccess(macc);
                             let addr_bytes = self.parent.resolve_address(
                                 &self.parent.to_loc(&macc.loc),
                                 &mident.value.address,


Test patch:
diff --git a/third_party/move/move-compiler-v2/tests/checking/naming/module_struct_same_name.exp b/third_party/move/move-compiler-v2/tests/checking/naming/module_struct_same_name.exp
new file mode 100644
index 0000000000000..5cc6f56d4dbef
--- /dev/null
+++ b/third_party/move/move-compiler-v2/tests/checking/naming/module_struct_same_name.exp
@@ -0,0 +1,33 @@
+
+Diagnostics:
+warning: unused alias
+  ┌─ tests/checking/naming/module_struct_same_name.move:8:15
+  │
+8 │     use 0x42::M::{Self, M};
+  │               ^ Unused 'use' of alias 'M'. Consider removing it
+
+// -- Model dump before bytecode pipeline
+module 0x42::M {
+    enum M {
+        M,
+    }
+} // end 0x42::M
+module 0x42::M1 {
+    use 0x42::M::{Self, M}; // resolved as: 0x42::M
+    private fun test(_m: M::M): u64 {
+        3
+    }
+} // end 0x42::M1
+
+// -- Sourcified model before bytecode pipeline
+module 0x42::M {
+    enum M has drop {
+        M,
+    }
+}
+module 0x42::M1 {
+    use 0x42::M;
+    fun test(_m: M::M): u64 {
+        3
+    }
+}
diff --git a/third_party/move/move-compiler-v2/tests/checking/naming/module_struct_same_name.move b/third_party/move/move-compiler-v2/tests/checking/naming/module_struct_same_name.move
new file mode 100644
index 0000000000000..a222ec7a2cb41
--- /dev/null
+++ b/third_party/move/move-compiler-v2/tests/checking/naming/module_struct_same_name.move
@@ -0,0 +1,13 @@
+module 0x42::M {
+    enum M has drop {
+        M
+    }
+}
+
+module 0x42::M1 {
+    use 0x42::M::{Self, M};
+
+    fun test(_m: M::M): u64 {
+        3
+    }
+}
diff --git a/third_party/move/move-compiler-v2/tests/more-v1/expansion/use_struct_overlap_with_module.exp b/third_party/move/move-compiler-v2/tests/more-v1/expansion/use_struct_overlap_with_module.exp
index ea5bd62d87125..c656af6345167 100644
--- a/third_party/move/move-compiler-v2/tests/more-v1/expansion/use_struct_overlap_with_module.exp
+++ b/third_party/move/move-compiler-v2/tests/more-v1/expansion/use_struct_overlap_with_module.exp
@@ -6,8 +6,5 @@ warning: unused alias
 7 │     use 0x2::X::{Self, S as X};
   │              ^ Unused 'use' of alias 'X'. Consider removing it
 
-error: variants not allowed in this context
-  ┌─ tests/more-v1/expansion/use_struct_overlap_with_module.move:8:27
-  │
-8 │     struct A { f1: X, f2: X::S }
-  │                           ^^^^
+
+============ bytecode verification succeeded ========