Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[move-compiler v2] Comparison testing for file-format generator #9345

Merged
merged 4 commits into from
Jul 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ members = [
"third_party/move/move-command-line-common",
"third_party/move/move-compiler",
"third_party/move/move-compiler-v2",
"third_party/move/move-compiler-v2/transactional-tests",
"third_party/move/move-compiler/transactional-tests",
"third_party/move/move-core/types",
"third_party/move/move-ir-compiler",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ use move_resource_viewer::{AnnotatedMoveValue, MoveValueAnnotator};
use move_transactional_test_runner::{
framework::{run_test_impl, CompiledState, MoveTestAdapter},
tasks::{InitCommand, SyntaxChoice, TaskInput},
vm_test_harness::view_resource_in_move_storage,
vm_test_harness::{view_resource_in_move_storage, TestRunConfig},
};
use move_vm_runtime::session::SerializedReturnValues;
use once_cell::sync::Lazy;
Expand Down Expand Up @@ -553,6 +553,8 @@ impl<'a> MoveTestAdapter<'a> for AptosTestAdapter<'a> {

fn init(
default_syntax: SyntaxChoice,
_comparison_mode: bool,
_run_config: TestRunConfig,
pre_compiled_deps: Option<&'a FullyCompiledProgram>,
task_opt: Option<TaskInput<(InitCommand, Self::ExtraInitArgs)>>,
) -> (Self, Option<String>) {
Expand Down Expand Up @@ -959,5 +961,9 @@ fn render_events(events: &[ContractEvent]) -> Option<String> {
pub fn run_aptos_test(path: &Path) -> Result<(), Box<dyn std::error::Error>> {
// TODO: remove once bundles removed
aptos_vm::aptos_vm::allow_module_bundle_for_test();
run_test_impl::<AptosTestAdapter>(path, Some(&*PRECOMPILED_APTOS_FRAMEWORK))
run_test_impl::<AptosTestAdapter>(
TestRunConfig::CompilerV1,
path,
Some(&*PRECOMPILED_APTOS_FRAMEWORK),
)
}
26 changes: 26 additions & 0 deletions third_party/move/move-command-line-common/src/testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,29 @@ pub fn format_diff(expected: impl AsRef<str>, actual: impl AsRef<str>) -> String
}
ret
}

pub fn format_diff_no_color(expected: impl AsRef<str>, actual: impl AsRef<str>) -> String {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is kind of redundant with the code above, but I can't simplify it much in Rust, so it's fine as is. Higher-order functions in Rust are a bit verbose.

Here's a try:

pub fn format_diff_no_color(expected: impl AsRef<str>, actual: impl AsRef<str>) -> String {
    format_diff_shared(
        expected,
        actual,
        |x: &String, ret: &mut String| {
            ret.push_str(&format!("= {}", x.replace('\n', "\n= ")));
            ret.push('\n');
        },
        |x: &String, ret: &mut String| {
            ret.push_str(&format!("+ {}", x.replace('\n', "\n+ ")));
            ret.push('\n');
        },
        |x: &String, ret: &mut String| {
            ret.push_str(&format!("- {}", x.replace('\n', "\n- ")));
            ret.push('\n');
        },
    )
}

Where the code above is changed to:


fn format_diff_shared(
    expected: impl AsRef<str>,
    actual: impl AsRef<str>,
    same_proc: impl Fn(&String, &mut String),
    add_proc: impl Fn(&String, &mut String),
    rem_proc: impl Fn(&String, &mut String),
) -> String {
    use difference::{Changeset, Difference};

    let changeset = Changeset::new(expected.as_ref(), actual.as_ref(), "\n");

    let mut ret = String::new();

    for seq in changeset.diffs {
        match &seq {
            Difference::Same(x) => {
                same_proc(x, &mut ret);
            },
            Difference::Add(x) => {
                add_proc(x, &mut ret);
            },
            Difference::Rem(x) => {
                rem_proc(x, &mut ret);
            },
        }
    }
    ret
}

pub fn format_diff(expected: impl AsRef<str>, actual: impl AsRef<str>) -> String {
    format_diff_shared(
        expected,
        actual,
        |x: &String, ret: &mut String| {
            ret.push_str(x);
            ret.push('\n');
        },
        |x: &String, ret: &mut String| {
            ret.push_str("\x1B[92m");
            ret.push_str(x);
            ret.push_str("\x1B[0m");
            ret.push('\n');
        },
        |x: &String, ret: &mut String| {
            ret.push_str("\x1B[91m");
            ret.push_str(x);
            ret.push_str("\x1B[0m");
            ret.push('\n');
        },
    )
}

use difference::*;

let changeset = Changeset::new(expected.as_ref(), actual.as_ref(), "\n");

let mut ret = String::new();

for seq in changeset.diffs {
match &seq {
Difference::Same(x) => {
ret.push_str(&format!("= {}", x.replace('\n', "\n= ")));
ret.push('\n');
},
Difference::Add(x) => {
ret.push_str(&format!("+ {}", x.replace('\n', "\n+ ")));
ret.push('\n');
},
Difference::Rem(x) => {
ret.push_str(&format!("- {}", x.replace('\n', "\n- ")));
ret.push('\n');
},
}
}
ret
}
6 changes: 5 additions & 1 deletion third_party/move/move-compiler-v2/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@ publish = false
edition = "2021"

[dependencies]
anyhow = "1.0.62"
move-binary-format = { path = "../move-binary-format" }
move-core-types = { path = "../move-core/types" }
move-model = { path = "../move-model" }
move-stackless-bytecode = { path = "../move-prover/bytecode" }

anyhow = "1.0.62"
bcs = { workspace = true }
clap = { version = "3.2.23", features = ["derive", "env"] }
codespan = "0.11.1"
codespan-reporting = { version = "0.11.1", features = ["serde", "serialization"] }
Expand All @@ -32,6 +33,9 @@ serde = { version = "1.0.124", features = ["derive"] }
[dev-dependencies]
anyhow = "1.0.52"
datatest-stable = "0.1.1"
move-command-line-common = { path = "../move-command-line-common" }
move-disassembler = { path = "../tools/move-disassembler" }
move-ir-types = { path = "../move-ir/types" }
move-prover-test-utils = { path = "../move-prover/test-utils" }
move-stdlib = { path = "../move-stdlib" }

Expand Down
55 changes: 49 additions & 6 deletions third_party/move/move-compiler-v2/src/bytecode_generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,20 +38,21 @@ pub fn generate_bytecode(env: &GlobalEnv, fid: QualifiedId<FunId>) -> FunctionDa
loops: vec![],
reference_mode_counter: 0,
reference_mode_kind: ReferenceKind::Immutable,
results: vec![],
code: vec![],
};
let mut scope = BTreeMap::new();
for Parameter(name, ty) in gen.func_env.get_parameters() {
let temp = gen.new_temp(ty);
scope.insert(name, temp);
}
let mut results = vec![];
for ty in gen.func_env.get_result_type().flatten() {
let temp = gen.new_temp(ty);
results.push(temp)
gen.results.push(temp)
}
gen.scopes.push(scope);
if let Some(def) = gen.func_env.get_def().cloned() {
let results = gen.results.clone();
// Need to clone expression if present because of sharing issues with `gen`. However, because
// of interning, clone is cheap.
gen.gen(results.clone(), &def);
Expand All @@ -66,6 +67,7 @@ pub fn generate_bytecode(env: &GlobalEnv, fid: QualifiedId<FunId>) -> FunctionDa
loops: _,
reference_mode_counter: _,
reference_mode_kind: _,
results: _,
code,
} = gen;
FunctionData::new(
Expand Down Expand Up @@ -107,6 +109,8 @@ struct Generator<'env> {
reference_mode_counter: usize,
/// The kind of the reference mode.
reference_mode_kind: ReferenceKind,
/// The list of temporaries where to store function return result.
results: Vec<TempIndex>,
/// The bytecode, as generated so far.
code: Vec<Bytecode>,
// TODO: location_table,
Expand Down Expand Up @@ -327,6 +331,7 @@ impl<'env> Generator<'env> {
self.scopes.pop();
},
ExpData::Mutate(id, lhs, rhs) => {
let rhs_temp = self.gen_arg(rhs);
let lhs_temp = self.gen_auto_ref_arg(lhs, ReferenceKind::Mutable);
if !self.temp_type(lhs_temp).is_mutable_reference() {
self.error(
Expand All @@ -338,13 +343,16 @@ impl<'env> Generator<'env> {
),
);
}
let rhs_temp = self.gen_arg(rhs);
self.emit_call(*id, targets, BytecodeOperation::WriteRef, vec![
lhs_temp, rhs_temp,
])
},
ExpData::Assign(id, lhs, rhs) => self.gen_assign(*id, lhs, rhs, None),
ExpData::Return(_, exp) => self.gen(targets, exp),
ExpData::Return(id, exp) => {
let results = self.results.clone();
self.gen(results.clone(), exp);
self.emit_with(*id, |attr| Bytecode::Ret(attr, results))
},
ExpData::IfElse(id, cond, then_exp, else_exp) => {
let cond_temp = self.gen_arg(cond);
let then_label = self.new_label(*id);
Expand Down Expand Up @@ -574,8 +582,8 @@ impl<'env> Generator<'env> {
Operation::Xor => self.gen_op_call(targets, id, BytecodeOperation::Xor, args),
Operation::Shl => self.gen_op_call(targets, id, BytecodeOperation::Shl, args),
Operation::Shr => self.gen_op_call(targets, id, BytecodeOperation::Shr, args),
Operation::And => self.gen_op_call(targets, id, BytecodeOperation::And, args),
Operation::Or => self.gen_op_call(targets, id, BytecodeOperation::Or, args),
Operation::And => self.gen_logical_shortcut(true, targets, id, args),
Operation::Or => self.gen_logical_shortcut(false, targets, id, args),
Operation::Eq => self.gen_op_call(targets, id, BytecodeOperation::Eq, args),
Operation::Neq => self.gen_op_call(targets, id, BytecodeOperation::Neq, args),
Operation::Lt => self.gen_op_call(targets, id, BytecodeOperation::Lt, args),
Expand Down Expand Up @@ -666,6 +674,41 @@ impl<'env> Generator<'env> {
})
}

fn gen_logical_shortcut(
&mut self,
is_and: bool,
targets: Vec<TempIndex>,
id: NodeId,
args: &[Exp],
) {
let target = self.require_unary_target(id, targets);
let arg1 = self.gen_arg(&args[0]);
let true_label = self.new_label(id);
let false_label = self.new_label(id);
let done_label = self.new_label(id);
self.emit_with(id, |attr| {
Bytecode::Branch(attr, true_label, false_label, arg1)
});
self.emit_with(id, |attr| Bytecode::Label(attr, true_label));
if is_and {
self.gen(vec![target], &args[1]);
} else {
self.emit_with(id, |attr| {
Bytecode::Load(attr, target, Constant::Bool(true))
})
}
self.emit_with(id, |attr| Bytecode::Jump(attr, done_label));
self.emit_with(id, |attr| Bytecode::Label(attr, false_label));
if is_and {
self.emit_with(id, |attr| {
Bytecode::Load(attr, target, Constant::Bool(false))
})
} else {
self.gen(vec![target], &args[1]);
}
self.emit_with(id, |attr| Bytecode::Label(attr, done_label));
}

fn gen_function_call(
&mut self,
targets: Vec<TempIndex>,
Expand Down
3 changes: 0 additions & 3 deletions third_party/move/move-compiler-v2/src/bytecode_pipeline.rs

This file was deleted.

Loading