From 9e3c498bc083e5965e90a960e0cb7bec5f53eb9e Mon Sep 17 00:00:00 2001 From: Marco Stronati Date: Mon, 13 Jan 2025 11:43:18 +0100 Subject: [PATCH 1/6] fix(cli): add support for non-vendored libraries to `run` command --- miden/src/cli/run.rs | 10 ++++++++++ miden/tests/integration/cli/cli_test.rs | 22 ++++++++++++++++++++++ miden/tests/integration/cli/data/main.masm | 5 +++++ 3 files changed, 37 insertions(+) create mode 100644 miden/tests/integration/cli/data/main.masm diff --git a/miden/src/cli/run.rs b/miden/src/cli/run.rs index 663d393b46..2690e537c3 100644 --- a/miden/src/cli/run.rs +++ b/miden/src/cli/run.rs @@ -109,6 +109,13 @@ impl RunCmd { #[instrument(name = "run_program", skip_all)] fn run_program(params: &RunCmd) -> Result<(ExecutionTrace, [u8; 32]), Report> { + for lib in ¶ms.library_paths { + if !lib.is_file() { + let name = lib.display(); + return Err(Report::msg(format!("{name} must be a file."))); + }; + } + // load libraries from files let libraries = Libraries::new(¶ms.library_paths)?; @@ -131,6 +138,9 @@ fn run_program(params: &RunCmd) -> Result<(ExecutionTrace, [u8; 32]), Report> { let stack_inputs = input_data.parse_stack_inputs().map_err(Report::msg)?; let mut host = DefaultHost::new(input_data.parse_advice_provider().map_err(Report::msg)?); host.load_mast_forest(StdLibrary::default().mast_forest().clone()).unwrap(); + for lib in libraries.libraries { + host.load_mast_forest(lib.mast_forest().clone()).unwrap(); + } let program_hash: [u8; 32] = program.hash().into(); diff --git a/miden/tests/integration/cli/cli_test.rs b/miden/tests/integration/cli/cli_test.rs index b50801e98a..c0278031e9 100644 --- a/miden/tests/integration/cli/cli_test.rs +++ b/miden/tests/integration/cli/cli_test.rs @@ -103,3 +103,25 @@ fn cli_bundle_output() { assert!(Path::new("test.masl").exists()); fs::remove_file("test.masl").unwrap() } + +#[test] +// First compile a library to a .masl file, then run a program that uses it. +fn cli_run_with_lib() -> Result<(), Box> { + let mut cmd = bin_under_test().command(); + cmd.arg("bundle") + .arg("./tests/integration/cli/data/lib") + .arg("--output") + .arg("lib.masl"); + cmd.assert().success(); + + let mut cmd = bin_under_test().command(); + cmd.arg("run") + .arg("-a") + .arg("./tests/integration/cli/data/main.masm") + .arg("-l") + .arg("./lib.masl"); + cmd.assert().success(); + + fs::remove_file("lib.masl").unwrap(); + Ok(()) +} diff --git a/miden/tests/integration/cli/data/main.masm b/miden/tests/integration/cli/data/main.masm new file mode 100644 index 0000000000..057b852c80 --- /dev/null +++ b/miden/tests/integration/cli/data/main.masm @@ -0,0 +1,5 @@ +use.lib::lib + +begin + exec.lib::lib_proc +end From 77422d9e5e6857af1d7fbb4510bee85d52ac61c7 Mon Sep 17 00:00:00 2001 From: Marco Stronati Date: Mon, 13 Jan 2025 16:55:55 +0100 Subject: [PATCH 2/6] refactor(Assembler): minor leftovers from PR #1606 --- assembly/src/assembler/mod.rs | 4 ++-- assembly/src/assembler/module_graph/mod.rs | 11 +++-------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/assembly/src/assembler/mod.rs b/assembly/src/assembler/mod.rs index 25df78c870..abac93f3d8 100644 --- a/assembly/src/assembler/mod.rs +++ b/assembly/src/assembler/mod.rs @@ -170,7 +170,7 @@ impl Assembler { module: impl Compile, options: CompileOptions, ) -> Result { - let ids = self.add_modules_with_options(vec![module], options)?; + let ids = self.add_modules_with_options([module], options)?; Ok(ids[0]) } @@ -196,7 +196,7 @@ impl Assembler { Ok(module) }) .collect::, Report>>()?; - let ids = self.module_graph.add_ast_modules(modules.into_iter())?; + let ids = self.module_graph.add_ast_modules(modules)?; Ok(ids) } /// Adds all modules (defined by ".masm" files) from the specified directory to the module diff --git a/assembly/src/assembler/module_graph/mod.rs b/assembly/src/assembler/module_graph/mod.rs index 6c02e89f7b..1a237ef686 100644 --- a/assembly/src/assembler/module_graph/mod.rs +++ b/assembly/src/assembler/module_graph/mod.rs @@ -207,10 +207,6 @@ impl ModuleGraph { /// Add `module` to the graph. /// - /// NOTE: This operation only adds a module to the graph, but does not perform the - /// important analysis needed for compilation, you must call [recompute] once all modules - /// are added to ensure the analysis results reflect the current version of the graph. - /// /// # Errors /// /// This operation can fail for the following reasons: @@ -223,9 +219,8 @@ impl ModuleGraph { /// This function will panic if the number of modules exceeds the maximum representable /// [ModuleIndex] value, `u16::MAX`. pub fn add_ast_module(&mut self, module: Box) -> Result { - let res = self.add_module(PendingWrappedModule::Ast(module))?; - self.recompute()?; - Ok(res) + let ids = self.add_ast_modules([module])?; + Ok(ids[0]) } fn add_module(&mut self, module: PendingWrappedModule) -> Result { @@ -242,7 +237,7 @@ impl ModuleGraph { pub fn add_ast_modules( &mut self, - modules: impl Iterator>, + modules: impl IntoIterator>, ) -> Result, AssemblyError> { let idx = modules .into_iter() From 90545d4a766023deab9ce609a2506312c56d5716 Mon Sep 17 00:00:00 2001 From: Marco Stronati Date: Tue, 28 Jan 2025 11:23:15 +0100 Subject: [PATCH 3/6] refactor(Assembler): remove_nodes do not return option --- assembly/src/assembler/mast_forest_builder.rs | 7 +++---- assembly/src/assembler/mod.rs | 12 ++++-------- core/src/mast/mod.rs | 11 +++++------ 3 files changed, 12 insertions(+), 18 deletions(-) diff --git a/assembly/src/assembler/mast_forest_builder.rs b/assembly/src/assembler/mast_forest_builder.rs index 0a17bd07c7..0734673218 100644 --- a/assembly/src/assembler/mast_forest_builder.rs +++ b/assembly/src/assembler/mast_forest_builder.rs @@ -65,10 +65,9 @@ impl MastForestBuilder { /// Removes the unused nodes that were created as part of the assembly process, and returns the /// resulting MAST forest. /// - /// It also returns the map from old node IDs to new node IDs; or `None` if the `MastForest` was - /// unchanged. Any [`MastNodeId`] used in reference to the old [`MastForest`] should be remapped - /// using this map. - pub fn build(mut self) -> (MastForest, Option>) { + /// It also returns the map from old node IDs to new node IDs. Any [`MastNodeId`] used in + /// reference to the old [`MastForest`] should be remapped using this map. + pub fn build(mut self) -> (MastForest, BTreeMap) { let nodes_to_remove = get_nodes_to_remove(self.merged_basic_block_ids, &self.mast_forest); let id_remappings = self.mast_forest.remove_nodes(&nodes_to_remove); diff --git a/assembly/src/assembler/mod.rs b/assembly/src/assembler/mod.rs index abac93f3d8..0b2c04fb24 100644 --- a/assembly/src/assembler/mod.rs +++ b/assembly/src/assembler/mod.rs @@ -326,11 +326,9 @@ impl Assembler { }; let (mast_forest, id_remappings) = mast_forest_builder.build(); - if let Some(id_remappings) = id_remappings { - for (_proc_name, node_id) in exports.iter_mut() { - if let Some(&new_node_id) = id_remappings.get(node_id) { - *node_id = new_node_id; - } + for (_proc_name, node_id) in exports.iter_mut() { + if let Some(&new_node_id) = id_remappings.get(node_id) { + *node_id = new_node_id; } } @@ -402,9 +400,7 @@ impl Assembler { // in case the node IDs changed, update the entrypoint ID to the new value let (mast_forest, id_remappings) = mast_forest_builder.build(); - let entry_node_id = id_remappings - .map(|id_remappings| id_remappings[&entry_node_id]) - .unwrap_or(entry_node_id); + let entry_node_id = id_remappings.get(&entry_node_id).unwrap_or(&entry_node_id); Ok(Program::with_kernel( mast_forest.into(), diff --git a/core/src/mast/mod.rs b/core/src/mast/mod.rs index d71be872d9..d1d1c08ce6 100644 --- a/core/src/mast/mod.rs +++ b/core/src/mast/mod.rs @@ -179,15 +179,14 @@ impl MastForest { /// removal (i.e. will point to an incorrect node after the removal), and this removal operation /// would result in an invalid [`MastForest`]. /// - /// It also returns the map from old node IDs to new node IDs; or `None` if the set of nodes to - /// remove was empty. Any [`MastNodeId`] used in reference to the old [`MastForest`] should be - /// remapped using this map. + /// It also returns the map from old node IDs to new node IDs. Any [`MastNodeId`] used in + /// reference to the old [`MastForest`] should be remapped using this map. pub fn remove_nodes( &mut self, nodes_to_remove: &BTreeSet, - ) -> Option> { + ) -> BTreeMap { if nodes_to_remove.is_empty() { - return None; + return [].into(); } let old_nodes = mem::take(&mut self.nodes); @@ -196,7 +195,7 @@ impl MastForest { self.remap_and_add_nodes(retained_nodes, &id_remappings); self.remap_and_add_roots(old_root_ids, &id_remappings); - Some(id_remappings) + id_remappings } pub fn set_before_enter(&mut self, node_id: MastNodeId, decorator_ids: Vec) { From a0ad3f274eb4ecfb157889bab5d86e41ac9f36ed Mon Sep 17 00:00:00 2001 From: Marco Stronati Date: Fri, 24 Jan 2025 14:40:34 +0100 Subject: [PATCH 4/6] feat(Mast): add SubtreeIterator and remapping of NodeIds --- core/src/mast/mod.rs | 44 +++++++++++++++++++++++++++++++- core/src/mast/node/call_node.rs | 6 ++++- core/src/mast/node/join_node.rs | 7 ++++- core/src/mast/node/loop_node.rs | 6 ++++- core/src/mast/node/mod.rs | 38 ++++++++++++++++++++++++++- core/src/mast/node/split_node.rs | 7 ++++- 6 files changed, 102 insertions(+), 6 deletions(-) diff --git a/core/src/mast/mod.rs b/core/src/mast/mod.rs index d1d1c08ce6..49d536e274 100644 --- a/core/src/mast/mod.rs +++ b/core/src/mast/mod.rs @@ -186,7 +186,7 @@ impl MastForest { nodes_to_remove: &BTreeSet, ) -> BTreeMap { if nodes_to_remove.is_empty() { - return [].into(); + return BTreeMap::new(); } let old_nodes = mem::take(&mut self.nodes); @@ -528,6 +528,9 @@ impl IndexMut for MastForest { #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct MastNodeId(u32); +/// Operations that mutate a MAST often produce this mapping between old and new NodeIds. +pub type Remapping = BTreeMap; + impl MastNodeId { /// Returns a new `MastNodeId` with the provided inner value, or an error if the provided /// `value` is greater than the number of nodes in the forest. @@ -594,6 +597,13 @@ impl MastNodeId { pub fn as_u32(&self) -> u32 { self.0 } + + /// Remap the NodeId to its new position using the given [`Remapping`]. + pub fn remap(&mut self, remapping: &Remapping) { + if let Some(new_id) = remapping.get(self) { + self.0 = new_id.0 + } + } } impl From for usize { @@ -620,6 +630,38 @@ impl fmt::Display for MastNodeId { } } +// ITERATOR + +/// Iterates over all the nodes a root depends on, in pre-order. +/// The iteration can include other roots in the same forest. +pub struct SubtreeIterator<'a> { + forest: &'a MastForest, + discovered: Vec, + unvisited: Vec, +} +impl<'a> SubtreeIterator<'a> { + pub fn new(root: &MastNodeId, forest: &'a MastForest) -> Self { + let discovered = vec![]; + let unvisited = vec![*root]; + SubtreeIterator { forest, discovered, unvisited } + } +} +impl Iterator for SubtreeIterator<'_> { + type Item = MastNodeId; + fn next(&mut self) -> Option { + while let Some(id) = self.unvisited.pop() { + let node = &self.forest[id]; + if !node.has_children() { + return Some(id); + } else { + self.discovered.push(id); + node.append_children_to(&mut self.unvisited); + } + } + self.discovered.pop() + } +} + // DECORATOR ID // ================================================================================================ diff --git a/core/src/mast/node/call_node.rs b/core/src/mast/node/call_node.rs index 7f207d386f..97e9eff2ec 100644 --- a/core/src/mast/node/call_node.rs +++ b/core/src/mast/node/call_node.rs @@ -9,7 +9,7 @@ use miden_formatting::{ use crate::{ chiplets::hasher, - mast::{DecoratorId, MastForest, MastForestError, MastNodeId}, + mast::{DecoratorId, MastForest, MastForestError, MastNodeId, Remapping}, OPCODE_CALL, OPCODE_SYSCALL, }; @@ -169,6 +169,10 @@ impl CallNode { /// Mutators impl CallNode { + pub fn remap_children(&mut self, remapping: &Remapping) { + self.callee.remap(remapping) + } + /// Sets the list of decorators to be executed before this node. pub fn set_before_enter(&mut self, decorator_ids: Vec) { self.before_enter = decorator_ids; diff --git a/core/src/mast/node/join_node.rs b/core/src/mast/node/join_node.rs index d3d04b4510..0be3a828b1 100644 --- a/core/src/mast/node/join_node.rs +++ b/core/src/mast/node/join_node.rs @@ -5,7 +5,7 @@ use miden_crypto::{hash::rpo::RpoDigest, Felt}; use crate::{ chiplets::hasher, - mast::{DecoratorId, MastForest, MastForestError, MastNodeId}, + mast::{DecoratorId, MastForest, MastForestError, MastNodeId, Remapping}, prettier::PrettyPrint, OPCODE_JOIN, }; @@ -110,6 +110,11 @@ impl JoinNode { /// Mutators impl JoinNode { + pub fn remap_children(&mut self, remapping: &Remapping) { + self.children[0].remap(remapping); + self.children[1].remap(remapping); + } + /// Sets the list of decorators to be executed before this node. pub fn set_before_enter(&mut self, decorator_ids: Vec) { self.before_enter = decorator_ids; diff --git a/core/src/mast/node/loop_node.rs b/core/src/mast/node/loop_node.rs index 6091ce034d..d4696d2cba 100644 --- a/core/src/mast/node/loop_node.rs +++ b/core/src/mast/node/loop_node.rs @@ -6,7 +6,7 @@ use miden_formatting::prettier::PrettyPrint; use crate::{ chiplets::hasher, - mast::{DecoratorId, MastForest, MastForestError, MastNodeId}, + mast::{DecoratorId, MastForest, MastForestError, MastNodeId, Remapping}, OPCODE_LOOP, }; @@ -99,6 +99,10 @@ impl LoopNode { /// Mutators impl LoopNode { + pub fn remap_children(&mut self, remapping: &Remapping) { + self.body.remap(remapping); + } + /// Sets the list of decorators to be executed before this node. pub fn set_before_enter(&mut self, decorator_ids: Vec) { self.before_enter = decorator_ids; diff --git a/core/src/mast/node/mod.rs b/core/src/mast/node/mod.rs index 2dfa9dd037..0845acdd0d 100644 --- a/core/src/mast/node/mod.rs +++ b/core/src/mast/node/mod.rs @@ -29,7 +29,7 @@ pub use loop_node::LoopNode; use super::{DecoratorId, MastForestError}; use crate::{ - mast::{MastForest, MastNodeId}, + mast::{MastForest, MastNodeId, Remapping}, DecoratorList, Operation, }; @@ -143,6 +143,42 @@ impl MastNode { } } + /// Remap the node children to their new positions indicated by the given [`Remapping`]. + pub fn remap_children(&mut self, remapping: &Remapping) { + match self { + MastNode::Join(join_node) => join_node.remap_children(remapping), + MastNode::Split(split_node) => split_node.remap_children(remapping), + MastNode::Loop(loop_node) => loop_node.remap_children(remapping), + MastNode::Call(call_node) => call_node.remap_children(remapping), + MastNode::Block(_) | MastNode::Dyn(_) | MastNode::External(_) => (), + } + } + + /// Returns true if the this node has children. + pub fn has_children(&self) -> bool { + match &self { + MastNode::Join(_) | MastNode::Split(_) | MastNode::Loop(_) | MastNode::Call(_) => true, + MastNode::Block(_) | MastNode::Dyn(_) | MastNode::External(_) => false, + } + } + + /// Appends the NodeIds of the children of this node, if any, to the vector. + pub fn append_children_to(&self, target: &mut Vec) { + match &self { + MastNode::Join(join_node) => { + target.push(join_node.first()); + target.push(join_node.second()) + }, + MastNode::Split(split_node) => { + target.push(split_node.on_true()); + target.push(split_node.on_false()) + }, + MastNode::Loop(loop_node) => target.push(loop_node.body()), + MastNode::Call(call_node) => target.push(call_node.callee()), + MastNode::Block(_) | MastNode::Dyn(_) | MastNode::External(_) => (), + } + } + pub fn to_pretty_print<'a>(&'a self, mast_forest: &'a MastForest) -> impl PrettyPrint + 'a { match self { MastNode::Block(basic_block_node) => { diff --git a/core/src/mast/node/split_node.rs b/core/src/mast/node/split_node.rs index 8a46fcdc70..0dee745296 100644 --- a/core/src/mast/node/split_node.rs +++ b/core/src/mast/node/split_node.rs @@ -6,7 +6,7 @@ use miden_formatting::prettier::PrettyPrint; use crate::{ chiplets::hasher, - mast::{DecoratorId, MastForest, MastForestError, MastNodeId}, + mast::{DecoratorId, MastForest, MastForestError, MastNodeId, Remapping}, OPCODE_SPLIT, }; @@ -112,6 +112,11 @@ impl SplitNode { /// Mutators impl SplitNode { + pub fn remap_children(&mut self, remapping: &Remapping) { + self.branches[0].remap(remapping); + self.branches[1].remap(remapping); + } + /// Sets the list of decorators to be executed before this node. pub fn set_before_enter(&mut self, decorator_ids: Vec) { self.before_enter = decorator_ids; From 6d04a86603c1ee5adb6e948a253eca5cc899c7e3 Mon Sep 17 00:00:00 2001 From: Marco Stronati Date: Thu, 30 Jan 2025 13:57:37 +0100 Subject: [PATCH 5/6] feat(Assembler): add support for vendoring compiled libraries --- CHANGELOG.md | 1 + assembly/src/assembler/mast_forest_builder.rs | 54 +++++++++++++++++-- assembly/src/assembler/mod.rs | 54 ++++++++++++++----- assembly/src/tests.rs | 28 ++++++++++ 4 files changed, 119 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 03a183f366..7f147fa6ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ #### Changes - Update minimum supported Rust version to 1.84. - Change Chiplet Fields to Public (#1629). +- Added to the `Assembler` the ability to vendor a compiled library. ## 0.12.0 (2025-01-22) diff --git a/assembly/src/assembler/mast_forest_builder.rs b/assembly/src/assembler/mast_forest_builder.rs index 0734673218..98ea5e5b8c 100644 --- a/assembly/src/assembler/mast_forest_builder.rs +++ b/assembly/src/assembler/mast_forest_builder.rs @@ -1,19 +1,22 @@ use alloc::{ collections::{BTreeMap, BTreeSet}, + sync::Arc, vec::Vec, }; use core::ops::{Index, IndexMut}; +use miette::{IntoDiagnostic, Report}; use vm_core::{ crypto::hash::RpoDigest, mast::{ DecoratorFingerprint, DecoratorId, MastForest, MastNode, MastNodeFingerprint, MastNodeId, + Remapping, SubtreeIterator, }, Decorator, DecoratorList, Operation, }; use super::{GlobalProcedureIndex, Procedure}; -use crate::AssemblyError; +use crate::{AssemblyError, Library}; // CONSTANTS // ================================================================================================ @@ -59,9 +62,36 @@ pub struct MastForestBuilder { /// used as a candidate set of nodes that may be eliminated if the are not referenced by any /// other node in the forest and are not a root of any procedure. merged_basic_block_ids: BTreeSet, + /// A MastForest that contains vendored libraries, it's used to find precompiled procedures and + /// copy their subtrees instead of inserting external nodes. + vendored_mast: Arc, + /// Keeps track of the new ids assigned to nodes that are copied from the vendored_mast. + vendored_remapping: Remapping, } impl MastForestBuilder { + /// Creates a new builder that can access vendored libraries. + /// + /// When [`Self::vendor_or_ensure_external`] is called, if the root is present in the vendored + /// libraries, the body of the function is copied in the MAST Forest being built. Otherwise an + /// external node is inserted and the funtion body is expected to be found in a library added at + /// runtime. + pub fn new<'a>( + vendored_libraries: impl IntoIterator, + ) -> Result { + // All vendored library are merged into a single MastForest. + let forests = vendored_libraries.into_iter().map(|lib| lib.mast_forest().as_ref()); + let (vendored_mast, _remapping) = MastForest::merge(forests).into_diagnostic()?; + // The adviceMap of the vendored forest is copied to the forest being built. + let mut mast_forest = MastForest::default(); + *mast_forest.advice_map_mut() = vendored_mast.advice_map().clone(); + Ok(MastForestBuilder { + mast_forest, + vendored_mast: Arc::new(vendored_mast), + ..Self::default() + }) + } + /// Removes the unused nodes that were created as part of the assembly process, and returns the /// resulting MAST forest. /// @@ -449,9 +479,25 @@ impl MastForestBuilder { self.ensure_node(MastNode::new_dyncall()) } - /// Adds an external node to the forest, and returns the [`MastNodeId`] associated with it. - pub fn ensure_external(&mut self, mast_root: RpoDigest) -> Result { - self.ensure_node(MastNode::new_external(mast_root)) + /// If the root is present in the vendored MAST, its subtree is copied. Otherwise an + /// external node is added to the forest. + pub fn vendor_or_ensure_external( + &mut self, + mast_root: RpoDigest, + ) -> Result { + if let Some(root_id) = self.vendored_mast.find_procedure_root(mast_root) { + for old_id in SubtreeIterator::new(&root_id, &self.vendored_mast.clone()) { + let mut node = self.vendored_mast[old_id].clone(); + node.remap_children(&self.vendored_remapping); + let new_id = self.ensure_node(node)?; + self.vendored_remapping.insert(old_id, new_id); + } + let mut new_root_id = root_id; + new_root_id.remap(&self.vendored_remapping); + Ok(root_id) + } else { + self.ensure_node(MastNode::new_external(mast_root)) + } } pub fn set_before_enter(&mut self, node_id: MastNodeId, decorator_ids: Vec) { diff --git a/assembly/src/assembler/mod.rs b/assembly/src/assembler/mod.rs index 0b2c04fb24..72b3565140 100644 --- a/assembly/src/assembler/mod.rs +++ b/assembly/src/assembler/mod.rs @@ -71,6 +71,8 @@ pub struct Assembler { warnings_as_errors: bool, /// Whether the assembler enables extra debugging information. in_debug_mode: bool, + /// Collects libraries that can be used during assembly to vendor procedures. + vendored_libraries: BTreeMap, } impl Default for Assembler { @@ -82,6 +84,7 @@ impl Default for Assembler { module_graph, warnings_as_errors: false, in_debug_mode: false, + vendored_libraries: BTreeMap::new(), } } } @@ -97,6 +100,7 @@ impl Assembler { module_graph, warnings_as_errors: false, in_debug_mode: false, + vendored_libraries: BTreeMap::new(), } } @@ -223,13 +227,11 @@ impl Assembler { /// Adds the compiled library to provide modules for the compilation. /// - /// We only current support adding non-vendored libraries - that is, the source code of exported - /// procedures is not included in the program that compiles against the library. The library's - /// source code is instead expected to be loaded in the processor at execution time. Hence, all - /// calls to library procedures will be compiled down to a [`vm_core::mast::ExternalNode`] (i.e. - /// a reference to the procedure's MAST root). This means that when executing a program compiled - /// against a library, the processor will not be able to differentiate procedures with the same - /// MAST root but different decorators. + /// All calls to the library's procedures will be compiled down to a + /// [`vm_core::mast::ExternalNode`] (i.e. a reference to the procedure's MAST root). + /// The library's source code is expected to be loaded in the processor at execution time. + /// This means that when executing a program compiled against a library, the processor will not + /// be able to differentiate procedures with the same MAST root but different decorators. /// /// Hence, it is not recommended to export two procedures that have the same MAST root (i.e. are /// identical except for their decorators). Note however that we don't expect this scenario to @@ -251,6 +253,27 @@ impl Assembler { self.add_library(library)?; Ok(self) } + + /// Adds a compiled library from which procedures will be vendored into the assembled code. + /// + /// Vendoring in this context means that when a procedure from this library is invoked from the + /// assembled code, the entire procedure MAST will be copied into the assembled code. Thus, + /// when the resulting code is executed on the VM, the vendored library does not need to be + /// provided to the VM to resolve external calls. + pub fn add_vendored_library(&mut self, library: impl AsRef) -> Result<(), Report> { + self.add_library(&library)?; + self.vendored_libraries + .insert(*library.as_ref().digest(), library.as_ref().clone()); + Ok(()) + } + + /// Adds a compiled library from which procedures will be vendored into the assembled code. + /// + /// See [`Self::add_vendored_library`] + pub fn with_vendored_library(mut self, library: impl AsRef) -> Result { + self.add_vendored_library(library)?; + Ok(self) + } } // ------------------------------------------------------------------------------------------------ @@ -298,9 +321,9 @@ impl Assembler { modules: impl IntoIterator, options: CompileOptions, ) -> Result { - let ast_module_indices = self.add_modules_with_options(modules, options)?; + let mut mast_forest_builder = MastForestBuilder::new(self.vendored_libraries.values())?; - let mut mast_forest_builder = MastForestBuilder::default(); + let ast_module_indices = self.add_modules_with_options(modules, options)?; let mut exports = { let mut exports = BTreeMap::new(); @@ -391,7 +414,8 @@ impl Assembler { .ok_or(SemanticAnalysisError::MissingEntrypoint)?; // Compile the module graph rooted at the entrypoint - let mut mast_forest_builder = MastForestBuilder::default(); + let mut mast_forest_builder = MastForestBuilder::new(self.vendored_libraries.values())?; + self.compile_subgraph(entrypoint, &mut mast_forest_builder)?; let entry_node_id = mast_forest_builder .get_procedure(entrypoint) @@ -400,7 +424,7 @@ impl Assembler { // in case the node IDs changed, update the entrypoint ID to the new value let (mast_forest, id_remappings) = mast_forest_builder.build(); - let entry_node_id = id_remappings.get(&entry_node_id).unwrap_or(&entry_node_id); + let entry_node_id = *id_remappings.get(&entry_node_id).unwrap_or(&entry_node_id); Ok(Program::with_kernel( mast_forest.into(), @@ -765,8 +789,10 @@ impl Assembler { } } - /// Verifies the validity of the MAST root as a procedure root hash, and returns the ID of the - /// [`core::mast::ExternalNode`] that wraps it. + /// Verifies the validity of the MAST root as a procedure root hash, and adds it to the forest. + /// + /// If the root is present in the vendored MAST, its subtree is copied. Otherwise an + /// external node is added to the forest. fn ensure_valid_procedure_mast_root( &self, kind: InvokeKind, @@ -818,7 +844,7 @@ impl Assembler { Some(_) | None => (), } - mast_forest_builder.ensure_external(mast_root) + mast_forest_builder.vendor_or_ensure_external(mast_root) } } diff --git a/assembly/src/tests.rs b/assembly/src/tests.rs index e62802cce9..9640641052 100644 --- a/assembly/src/tests.rs +++ b/assembly/src/tests.rs @@ -3029,3 +3029,31 @@ fn test_program_serde_with_decorators() { assert_eq!(original_program, deserialized_program); } + +#[test] +fn vendoring() -> TestResult { + let context = TestContext::new(); + let mut mod_parser = ModuleParser::new(ModuleKind::Library); + let vendor_lib = { + let source = source_file!(&context, "export.bar push.1 end export.prune push.2 end"); + let mod1 = mod_parser.parse(LibraryPath::new("test::mod1").unwrap(), source).unwrap(); + Assembler::default().assemble_library([mod1]).unwrap() + }; + + let lib = { + let source = source_file!(&context, "export.foo exec.::test::mod1::bar end"); + let mod2 = mod_parser.parse(LibraryPath::new("test::mod2").unwrap(), source).unwrap(); + + let mut assembler = Assembler::default(); + assembler.add_vendored_library(vendor_lib)?; + assembler.assemble_library([mod2]).unwrap() + }; + + let expected_lib = { + let source = source_file!(&context, "export.foo push.1 end"); + let mod2 = mod_parser.parse(LibraryPath::new("test::mod2").unwrap(), source).unwrap(); + Assembler::default().assemble_library([mod2]).unwrap() + }; + assert!(lib == expected_lib); + Ok(()) +} From ea1289990117e6b120fdce57f4ecb670f8fdcf9b Mon Sep 17 00:00:00 2001 From: Marco Stronati Date: Mon, 10 Feb 2025 11:23:25 +0100 Subject: [PATCH 6/6] fixup: remap returns a node --- assembly/src/assembler/mast_forest_builder.rs | 7 ++----- core/src/mast/mod.rs | 6 ++---- core/src/mast/node/call_node.rs | 6 ++++-- core/src/mast/node/join_node.rs | 8 +++++--- core/src/mast/node/loop_node.rs | 6 ++++-- core/src/mast/node/mod.rs | 13 +++++++------ core/src/mast/node/split_node.rs | 8 +++++--- 7 files changed, 29 insertions(+), 25 deletions(-) diff --git a/assembly/src/assembler/mast_forest_builder.rs b/assembly/src/assembler/mast_forest_builder.rs index 98ea5e5b8c..6eb3c9b7f1 100644 --- a/assembly/src/assembler/mast_forest_builder.rs +++ b/assembly/src/assembler/mast_forest_builder.rs @@ -487,14 +487,11 @@ impl MastForestBuilder { ) -> Result { if let Some(root_id) = self.vendored_mast.find_procedure_root(mast_root) { for old_id in SubtreeIterator::new(&root_id, &self.vendored_mast.clone()) { - let mut node = self.vendored_mast[old_id].clone(); - node.remap_children(&self.vendored_remapping); + let node = self.vendored_mast[old_id].remap_children(&self.vendored_remapping); let new_id = self.ensure_node(node)?; self.vendored_remapping.insert(old_id, new_id); } - let mut new_root_id = root_id; - new_root_id.remap(&self.vendored_remapping); - Ok(root_id) + Ok(root_id.remap(&self.vendored_remapping)) } else { self.ensure_node(MastNode::new_external(mast_root)) } diff --git a/core/src/mast/mod.rs b/core/src/mast/mod.rs index 49d536e274..3e9f8c46a0 100644 --- a/core/src/mast/mod.rs +++ b/core/src/mast/mod.rs @@ -599,10 +599,8 @@ impl MastNodeId { } /// Remap the NodeId to its new position using the given [`Remapping`]. - pub fn remap(&mut self, remapping: &Remapping) { - if let Some(new_id) = remapping.get(self) { - self.0 = new_id.0 - } + pub fn remap(&self, remapping: &Remapping) -> Self { + *remapping.get(self).unwrap_or(self) } } diff --git a/core/src/mast/node/call_node.rs b/core/src/mast/node/call_node.rs index 97e9eff2ec..eada54225b 100644 --- a/core/src/mast/node/call_node.rs +++ b/core/src/mast/node/call_node.rs @@ -169,8 +169,10 @@ impl CallNode { /// Mutators impl CallNode { - pub fn remap_children(&mut self, remapping: &Remapping) { - self.callee.remap(remapping) + pub fn remap_children(&self, remapping: &Remapping) -> Self { + let mut node = self.clone(); + node.callee = node.callee.remap(remapping); + node } /// Sets the list of decorators to be executed before this node. diff --git a/core/src/mast/node/join_node.rs b/core/src/mast/node/join_node.rs index 0be3a828b1..a042de5e39 100644 --- a/core/src/mast/node/join_node.rs +++ b/core/src/mast/node/join_node.rs @@ -110,9 +110,11 @@ impl JoinNode { /// Mutators impl JoinNode { - pub fn remap_children(&mut self, remapping: &Remapping) { - self.children[0].remap(remapping); - self.children[1].remap(remapping); + pub fn remap_children(&self, remapping: &Remapping) -> Self { + let mut node = self.clone(); + node.children[0] = node.children[0].remap(remapping); + node.children[1] = node.children[1].remap(remapping); + node } /// Sets the list of decorators to be executed before this node. diff --git a/core/src/mast/node/loop_node.rs b/core/src/mast/node/loop_node.rs index d4696d2cba..ab0afe1208 100644 --- a/core/src/mast/node/loop_node.rs +++ b/core/src/mast/node/loop_node.rs @@ -99,8 +99,10 @@ impl LoopNode { /// Mutators impl LoopNode { - pub fn remap_children(&mut self, remapping: &Remapping) { - self.body.remap(remapping); + pub fn remap_children(&self, remapping: &Remapping) -> Self { + let mut node = self.clone(); + node.body = node.body.remap(remapping); + node } /// Sets the list of decorators to be executed before this node. diff --git a/core/src/mast/node/mod.rs b/core/src/mast/node/mod.rs index 0845acdd0d..5135e1eb8d 100644 --- a/core/src/mast/node/mod.rs +++ b/core/src/mast/node/mod.rs @@ -144,13 +144,14 @@ impl MastNode { } /// Remap the node children to their new positions indicated by the given [`Remapping`]. - pub fn remap_children(&mut self, remapping: &Remapping) { + pub fn remap_children(&self, remapping: &Remapping) -> Self { + use MastNode::*; match self { - MastNode::Join(join_node) => join_node.remap_children(remapping), - MastNode::Split(split_node) => split_node.remap_children(remapping), - MastNode::Loop(loop_node) => loop_node.remap_children(remapping), - MastNode::Call(call_node) => call_node.remap_children(remapping), - MastNode::Block(_) | MastNode::Dyn(_) | MastNode::External(_) => (), + Join(join_node) => Join(join_node.remap_children(remapping)), + Split(split_node) => Split(split_node.remap_children(remapping)), + Loop(loop_node) => Loop(loop_node.remap_children(remapping)), + Call(call_node) => Call(call_node.remap_children(remapping)), + Block(_) | Dyn(_) | External(_) => self.clone(), } } diff --git a/core/src/mast/node/split_node.rs b/core/src/mast/node/split_node.rs index 0dee745296..aedc8f8b3d 100644 --- a/core/src/mast/node/split_node.rs +++ b/core/src/mast/node/split_node.rs @@ -112,9 +112,11 @@ impl SplitNode { /// Mutators impl SplitNode { - pub fn remap_children(&mut self, remapping: &Remapping) { - self.branches[0].remap(remapping); - self.branches[1].remap(remapping); + pub fn remap_children(&self, remapping: &Remapping) -> Self { + let mut node = self.clone(); + node.branches[0] = node.branches[0].remap(remapping); + node.branches[1] = node.branches[1].remap(remapping); + node } /// Sets the list of decorators to be executed before this node.