[Compiler-v2] Check recursive definition for constants (#14741)

* check recursive in constant definition

* add tests

* add tests for duplicated constants

* fix

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

@@ -0,0 +1,7 @@
+
+Diagnostics:
+error: not supported before language version `2.0`: constant definitions referring to other constants
+  ┌─ tests/checking-lang-v1/v1-typing/refer_other_constants.move:3:5
+  │
+3 │     const X: u64 = Y;
+  │     ^^^^^^^^^^^^^^^^^

third_party/move/move-compiler-v2/tests/checking-lang-v1/v1-typing/refer_other_constants.move

@@ -0,0 +1,14 @@
+address 0x42 {
+module M {
+    const X: u64 = Y;
+    const Y: u64 = 0;
+
+    public fun get_x(): u64 {
+        X
+    }
+
+    public fun get_y(): u64 {
+        Y
+    }
+}
+}

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

@@ -24,6 +24,9 @@ module 0x42::M {
     private fun t8(): vector<address> {
         [Address(Numerical(0000000000000000000000000000000000000000000000000000000000000000)), Address(Numerical(0000000000000000000000000000000000000000000000000000000000000001))]
     }
+    private fun t9(): u8 {
+        0
+    }
 } // end 0x42::M
 module <SELF>_0 {
     private fun t() {

third_party/move/move-compiler-v2/tests/checking/typing/constant_all_valid_types.move

@@ -1,5 +1,6 @@
 address 0x42 {
 module M {
+    const B: u8 = C1;
     const C1: u8 = 0;
     const C2: u64 = 0;
     const C3: u128 = 0;
@@ -17,6 +18,7 @@ module M {
     fun t6(): vector<u8> { C6 }
     fun t7(): vector<u8> { C7 }
     fun t8(): vector<address> { C8 }
+    fun t9(): u8 { B }
 }
 }
 

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

@@ -0,0 +1,9 @@
+
+Diagnostics:
+error: duplicate declaration, item, or annotation
+  ┌─ tests/checking/typing/constant_duplicate.move:3:11
+  │
+2 │     const LN2_X_32: u64 = 32 * 2977044472;
+  │           -------- Alias previously defined here
+3 │     const LN2_X_32: u64 = 32 * 2977044472;
+  │           ^^^^^^^^ Duplicate module member or alias 'LN2_X_32'. Top level names in a namespace must be unique

third_party/move/move-compiler-v2/tests/checking/typing/constant_duplicate.move

@@ -0,0 +1,4 @@
+module 0x42::constant_duplicate {
+    const LN2_X_32: u64 = 32 * 2977044472;
+    const LN2_X_32: u64 = 32 * 2977044472;
+}

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

@@ -1,9 +1,51 @@
-// -- Model dump before bytecode pipeline
-module 0x42::M {
-    public fun get_x(): u64 {
-        false
-    }
-    public fun get_y(): u64 {
-        false
-    }
-} // end 0x42::M
+
+Diagnostics:
+error: Found recursive definition of a constant `X`; cycle formed by definitions below
+  ┌─ tests/checking/typing/recursive_constant.move:3:5
+  │
+3 │     const X: u64 = Y;
+  │     ^^^^^^^^^^^^^^^^^
+  │     │
+  │     `X` is defined here
+4 │     const Y: u64 = X;
+  │     ----------------- `Y` is defined here
+
+error: Found recursive definition of a constant `F`; cycle formed by definitions below
+  ┌─ tests/checking/typing/recursive_constant.move:6:5
+  │
+6 │     const F: u64 = F;
+  │     ^^^^^^^^^^^^^^^^^
+  │     │
+  │     `F` is defined here
+
+error: Invalid expression in `const`. Constant folding failed due to incomplete evaluation
+  ┌─ tests/checking/typing/recursive_constant.move:8:8
+  │
+8 │        Z + A
+  │        ^^^^^
+
+error: Invalid expression in `const`. Constant folding failed due to incomplete evaluation
+   ┌─ tests/checking/typing/recursive_constant.move:10:20
+   │
+10 │     const A: u64 = B + C;
+   │                    ^^^^^
+
+error: Found recursive definition of a constant `A`; cycle formed by definitions below
+   ┌─ tests/checking/typing/recursive_constant.move:10:5
+   │
+ 7 │ ╭     const X1: u64 = {
+ 8 │ │        Z + A
+ 9 │ │     };
+   │ ╰──────' `X1` is defined here
+10 │       const A: u64 = B + C;
+   │       ^^^^^^^^^^^^^^^^^^^^^
+   │       │
+   │       `A` is defined here
+11 │       const B: u64 = X1;
+   │       ------------------ `B` is defined here
+
+error: Invalid expression in `const`. Constant folding failed due to incomplete evaluation
+   ┌─ tests/checking/typing/recursive_constant.move:12:20
+   │
+12 │     const C: u64 = Z + B;
+   │                    ^^^^^

third_party/move/move-compiler-v2/tests/checking/typing/recursive_constant.move

@@ -3,6 +3,13 @@ module M {
     const X: u64 = Y;
     const Y: u64 = X;
     const Z: u64 = 0;
+    const F: u64 = F;
+    const X1: u64 = {
+       Z + A
+    };
+    const A: u64 = B + C;
+    const B: u64 = X1;
+    const C: u64 = Z + B;
 
     public fun get_x(): u64 {
         X

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

@@ -382,7 +382,7 @@ pub enum ModuleAccess_ {
 }
 
 impl ModuleAccess_ {
-    fn get_name(&self) -> &Name {
+    pub fn get_name(&self) -> &Name {
         match self {
             ModuleAccess_::Name(n) | ModuleAccess_::ModuleAccess(_, n, _) => n,
         }

third_party/move/move-compiler/tests/move_check/typing/constant_duplicate.exp

@@ -0,0 +1,8 @@
+error[E02001]: duplicate declaration, item, or annotation
+  ┌─ tests/move_check/typing/constant_duplicate.move:3:11
+  │
+2 │     const LN2_X_32: u64 = 32 * 2977044472;
+  │           -------- Alias previously defined here
+3 │     const LN2_X_32: u64 = 32 * 2977044472;
+  │           ^^^^^^^^ Duplicate module member or alias 'LN2_X_32'. Top level names in a namespace must be unique
+

third_party/move/move-compiler/tests/move_check/typing/constant_duplicate.move

@@ -0,0 +1,4 @@
+module 0x42::constant_duplicate {
+    const LN2_X_32: u64 = 32 * 2977044472;
+    const LN2_X_32: u64 = 32 * 2977044472;
+}

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

@@ -176,15 +176,8 @@ impl<'env, 'translator, 'module_translator> ExpTranslator<'env, 'translator, 'mo
     }
 
     pub fn check_language_version(&self, loc: &Loc, feature: &str, version_min: LanguageVersion) {
-        if !self.env().language_version().is_at_least(version_min) {
-            self.env().error(
-                loc,
-                &format!(
-                    "not supported before language version `{}`: {}",
-                    version_min, feature
-                ),
-            )
-        }
+        self.parent
+            .check_language_version(loc, feature, version_min);
     }
 
     pub fn set_spec_block_map(&mut self, map: BTreeMap<EA::SpecId, EA::SpecBlock>) {

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

@@ -875,6 +875,141 @@ impl<'env, 'translator> ModuleBuilder<'env, 'translator> {
 /// # Definition Analysis
 
 impl<'env, 'translator> ModuleBuilder<'env, 'translator> {
+    pub fn check_language_version(&self, loc: &Loc, feature: &str, version_min: LanguageVersion) {
+        if !self.parent.env.language_version().is_at_least(version_min) {
+            self.parent.env.error(
+                loc,
+                &format!(
+                    "not supported before language version `{}`: {}",
+                    version_min, feature
+                ),
+            )
+        }
+    }
+
+    /// Evaluation of constant `key`
+    /// Performed in depth-first way to detect cyclic dependency
+    /// and constants are evaluated according to dependency relation
+    fn eval_constant(
+        &mut self,
+        key: &PA::ConstantName,
+        constant_map: &UniqueMap<PA::ConstantName, EA::Constant>,
+        visiting: &mut Vec<(PA::ConstantName, Loc)>, // constants that are being traversed during dfs
+        visited: &mut BTreeSet<PA::ConstantName>, // constants that are already visited during dfs
+        compiled_module: &Option<BytecodeModule>,
+    ) {
+        // Get all names from an expression
+        // only recursively check on expression types supported in constant definition.
+        fn get_names_from_const_exp(exp: &EA::Exp_) -> BTreeSet<Name> {
+            let mut names = BTreeSet::new();
+            let mut add_names = |v: &EA::Exp| {
+                let set = get_names_from_const_exp(&v.value);
+                for n in set.iter() {
+                    names.insert(*n);
+                }
+            };
+            match exp {
+                EA::Exp_::Name(access, _) => {
+                    names.insert(*access.value.get_name());
+                },
+                EA::Exp_::Call(_, _, _, exp_vec) | EA::Exp_::Vector(_, _, exp_vec) => {
+                    exp_vec.value.iter().for_each(&mut add_names);
+                },
+                EA::Exp_::UnaryExp(_, exp) => {
+                    add_names(exp);
+                },
+                EA::Exp_::BinopExp(exp1, _, exp2) => {
+                    add_names(exp1);
+                    add_names(exp2);
+                },
+                EA::Exp_::Block(seq) => {
+                    for s in seq.iter() {
+                        match &s.value {
+                            EA::SequenceItem_::Seq(exp) | EA::SequenceItem_::Bind(_, exp) => {
+                                add_names(exp);
+                            },
+                            _ => {},
+                        }
+                    }
+                },
+                _ => {},
+            }
+            names
+        }
+        if visited.contains(key) {
+            return;
+        }
+        let qsym = self.qualified_by_module_from_name(&key.0);
+        let loc = self
+            .parent
+            .const_table
+            .get(&qsym)
+            .expect("constant declared")
+            .loc
+            .clone();
+        if let Some(index) = visiting.iter().position(|r| r.0 == *key) {
+            self.parent.env.diag_with_labels(
+                Severity::Error,
+                &loc,
+                &format!("Found recursive definition of a constant `{}`; cycle formed by definitions below", key),
+                visiting[index..]
+                    .to_vec()
+                    .iter()
+                    .map(|(name, loc)| (loc.clone(), format!("`{}` is defined here", name)))
+                    .collect_vec(),
+            );
+            return;
+        }
+        visiting.push((*key, loc.clone()));
+        if let Some(exp) = constant_map.get(key) {
+            let names = get_names_from_const_exp(&exp.value.value);
+            for name in names {
+                let const_name = PA::ConstantName(name);
+                let qsym = self.qualified_by_module_from_name(&name);
+                if !self.parent.const_table.contains_key(&qsym) {
+                    continue;
+                }
+                self.check_language_version(
+                    &loc,
+                    "constant definitions referring to other constants",
+                    LanguageVersion::V2_0,
+                );
+                if visited.contains(&const_name) {
+                    continue;
+                }
+                self.eval_constant(
+                    &const_name,
+                    constant_map,
+                    visiting,
+                    visited,
+                    compiled_module,
+                );
+            }
+            self.def_ana_constant(key, exp, compiled_module);
+        }
+        visited.insert(*key);
+        visiting.pop();
+    }
+
+    /// Evaluation of constants in the module
+    fn analyze_constants(
+        &mut self,
+        module_def: &EA::ModuleDefinition,
+        compiled_module: &Option<BytecodeModule>,
+    ) {
+        let mut visited = BTreeSet::new();
+        let mut visiting = vec![];
+        for (name, _) in module_def.constants.key_cloned_iter() {
+            self.eval_constant(
+                &name,
+                &module_def.constants,
+                &mut visiting,
+                &mut visited,
+                compiled_module,
+            );
+        }
+    }
+
     fn def_ana(
         &mut self,
         module_def: &EA::ModuleDefinition,
@@ -886,9 +1021,7 @@ impl<'env, 'translator> ModuleBuilder<'env, 'translator> {
         }
 
         // Analyze all constants.
-        for (name, def) in module_def.constants.key_cloned_iter() {
-            self.def_ana_constant(&name, def, compiled_module);
-        }
+        self.analyze_constants(module_def, compiled_module);
 
         // Analyze all schemas. This must be done before other things because schemas need to be
         // ready for inclusion. We also must do this recursively, so use a visited set to detect

third_party/move/move-model/src/constant_folder.rs

@@ -328,9 +328,17 @@ impl<'env> ConstantFolder<'env> {
                     O::Neq => val0
                         .equivalent(val1)
                         .map(|equivalence| V(id, Bool(!equivalence)).into_exp()),
-                    _ => self.constant_folding_error(id, |_| {
-                        "Unknown binary expression in `const`".to_owned()
-                    }),
+                    _ => {
+                        if oper.is_binop() {
+                            self.constant_folding_error(id, |_| {
+                                "Constant folding failed due to incomplete evaluation".to_owned()
+                            })
+                        } else {
+                            self.constant_folding_error(id, |_| {
+                                "Unknown binary expression in `const`".to_owned()
+                            })
+                        }
+                    },
                 }
             }
         } else {