From ee2ef830f6560ea896647e6074ff63e0d7eafebf Mon Sep 17 00:00:00 2001 From: Andrey Khmuro Date: Tue, 24 Oct 2023 15:16:04 +0200 Subject: [PATCH] refactor: fix bug, add tests --- assembly/src/assembler/context.rs | 4 +- .../src/assembler/instruction/procedures.rs | 2 +- assembly/src/ast/nodes/format.rs | 8 +++ assembly/src/ast/parsers/context.rs | 8 +-- assembly/src/tests.rs | 63 ++++++++++++++++++- miden/tests/integration/flow_control/mod.rs | 53 ++++++++++++++++ 6 files changed, 126 insertions(+), 12 deletions(-) diff --git a/assembly/src/assembler/context.rs b/assembly/src/assembler/context.rs index e882e21675..2daf7ef4d3 100644 --- a/assembly/src/assembler/context.rs +++ b/assembly/src/assembler/context.rs @@ -81,7 +81,7 @@ impl AssemblyContext { /// Returns the name of the procedure by its ID from the procedure map. pub fn get_imported_procedure_name(&self, id: &ProcedureId) -> Option { - if let Some(module) = self.module_stack.first() { + if let Some(module) = self.module_stack.last() { module.proc_map.get(id).cloned() } else { None @@ -89,7 +89,7 @@ impl AssemblyContext { } /// Returns the [Procedure] by its index from the vector of compiled procedures. - pub fn get_compiled_procedure(&self, idx: u16) -> Result<&Procedure, AssemblyError> { + pub fn get_local_procedure(&self, idx: u16) -> Result<&Procedure, AssemblyError> { let module_context = self.module_stack.last().expect("no modules"); module_context .compiled_procs diff --git a/assembly/src/assembler/instruction/procedures.rs b/assembly/src/assembler/instruction/procedures.rs index 1c6a90ba9c..345d0aed70 100644 --- a/assembly/src/assembler/instruction/procedures.rs +++ b/assembly/src/assembler/instruction/procedures.rs @@ -144,7 +144,7 @@ impl Assembler { span: &mut SpanBuilder, ) -> Result, AssemblyError> { // get root of the compiled local procedure - let proc_root = context.get_compiled_procedure(proc_idx)?.mast_root(); + let proc_root = context.get_local_procedure(proc_idx)?.mast_root(); // create an array with `Push` operations containing root elements let ops: Vec = proc_root.iter().map(|elem| Operation::Push(*elem)).collect(); span.add_ops(ops) diff --git a/assembly/src/ast/nodes/format.rs b/assembly/src/ast/nodes/format.rs index 0136820fc6..ebbbb68f0a 100644 --- a/assembly/src/ast/nodes/format.rs +++ b/assembly/src/ast/nodes/format.rs @@ -120,6 +120,14 @@ impl fmt::Display for FormattableInstruction<'_> { write!(f, "call.")?; display_hex_bytes(f, &root.as_bytes())?; } + Instruction::ProcRefLocal(index) => { + let proc_name = self.context.local_proc(*index as usize); + write!(f, "procref.{proc_name}")?; + } + Instruction::ProcRefImported(proc_id) => { + let (_, path) = self.context.imported_proc(proc_id); + write!(f, "procref.{path}")?; + } _ => { // Not a procedure call. Use the normal formatting write!(f, "{}", self.instruction)?; diff --git a/assembly/src/ast/parsers/context.rs b/assembly/src/ast/parsers/context.rs index 6b70d78354..a8f990ef7d 100644 --- a/assembly/src/ast/parsers/context.rs +++ b/assembly/src/ast/parsers/context.rs @@ -218,7 +218,7 @@ impl ParserContext<'_> { // -------------------------------------------------------------------------------------------- /// Parse a `procref` token into an instruction node. - pub fn parse_procref(&self, token: &Token) -> Result { + pub fn parse_procref(&mut self, token: &Token) -> Result { match token.parse_invocation(token.parts()[0])? { InvocationTarget::ProcedureName(proc_name) => { let index = self.get_local_proc_index(proc_name, token)?; @@ -226,11 +226,7 @@ impl ParserContext<'_> { Ok(Node::Instruction(inner)) } InvocationTarget::ProcedurePath { name, module } => { - let module_path = self - .import_info - .get_module_path(module) - .ok_or_else(|| ParsingError::procedure_module_not_imported(token, module))?; - let proc_id = ProcedureId::from_name(name.as_ref(), module_path); + let proc_id = self.import_info.add_invoked_proc(&name, module, token)?; let inner = Instruction::ProcRefImported(proc_id); Ok(Node::Instruction(inner)) } diff --git a/assembly/src/tests.rs b/assembly/src/tests.rs index 2278ba2ca4..b06edddd57 100644 --- a/assembly/src/tests.rs +++ b/assembly/src/tests.rs @@ -1,6 +1,7 @@ use crate::{ ast::{ModuleAst, ProgramAst}, - Assembler, AssemblyContext, Library, LibraryNamespace, LibraryPath, Module, Version, + Assembler, AssemblyContext, AssemblyError, Library, LibraryNamespace, LibraryPath, MaslLibrary, + Module, ProcedureName, Version, }; use core::slice::Iter; @@ -220,7 +221,7 @@ fn call_without_path() { // ================================================================================================ #[test] -fn test() { +fn procref_call() { // instantiate assembler let assembler = Assembler::default(); @@ -275,7 +276,7 @@ fn test() { proc.baz.4 push.3.4 end - + begin procref.two::bar procref.two::foo @@ -292,6 +293,62 @@ fn test() { .unwrap(); } +#[test] +fn get_proc_name_of_unknown_module() { + // Module `two` is unknown. This program should return + // `AssemblyError::imported_proc_module_not_found` error with `bar` procedure name. + let module_source1 = " + use.module::path::two + + export.foo + procref.two::bar + end"; + let module_ast1 = ModuleAst::parse(module_source1).unwrap(); + let module_path1 = LibraryPath::new("module::path::one").unwrap(); + let module1 = Module::new(module_path1, module_ast1); + + let masl_lib = MaslLibrary::new( + LibraryNamespace::new("module").unwrap(), + Version::default(), + false, + vec![module1], + vec![], + ) + .unwrap(); + + // instantiate assembler + let assembler = Assembler::default().with_library(&masl_lib).unwrap(); + + // compile program with procref calls + let program_source = ProgramAst::parse( + " + use.module::path::one + + begin + procref.one::foo + end", + ) + .unwrap(); + + let compilation_error = assembler + .compile_in_context( + &program_source, + &mut AssemblyContext::for_program(Some(&program_source)), + ) + .err() + .unwrap(); + + let expected_error = AssemblyError::imported_proc_module_not_found( + &crate::ProcedureId([ + 17, 137, 148, 17, 42, 108, 60, 23, 205, 115, 62, 70, 16, 121, 221, 142, 51, 247, 250, + 43, + ]), + ProcedureName::try_from("bar").ok(), + ); + + assert_eq!(compilation_error, expected_error); +} + // CONSTANTS // ================================================================================================ diff --git a/miden/tests/integration/flow_control/mod.rs b/miden/tests/integration/flow_control/mod.rs index aec1e0c046..e8140abdc1 100644 --- a/miden/tests/integration/flow_control/mod.rs +++ b/miden/tests/integration/flow_control/mod.rs @@ -1,3 +1,4 @@ +use assembly::{ast::ModuleAst, LibraryNamespace, LibraryPath, MaslLibrary, Module}; use test_utils::{build_test, AdviceInputs, StackInputs, Test, TestError}; // SIMPLE FLOW CONTROL TESTS @@ -352,3 +353,55 @@ fn simple_dyncall() { false, ); } + +// PROCREF INSTRUCTION +// ================================================================================================ + +#[test] +fn fmpadd() { + let module_source = " + export.foo + push.1.2 + end"; + let module_ast = ModuleAst::parse(module_source).unwrap(); + let library_path = LibraryPath::new("module::path::one").unwrap(); + let module = Module::new(library_path, module_ast); + let masl_lib = MaslLibrary::new( + LibraryNamespace::new("module").unwrap(), + assembly::Version::default(), + false, + vec![module], + vec![], + ) + .unwrap(); + + let source = " + use.module::path::one + + proc.baz.4 + push.3.4 + end + + begin + procref.one::foo + push.0 + procref.baz + end"; + + let mut test = build_test!(source, &[]); + test.libraries = vec![masl_lib]; + + test.expect_stack(&[ + 14955017261620687123, + 7483764806157722537, + 3983040829500348437, + 17415803850183235164, + 0, + 10769795280686168241, + 18286248910168089036, + 9534016474345631087, + 17844857521614540683, + ]); + + test.prove_and_verify(vec![], false); +}