From daad8b01862c1e075b37327e5a1c33a42ca4585c Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Tue, 30 Jul 2024 09:09:34 -0400 Subject: [PATCH] chore: fix tests that use `Assembler::add_compiled_library()` --- .../src/assembler/instruction/procedures.rs | 7 --- assembly/src/assembler/tests.rs | 61 ++++++------------- assembly/src/errors.rs | 11 +--- assembly/src/library/mod.rs | 5 ++ miden/tests/integration/flow_control/mod.rs | 32 +++++----- .../integration/operations/io_ops/env_ops.rs | 4 +- test-utils/src/lib.rs | 61 ++++++++++++------- test-utils/src/test_builders.rs | 8 +-- 8 files changed, 84 insertions(+), 105 deletions(-) diff --git a/assembly/src/assembler/instruction/procedures.rs b/assembly/src/assembler/instruction/procedures.rs index 4863fb49f7..97110f9915 100644 --- a/assembly/src/assembler/instruction/procedures.rs +++ b/assembly/src/assembler/instruction/procedures.rs @@ -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 => (), } diff --git a/assembly/src/assembler/tests.rs b/assembly/src/assembler/tests.rs index cfec29207d..77032f6e0b 100644 --- a/assembly/src/assembler/tests.rs +++ b/assembly/src/assembler/tests.rs @@ -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}; @@ -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>, - dependencies: Vec, - } + 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 + '_ { - self.modules.iter().map(|m| m.as_ref()) - } + let mut assembler = Assembler::with_kernel(kernel_lib); + 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(); @@ -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 diff --git a/assembly/src/errors.rs b/assembly/src/errors.rs index d55d73bbec..e8484ef95b 100644 --- a/assembly/src/errors.rs +++ b/assembly/src/errors.rs @@ -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 @@ -60,15 +60,6 @@ pub enum AssemblyError { source_file: Option>, 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>, - 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" diff --git a/assembly/src/library/mod.rs b/assembly/src/library/mod.rs index 9c88acddbf..0fa2fddc15 100644 --- a/assembly/src/library/mod.rs +++ b/assembly/src/library/mod.rs @@ -348,6 +348,11 @@ pub struct KernelLibrary { } impl KernelLibrary { + /// Returns the inner [`MastForest`]. + pub fn mast_forest(&self) -> &MastForest { + self.library.mast_forest() + } + /// Destructures this kernel library into individual parts. pub fn into_parts(self) -> (Kernel, ModuleInfo, MastForest) { (self.kernel, self.kernel_info, self.library.mast_forest) diff --git a/miden/tests/integration/flow_control/mod.rs b/miden/tests/integration/flow_control/mod.rs index d520da6b03..2eec746080 100644 --- a/miden/tests/integration/flow_control/mod.rs +++ b/miden/tests/integration/flow_control/mod.rs @@ -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; @@ -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 = " @@ -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) }; @@ -389,15 +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_compiled_library(StdLibrary::default().into()) - .unwrap(); - let module_source = " use.std::math::u64 export.u64::overflowing_add @@ -408,12 +401,19 @@ fn procref() { "; // obtain procedures' MAST roots by compiling them as module - let module_path = "test::foo".parse::().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 = Vec::new(); + let mast_roots: Vec = { + let module_path = "test::foo".parse::().unwrap(); + let module = Module::parse_str(module_path, ModuleKind::Library, module_source).unwrap(); + let library = Assembler::default() + .with_compiled_library(StdLibrary::default().into()) + .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 diff --git a/miden/tests/integration/operations/io_ops/env_ops.rs b/miden/tests/integration/operations/io_ops/env_ops.rs index 58e2230954..8744d9aafb 100644 --- a/miden/tests/integration/operations/io_ops/env_ops.rs +++ b/miden/tests/integration/operations/io_ops/env_ops.rs @@ -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 = " @@ -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) }; diff --git a/test-utils/src/lib.rs b/test-utils/src/lib.rs index d2ccc7ae61..8e718ae548 100644 --- a/test-utils/src/lib.rs +++ b/test-utils/src/lib.rs @@ -9,7 +9,8 @@ extern crate std; // ================================================================================================ use assembly::library::CompiledLibrary; -use processor::Program; +use processor::{MastForest, Program}; + #[cfg(not(target_family = "wasm"))] use proptest::prelude::{Arbitrary, Strategy}; @@ -174,7 +175,7 @@ macro_rules! assert_assembler_diagnostic { /// ExecutionError which contains the specified substring. pub struct Test { pub source: Arc, - pub kernel: Option, + pub kernel_source: Option, pub stack_inputs: StackInputs, pub advice_inputs: AdviceInputs, pub in_debug_mode: bool, @@ -190,7 +191,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, @@ -226,8 +227,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( @@ -276,14 +280,16 @@ impl Test { // -------------------------------------------------------------------------------------------- /// Compiles a test's source and returns the resulting Program or Assembly error. - pub fn compile(&self) -> Result { - 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), 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), Some(compiled_kernel)) } else { - assembly::Assembler::default() + (Assembler::default(), None) }; let mut assembler = self .add_modules @@ -301,15 +307,18 @@ impl Test { assembler.add_compiled_library(library.clone()).unwrap(); } - 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 { - 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()) } @@ -318,8 +327,12 @@ impl Test { pub fn execute_process( &self, ) -> Result>, 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(), @@ -335,8 +348,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, 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(); @@ -354,8 +370,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) } diff --git a/test-utils/src/test_builders.rs b/test-utils/src/test_builders.rs index 4cc6fde43d..79a54088fd 100644 --- a/test-utils/src/test_builders.rs +++ b/test-utils/src/test_builders.rs @@ -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, @@ -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, @@ -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, @@ -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,