Skip to content

Commit

Permalink
add tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@rahxephon89
rahxephon89 committed Sep 25, 2024
1 parent 73e3bf6 commit 794ea3d
Show file tree
Hide file tree
Showing 7 changed files with 148 additions and 64 deletions.
7 changes: 7 additions & 0 deletions third_party/move/move-compiler-v2/tests/checking-lang-v1/v1-typing/refer_other_constants.exp
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@

Diagnostics:
error: not supported before language version `2.0`: referring to other constants
┌─ tests/checking-lang-v1/v1-typing/refer_other_constants.move:3:5
│
3 │ const X: u64 = Y;
│ ^^^^^^^^^^^^^^^^^
14 changes: 14 additions & 0 deletions ...d_party/move/move-compiler-v2/tests/checking-lang-v1/v1-typing/refer_other_constants.move
View file
Original file line number Diff line number Diff line change
@@ -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
}
}
}
48 changes: 46 additions & 2 deletions third_party/move/move-compiler-v2/tests/checking/typing/recursive_constant.exp
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,51 @@

Diagnostics:
error: Found recursive definition of a constant
┌─ tests/checking/typing/recursive_constant.move:4:5
error: Found recursive definition of a constant X
┌─ tests/checking/typing/recursive_constant.move:3:5
│
3 │ const X: u64 = Y;
│ ^^^^^^^^^^^^^^^^^
│ │
│ X is defined
4 │ const Y: u64 = X;
│ ----------------- Y is defined

error: Found recursive definition of a constant F
┌─ tests/checking/typing/recursive_constant.move:6:5
│
6 │ const F: u64 = F;
│ ^^^^^^^^^^^^^^^^^
│ │
│ F is defined

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
┌─ tests/checking/typing/recursive_constant.move:10:5
│
7 │ ╭ const X1: u64 = {
8 │ │ Z + A
9 │ │ };
│ ╰──────' X1 is defined
10 │ const A: u64 = B + C;
│ ^^^^^^^^^^^^^^^^^^^^^
│ │
│ A is defined
11 │ const B: u64 = X1;
│ ------------------ B is defined

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;
│ ^^^^^
7 changes: 7 additions & 0 deletions third_party/move/move-compiler-v2/tests/checking/typing/recursive_constant.move
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
42 changes: 2 additions & 40 deletions third_party/move/move-compiler/src/expansion/ast.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use move_binary_format::file_format;
use move_ir_types::location::*;
use move_symbol_pool::Symbol;
use std::{
collections::{BTreeMap, BTreeSet, HashSet, VecDeque},
collections::{BTreeMap, BTreeSet, VecDeque},
fmt,
hash::Hash,
};
Expand Down Expand Up @@ -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,
}
Expand Down Expand Up @@ -548,44 +548,6 @@ pub enum Exp_ {
}
pub type Exp = Spanned<Exp_>;

impl Exp_ {
/// Get all names from an expression
/// only perform on expression supported in constant definition.
pub fn get_names_for_const_exp(&self) -> HashSet<Name> {
let mut names = HashSet::new();
let mut add_names = |v: &Exp| {
let set = v.value.get_names_for_const_exp();
for n in set.iter() {
names.insert(*n);
}
};
match self {
Self::Name(access, _) => {
names.insert(*access.value.get_name());
},
Self::Call(_, _, _, exp_vec) | Self::Vector(_, _, exp_vec) => {
let _ = exp_vec.value.iter().map(&mut add_names);
},
Self::UnaryExp(_, exp) => {
add_names(exp);
},
Self::BinopExp(exp1, _, exp2) => {
add_names(exp1);
add_names(exp2);
},
Self::Block(seq) => {
for s in seq.iter() {
if let SequenceItem_::Seq(exp) = &s.value {
add_names(exp);
}
}
},
_ => {},
}
names
}
}

pub type Sequence = VecDeque<SequenceItem>;
#[derive(Debug, Clone, PartialEq)]
pub enum SequenceItem_ {
Expand Down
80 changes: 61 additions & 19 deletions third_party/move/move-model/src/builder/module_builder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -891,10 +891,51 @@ impl<'env, 'translator> ModuleBuilder<'env, 'translator> {
&mut self,
key: &PA::ConstantName,
constant_map: &UniqueMap<PA::ConstantName, EA::Constant>,
visiting: &mut HashSet<PA::ConstantName>,
visited: &mut HashSet<PA::ConstantName>,
visiting: &mut Vec<(PA::ConstantName, Loc)>, // constants that are being traversed during dfs
visited: &mut HashSet<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
Expand All @@ -903,15 +944,22 @@ impl<'env, 'translator> ModuleBuilder<'env, 'translator> {
.expect("constant declared")
.loc
.clone();
if visited.contains(key) {
return;
}
if visiting.contains(key) {
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 {}", key),
visiting[index..]
.to_vec()
.iter()
.map(|(name, loc)| (loc.clone(), format!("{} is defined", name)))
.collect_vec(),
);
return;
}
visiting.insert(*key);
visiting.push((*key, loc.clone()));
if let Some(exp) = constant_map.get(key) {
let names = exp.value.value.get_names_for_const_exp();
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);
Expand All @@ -920,17 +968,11 @@ impl<'env, 'translator> ModuleBuilder<'env, 'translator> {
}
self.check_language_version(
&loc,
"Referring to other constants",
"referring to other constants",
LanguageVersion::V2_0,
);
if visited.contains(&const_name) {
return;
}
if visiting.contains(&const_name) {
self.parent
.env
.error(&loc, "Found recursive definition of a constant");
return;
continue;
}
self.analyze_constant(
&const_name,
Expand All @@ -941,9 +983,9 @@ impl<'env, 'translator> ModuleBuilder<'env, 'translator> {
);
}
self.def_ana_constant(key, exp, compiled_module);
visited.insert(*key);
visiting.remove(key);
}

Check warning on line 986 in third_party/move/move-model/src/builder/module_builder.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L986 was not covered by tests
visited.insert(*key);
visiting.pop();
}

fn analyze_constants(
Expand All @@ -952,7 +994,7 @@ impl<'env, 'translator> ModuleBuilder<'env, 'translator> {
compiled_module: &Option<BytecodeModule>,
) {
let mut visited = HashSet::new();
let mut visiting = HashSet::new();
let mut visiting = vec![];
for (name, _) in module_def.constants.key_cloned_iter() {
self.analyze_constant(
&name,
Expand Down
14 changes: 11 additions & 3 deletions third_party/move/move-model/src/constant_folder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Check warning on line 338 in third_party/move/move-model/src/constant_folder.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L338 was not covered by tests
})
}
},
}
}
} else {
Expand Down

0 comments on commit 794ea3d

Please sign in to comment.