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

Plafer fix add library tests #1420

Closed
wants to merge 1 commit into from
Closed
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
7 changes: 0 additions & 7 deletions assembly/src/assembler/instruction/procedures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,6 @@ impl Assembler {
proc_ctx.register_external_call(&proc, false)?;
}
Some(proc) => proc_ctx.register_external_call(&proc, false)?,
None if matches!(kind, InvokeKind::SysCall) => {
return Err(AssemblyError::UnknownSysCallTarget {
span,
source_file: current_source_file.clone(),
callee: mast_root,
});
}
None => (),
}

Expand Down
61 changes: 17 additions & 44 deletions assembly/src/assembler/tests.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use core::iter;

use alloc::{boxed::Box, vec::Vec};
use pretty_assertions::assert_eq;
use vm_core::{assert_matches, mast::MastForest, Program};
Expand All @@ -12,61 +14,32 @@ use crate::{
// TESTS
// ================================================================================================

// TODO: Fix test after we implement the new `Assembler::add_library()`
#[ignore]
#[allow(unused)]
#[test]
fn nested_blocks() {
const MODULE: &str = "foo::bar";
const KERNEL: &str = r#"
export.foo
add
end"#;
const PROCEDURE: &str = r#"
const MODULE: &str = "foo::bar";
const MODULE_PROCEDURE: &str = r#"
export.baz
push.29
end"#;

pub struct DummyLibrary {
namespace: LibraryNamespace,
#[allow(clippy::vec_box)]
modules: Vec<Box<Module>>,
dependencies: Vec<LibraryNamespace>,
}
let assembler = {
let kernel_lib = Assembler::default().assemble_kernel(KERNEL).unwrap();

impl Default for DummyLibrary {
fn default() -> Self {
let ast =
Module::parse_str(MODULE.parse().unwrap(), ModuleKind::Library, PROCEDURE).unwrap();
let namespace = ast.namespace().clone();
Self {
namespace,
modules: vec![ast],
dependencies: Vec::new(),
}
}
}
let dummy_module =
Module::parse_str(MODULE.parse().unwrap(), ModuleKind::Library, MODULE_PROCEDURE)
.unwrap();
let dummy_library =
Assembler::default().assemble_library(iter::once(dummy_module)).unwrap();

impl Library for DummyLibrary {
fn root_ns(&self) -> &LibraryNamespace {
&self.namespace
}

fn version(&self) -> &Version {
const MIN: Version = Version::min();
&MIN
}

fn modules(&self) -> impl ExactSizeIterator<Item = &Module> + '_ {
self.modules.iter().map(|m| m.as_ref())
}
let mut assembler = Assembler::with_kernel(kernel_lib).unwrap();
assembler.add_compiled_library(dummy_library).unwrap();

fn dependencies(&self) -> &[LibraryNamespace] {
&self.dependencies
}
}

let assembler = Assembler::default().with_library(&DummyLibrary::default()).unwrap();
assembler
};

// The expected `MastForest` for the program (that we will build by hand)
let mut expected_mast_forest_builder = MastForestBuilder::default();
Expand Down Expand Up @@ -192,8 +165,8 @@ fn nested_blocks() {
let expected_program = Program::new(expected_mast_forest_builder.build(), combined_node_id);
assert_eq!(expected_program.hash(), program.hash());

// also check that the program has the right number of procedures
assert_eq!(program.num_procedures(), 5);
// also check that the program has the right number of procedures (which excludes the dummy library and kernel)
assert_eq!(program.num_procedures(), 3);
}

/// Ensures that a single copy of procedures with the same MAST root are added only once to the MAST
Expand Down
11 changes: 1 addition & 10 deletions assembly/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use vm_core::mast::MastForestError;
use crate::{
ast::FullyQualifiedProcedureName,
diagnostics::{Diagnostic, RelatedError, RelatedLabel, Report, SourceFile},
LibraryNamespace, LibraryPath, RpoDigest, SourceSpan,
LibraryNamespace, LibraryPath, SourceSpan,
};

// ASSEMBLY ERROR
Expand Down Expand Up @@ -60,15 +60,6 @@ pub enum AssemblyError {
source_file: Option<Arc<SourceFile>>,
callee: FullyQualifiedProcedureName,
},
#[error("invalid syscall: kernel procedures must be available during compilation, but '{callee}' is not")]
#[diagnostic()]
UnknownSysCallTarget {
#[label("call occurs here")]
span: SourceSpan,
#[source_code]
source_file: Option<Arc<SourceFile>>,
callee: RpoDigest,
},
#[error("invalid use of 'caller' instruction outside of kernel")]
#[diagnostic(help(
"the 'caller' instruction is only allowed in procedures defined in a kernel"
Expand Down
30 changes: 16 additions & 14 deletions miden/tests/integration/flow_control/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use assembly::{ast::ModuleKind, Assembler, LibraryPath};
use core::iter;
use miden_vm::Module;
use processor::ExecutionError;
use prover::Digest;
use stdlib::StdLibrary;
Expand Down Expand Up @@ -187,8 +189,6 @@ fn local_fn_call_with_mem_access() {
test.prove_and_verify(vec![3, 7], false);
}

// TODO: Fix test after we implement the new `Assembler::add_library()`
#[ignore]
#[test]
fn simple_syscall() {
let kernel_source = "
Expand All @@ -204,7 +204,7 @@ fn simple_syscall() {

// TODO: update and use macro?
let test = Test {
kernel: Some(kernel_source.to_string()),
kernel_source: Some(kernel_source.to_string()),
stack_inputs: StackInputs::try_from_ints([1, 2]).unwrap(),
..Test::new(&format!("test{}", line!()), program_source, false)
};
Expand Down Expand Up @@ -389,13 +389,8 @@ fn simple_dyncall() {
// PROCREF INSTRUCTION
// ================================================================================================

// TODO: Fix test after we implement the new `Assembler::add_library()`
#[ignore]
#[allow(unused)]
#[test]
fn procref() {
let mut assembler = Assembler::default().with_library(&StdLibrary::default()).unwrap();

let module_source = "
use.std::math::u64
export.u64::overflowing_add
Expand All @@ -406,12 +401,19 @@ fn procref() {
";

// obtain procedures' MAST roots by compiling them as module
let module_path = "test::foo".parse::<LibraryPath>().unwrap();
let opts = assembly::CompileOptions::new(ModuleKind::Library, module_path).unwrap();

// TODO: Fix
// let mast_roots = assembler.assemble_module(module_source, opts).unwrap();
let mast_roots: Vec<Digest> = Vec::new();
let mast_roots: Vec<Digest> = {
let module_path = "test::foo".parse::<LibraryPath>().unwrap();
let module = Module::parse_str(module_path, ModuleKind::Library, module_source).unwrap();
let library = Assembler::default()
.with_library(&StdLibrary::default())
.unwrap()
.assemble_library(iter::once(module))
.unwrap();

let module_info = library.into_module_infos().next().unwrap();

module_info.procedure_digests().collect()
};

let source = "
use.std::math::u64
Expand Down
4 changes: 1 addition & 3 deletions miden/tests/integration/operations/io_ops/env_ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,6 @@ fn locaddr() {
// CALLER INSTRUCTION
// ================================================================================================

// TODO: Fix test after we implement the new `Assembler::add_library()`
#[ignore]
#[test]
fn caller() {
let kernel_source = "
Expand All @@ -147,7 +145,7 @@ fn caller() {

// TODO: update and use macro?
let test = Test {
kernel: Some(kernel_source.to_string()),
kernel_source: Some(kernel_source.to_string()),
stack_inputs: StackInputs::try_from_ints([1, 2, 3, 4, 5]).unwrap(),
..Test::new(&format!("test{}", line!()), program_source, false)
};
Expand Down
60 changes: 39 additions & 21 deletions test-utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ extern crate std;
// IMPORTS
// ================================================================================================

use processor::Program;
use processor::{MastForest, Program};
#[cfg(not(target_family = "wasm"))]
use proptest::prelude::{Arbitrary, Strategy};

Expand Down Expand Up @@ -173,7 +173,7 @@ macro_rules! assert_assembler_diagnostic {
/// ExecutionError which contains the specified substring.
pub struct Test {
pub source: Arc<SourceFile>,
pub kernel: Option<String>,
pub kernel_source: Option<String>,
pub stack_inputs: StackInputs,
pub advice_inputs: AdviceInputs,
pub in_debug_mode: bool,
Expand All @@ -189,7 +189,7 @@ impl Test {
pub fn new(name: &str, source: &str, in_debug_mode: bool) -> Self {
Test {
source: Arc::new(SourceFile::new(name, source.to_string())),
kernel: None,
kernel_source: None,
stack_inputs: StackInputs::default(),
advice_inputs: AdviceInputs::default(),
in_debug_mode,
Expand Down Expand Up @@ -225,8 +225,11 @@ impl Test {
expected_mem: &[u64],
) {
// compile the program
let program: Program = self.compile().expect("Failed to compile test source.");
let host = DefaultHost::new(MemAdviceProvider::from(self.advice_inputs.clone()));
let (program, kernel) = self.compile().expect("Failed to compile test source.");
let mut host = DefaultHost::new(MemAdviceProvider::from(self.advice_inputs.clone()));
if let Some(kernel) = kernel {
host.load_mast_forest(kernel);
}

// execute the test
let mut process = Process::new(
Expand Down Expand Up @@ -275,14 +278,16 @@ impl Test {
// --------------------------------------------------------------------------------------------

/// Compiles a test's source and returns the resulting Program or Assembly error.
pub fn compile(&self) -> Result<Program, Report> {
use assembly::{ast::ModuleKind, CompileOptions};
#[allow(unused)]
let assembler = if let Some(kernel) = self.kernel.as_ref() {
// TODO: Load in kernel after we add the new `Assembler::add_library()`
assembly::Assembler::default()
pub fn compile(&self) -> Result<(Program, Option<MastForest>), Report> {
use assembly::{ast::ModuleKind, Assembler, CompileOptions};

let (assembler, compiled_kernel) = if let Some(kernel) = self.kernel_source.as_ref() {
let kernel_lib = Assembler::default().assemble_kernel(kernel).unwrap();
let compiled_kernel = kernel_lib.mast_forest().clone();

(Assembler::with_kernel(kernel_lib).unwrap(), Some(compiled_kernel))
} else {
assembly::Assembler::default()
(Assembler::default(), None)
};
let assembler = self
.add_modules
Expand All @@ -299,15 +304,18 @@ impl Test {
.with_libraries(self.libraries.iter())
.expect("failed to load stdlib");

assembler.assemble_program(self.source.clone())
Ok((assembler.assemble_program(self.source.clone())?, compiled_kernel))
}

/// Compiles the test's source to a Program and executes it with the tests inputs. Returns a
/// resulting execution trace or error.
#[track_caller]
pub fn execute(&self) -> Result<ExecutionTrace, ExecutionError> {
let program: Program = self.compile().expect("Failed to compile test source.");
let host = DefaultHost::new(MemAdviceProvider::from(self.advice_inputs.clone()));
let (program, kernel) = self.compile().expect("Failed to compile test source.");
let mut host = DefaultHost::new(MemAdviceProvider::from(self.advice_inputs.clone()));
if let Some(kernel) = kernel {
host.load_mast_forest(kernel);
}
processor::execute(&program, self.stack_inputs.clone(), host, ExecutionOptions::default())
}

Expand All @@ -316,8 +324,12 @@ impl Test {
pub fn execute_process(
&self,
) -> Result<Process<DefaultHost<MemAdviceProvider>>, ExecutionError> {
let program: Program = self.compile().expect("Failed to compile test source.");
let host = DefaultHost::new(MemAdviceProvider::from(self.advice_inputs.clone()));
let (program, kernel) = self.compile().expect("Failed to compile test source.");
let mut host = DefaultHost::new(MemAdviceProvider::from(self.advice_inputs.clone()));
if let Some(kernel) = kernel {
host.load_mast_forest(kernel);
}

let mut process = Process::new(
program.kernel().clone(),
self.stack_inputs.clone(),
Expand All @@ -333,8 +345,11 @@ impl Test {
/// is true, this function will force a failure by modifying the first output.
pub fn prove_and_verify(&self, pub_inputs: Vec<u64>, test_fail: bool) {
let stack_inputs = StackInputs::try_from_ints(pub_inputs).unwrap();
let program: Program = self.compile().expect("Failed to compile test source.");
let host = DefaultHost::new(MemAdviceProvider::from(self.advice_inputs.clone()));
let (program, kernel) = self.compile().expect("Failed to compile test source.");
let mut host = DefaultHost::new(MemAdviceProvider::from(self.advice_inputs.clone()));
if let Some(kernel) = kernel {
host.load_mast_forest(kernel);
}
let (mut stack_outputs, proof) =
prover::prove(&program, stack_inputs.clone(), host, ProvingOptions::default()).unwrap();

Expand All @@ -352,8 +367,11 @@ impl Test {
/// VmStateIterator that allows us to iterate through each clock cycle and inspect the process
/// state.
pub fn execute_iter(&self) -> VmStateIterator {
let program: Program = self.compile().expect("Failed to compile test source.");
let host = DefaultHost::new(MemAdviceProvider::from(self.advice_inputs.clone()));
let (program, kernel) = self.compile().expect("Failed to compile test source.");
let mut host = DefaultHost::new(MemAdviceProvider::from(self.advice_inputs.clone()));
if let Some(kernel) = kernel {
host.load_mast_forest(kernel);
}
processor::execute_iter(&program, self.stack_inputs.clone(), host)
}

Expand Down
8 changes: 4 additions & 4 deletions test-utils/src/test_builders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ macro_rules! build_test_by_mode {
name,
::alloc::string::String::from($source),
)),
kernel: None,
kernel_source: None,
stack_inputs,
advice_inputs,
in_debug_mode: $in_debug_mode,
Expand All @@ -118,7 +118,7 @@ macro_rules! build_test_by_mode {
name,
::alloc::string::String::from($source),
)),
kernel: None,
kernel_source: None,
stack_inputs,
advice_inputs,
in_debug_mode: $in_debug_mode,
Expand All @@ -143,7 +143,7 @@ macro_rules! build_test_by_mode {
name,
String::from($source),
)),
kernel: None,
kernel_source: None,
stack_inputs,
advice_inputs,
in_debug_mode: $in_debug_mode,
Expand All @@ -167,7 +167,7 @@ macro_rules! build_test_by_mode {
name,
::alloc::string::String::from($source),
)),
kernel: None,
kernel_source: None,
stack_inputs,
advice_inputs,
in_debug_mode: $in_debug_mode,
Expand Down
Loading