feat: add runtime linking of dependencies

crates/mun_runtime/Cargo.toml

@@ -16,6 +16,7 @@ categories = ["game-development", "mun"]
 abi = { version = "=0.2.0", path = "../mun_abi", package = "mun_abi" }
 anyhow = "1.0"
 libloader = { version = "=0.1.0", path = "../mun_libloader", package = "mun_libloader" }
+log = "0.4"
 md5 = "0.7.0"
 memory = { version = "=0.1.0", path = "../mun_memory", package = "mun_memory" }
 notify = "4.0.12"

crates/mun_runtime/src/assembly.rs

@@ -1,13 +1,17 @@
-use std::io;
-use std::path::{Path, PathBuf};
-
-use crate::DispatchTable;
+use crate::{
+    garbage_collector::{GarbageCollector, UnsafeTypeInfo},
+    DispatchTable,
+};
 use abi::AssemblyInfo;
+use anyhow::anyhow;
 use libloader::{MunLibrary, TempLibrary};
-
-use crate::garbage_collector::{GarbageCollector, UnsafeTypeInfo};
+use log::error;
 use memory::mapping::{Mapping, MemoryMapper};
-use std::{collections::HashSet, ptr::NonNull, sync::Arc};
+use std::{
+    path::{Path, PathBuf},
+    ptr::NonNull,
+    sync::Arc,
+};
 
 /// An assembly is a hot reloadable compilation unit, consisting of one or more Mun modules.
 pub struct Assembly {
@@ -21,11 +25,7 @@ pub struct Assembly {
 impl Assembly {
     /// Loads an assembly and its information for the shared library at `library_path`. The
     /// resulting `Assembly` is ensured to be linkable.
-    pub fn load(
-        library_path: &Path,
-        gc: Arc<GarbageCollector>,
-        runtime_dispatch_table: &DispatchTable,
-    ) -> Result<Self, anyhow::Error> {
+    pub fn load(library_path: &Path, gc: Arc<GarbageCollector>) -> Result<Self, anyhow::Error> {
         let mut library = MunLibrary::new(library_path)?;
 
         let version = library.get_abi_version();
@@ -49,99 +49,72 @@ impl Assembly {
             allocator: gc,
         };
 
-        // Ensure that any loaded `Assembly` can be linked safely.
-        assembly.ensure_linkable(runtime_dispatch_table)?;
         Ok(assembly)
     }
 
-    /// Verifies that the `Assembly` resolves all dependencies in the `DispatchTable`.
-    fn ensure_linkable(&self, runtime_dispatch_table: &DispatchTable) -> Result<(), io::Error> {
-        let fn_names: HashSet<&str> = self
-            .info
-            .symbols
-            .functions()
-            .iter()
-            .map(|f| f.prototype.name())
-            .collect();
+    /// Tries to link the `assemblies`, resulting in a new [`DispatchTable`] on success. This leaves
+    /// the original `dispatch_table` intact, in case of linking errors.
+    pub(super) fn link_all<'a>(
+        assemblies: impl Iterator<Item = &'a mut Assembly>,
+        dispatch_table: &DispatchTable,
+    ) -> Result<DispatchTable, anyhow::Error> {
+        let assemblies: Vec<&'a mut _> = assemblies.collect();
+
+        // Clone the dispatch table, such that we can roll back if linking fails
+        let mut dispatch_table = dispatch_table.clone();
+
+        // Insert all assemblies' functions into the dispatch table
+        for assembly in assemblies.iter() {
+            for function in assembly.info().symbols.functions() {
+                dispatch_table.insert_fn(function.prototype.name(), function.clone());
+            }
+        }
 
-        for (fn_ptr, fn_prototype) in self.info.dispatch_table.iter() {
+        let mut to_link: Vec<_> = assemblies
+            .into_iter()
+            .flat_map(|asm| asm.info.dispatch_table.iter_mut())
             // Only take signatures into account that do *not* yet have a function pointer assigned
             // by the compiler.
-            if !fn_ptr.is_null() {
-                continue;
-            }
+            .filter(|(ptr, _)| ptr.is_null())
+            .collect();
 
-            // Ensure that the required function is in the runtime dispatch table and that its signature
-            // is the same.
-            match runtime_dispatch_table.get_fn(fn_prototype.name()) {
-                Some(fn_definition) => {
-                    if fn_prototype.signature != fn_definition.prototype.signature {
-                        return Err(io::Error::new(
-                            io::ErrorKind::NotFound,
-                            format!("Failed to link: function '{}' is missing. A function with the same name does exist, but the signatures do not match (expected: {}, found: {}).", fn_prototype.name(), fn_prototype, fn_definition.prototype),
-                        ));
+        let mut retry = true;
+        while retry {
+            retry = false;
+            let mut failed_to_link = Vec::new();
+
+            // Try to link outstanding entries
+            for (dispatch_ptr, fn_prototype) in to_link.into_iter() {
+                // Ensure that the function is in the runtime dispatch table
+                if let Some(fn_def) = dispatch_table.get_fn(fn_prototype.name()) {
+                    // Ensure that the function's signature is the same.
+                    if fn_prototype.signature != fn_def.prototype.signature {
+                        return Err(anyhow!("Failed to link: function '{}' is missing. A function with the same name does exist, but the signatures do not match (expected: {}, found: {}).", fn_prototype.name(), fn_prototype, fn_def.prototype));
                     }
-                }
-                None => {
-                    return Err(io::Error::new(
-                        io::ErrorKind::NotFound,
-                        format!(
-                            "Failed to link: function `{}` is missing.",
-                            fn_prototype.name()
-                        ),
-                    ))
-                }
-            }
-        }
 
-        if let Some(dependencies) = runtime_dispatch_table
-            .fn_dependencies
-            .get(self.info.symbols.path())
-        {
-            for fn_name in dependencies.keys() {
-                if !fn_names.contains(&fn_name.as_str()) {
-                    return Err(io::Error::new(
-                        io::ErrorKind::NotFound,
-                        format!("Failed to link: function `{}` is missing.", fn_name),
-                    ));
+                    *dispatch_ptr = fn_def.fn_ptr;
+                    retry = true;
+                } else {
+                    failed_to_link.push((dispatch_ptr, fn_prototype));
                 }
             }
 
-            for fn_definition in self.info.symbols.functions().iter() {
-                let (fn_prototype, _) = dependencies
-                    .get(fn_definition.prototype.name())
-                    .expect("The dependency must exist after the previous check.");
-
-                if fn_prototype.signature != fn_definition.prototype.signature {
-                    return Err(io::Error::new(
-                        io::ErrorKind::NotFound,
-                        format!("Failed to link: function '{}' is missing. A function with the same name does exist, but the signatures do not match (expected: {}, found: {}).", fn_prototype.name(), fn_prototype, fn_definition.prototype),
-                    ));
-                }
-            }
+            // Move all failed entries, for (potentially) another try
+            to_link = failed_to_link;
         }
 
-        Ok(())
-    }
+        if !to_link.is_empty() {
+            for (_, fn_prototype) in to_link {
+                error!(
+                    "Failed to link: function `{}` is missing.",
+                    fn_prototype.name()
+                );
+            }
 
-    /// Links the assembly using the runtime's dispatch table.
-    ///
-    /// Requires that `ensure_linkable` has been called beforehand. This happens upon creation of
-    /// an `Assembly` - in the `load` function - making this function safe.
-    pub fn link(&mut self, runtime_dispatch_table: &mut DispatchTable) {
-        for function in self.info.symbols.functions() {
-            runtime_dispatch_table.insert_fn(function.prototype.name(), function.clone());
+            return Err(anyhow!("Failed to link due to missing dependencies."));
         }
 
-        for (dispatch_ptr, fn_prototype) in self.info.dispatch_table.iter_mut() {
-            if dispatch_ptr.is_null() {
-                let fn_ptr = runtime_dispatch_table
-                    .get_fn(fn_prototype.name())
-                    .unwrap_or_else(|| panic!("Function '{}' is expected to exist.", fn_prototype))
-                    .fn_ptr;
-                *dispatch_ptr = fn_ptr;
-            }
-        }
+        Ok(dispatch_table)
     }
 
     /// Swaps the assembly's shared library and its information for the library at `library_path`.
@@ -150,8 +123,7 @@ impl Assembly {
         library_path: &Path,
         runtime_dispatch_table: &mut DispatchTable,
     ) -> Result<(), anyhow::Error> {
-        let mut new_assembly =
-            Assembly::load(library_path, self.allocator.clone(), runtime_dispatch_table)?;
+        let mut new_assembly = Assembly::load(library_path, self.allocator.clone())?;
 
         let old_types: Vec<UnsafeTypeInfo> = self
             .info

crates/mun_runtime/src/lib.rs

@@ -13,14 +13,13 @@ mod adt;
 mod marshal;
 mod reflection;
 
-use anyhow::Error;
 use garbage_collector::GarbageCollector;
 use memory::gc::{self, GcRuntime};
 use notify::{DebouncedEvent, RecommendedWatcher, RecursiveMode, Watcher};
 use rustc_hash::FxHashMap;
 use std::{
     cell::RefCell,
-    collections::HashMap,
+    collections::{HashMap, VecDeque},
     ffi, io, mem,
     path::{Path, PathBuf},
     ptr::NonNull,
@@ -86,7 +85,7 @@ impl RuntimeBuilder {
     }
 
     /// Spawns a [`Runtime`] with the builder's options.
-    pub fn spawn(self) -> Result<Rc<RefCell<Runtime>>, Error> {
+    pub fn spawn(self) -> anyhow::Result<Rc<RefCell<Runtime>>> {
         Runtime::new(self.options).map(|runtime| Rc::new(RefCell::new(runtime)))
     }
 }
@@ -96,7 +95,7 @@ type Dependency<T> = (T, DependencyCounter);
 type DependencyMap<T> = FxHashMap<String, Dependency<T>>;
 
 /// A runtime dispatch table that maps full paths to function and struct information.
-#[derive(Default)]
+#[derive(Clone, Default)]
 pub struct DispatchTable {
     functions: FxHashMap<String, abi::FunctionDefinition>,
     fn_dependencies: FxHashMap<String, DependencyMap<abi::FunctionPrototype>>,
@@ -161,6 +160,14 @@ impl DispatchTable {
 }
 
 /// A runtime for the Mun language.
+///
+/// # Logging
+///
+/// The runtime uses [log] as a logging facade, but does not install a logger. To produce log
+/// output, you have to use a [logger implementation][log-impl] compatible with the facade.
+///
+/// [log]: https://docs.rs/log
+/// [log-impl]: https://docs.rs/log/0.4.13/log/#available-logging-implementations
 pub struct Runtime {
     assemblies: HashMap<PathBuf, Assembly>,
     dispatch_table: DispatchTable,
@@ -202,7 +209,7 @@ impl Runtime {
     /// Constructs a new `Runtime` that loads the library at `library_path` and its
     /// dependencies. The `Runtime` contains a file watcher that is triggered with an interval
     /// of `dur`.
-    pub fn new(mut options: RuntimeOptions) -> Result<Runtime, Error> {
+    pub fn new(mut options: RuntimeOptions) -> anyhow::Result<Runtime> {
         let (tx, rx) = channel();
 
         let mut dispatch_table = DispatchTable::default();
@@ -234,7 +241,7 @@ impl Runtime {
     }
 
     /// Adds an assembly corresponding to the library at `library_path`.
-    fn add_assembly(&mut self, library_path: &Path) -> Result<(), Error> {
+    fn add_assembly(&mut self, library_path: &Path) -> anyhow::Result<()> {
         let library_path = library_path.canonicalize()?;
         if self.assemblies.contains_key(&library_path) {
             return Err(io::Error::new(
@@ -244,21 +251,48 @@ impl Runtime {
             .into());
         }
 
-        let mut assembly = Assembly::load(&library_path, self.gc.clone(), &self.dispatch_table)?;
-        for dependency in assembly.info().dependencies() {
-            self.add_assembly(Path::new(dependency))?;
+        let mut loaded = HashMap::new();
+        let mut to_load = VecDeque::new();
+        to_load.push_back(library_path);
+
+        // Load all assemblies and their dependencies
+        while let Some(library_path) = to_load.pop_front() {
+            let assembly = Assembly::load(&library_path, self.gc.clone())?;
+
+            let parent = library_path.parent().expect("Invalid library path");
+            let extension = library_path.extension();
+
+            let dependencies: Vec<String> =
+                assembly.info().dependencies().map(From::from).collect();
+            loaded.insert(library_path.clone(), assembly);
+
+            for dependency in dependencies {
+                let mut library_path = PathBuf::from(parent.join(dependency));
+                if let Some(extension) = extension {
+                    library_path = library_path.with_extension(extension);
+                }
+
+                if !loaded.contains_key(&library_path) {
+                    to_load.push_back(library_path);
+                }
+            }
         }
-        assembly.link(&mut self.dispatch_table);
 
-        self.watcher
-            .watch(library_path.parent().unwrap(), RecursiveMode::NonRecursive)?;
+        self.dispatch_table = Assembly::link_all(loaded.values_mut(), &self.dispatch_table)?;
+
+        for (library_path, assembly) in loaded.into_iter() {
+            self.watcher
+                .watch(library_path.parent().unwrap(), RecursiveMode::NonRecursive)?;
+
+            self.assemblies.insert(library_path, assembly);
+        }
 
-        self.assemblies.insert(library_path, assembly);
         Ok(())
     }
 
     /// Retrieves the function definition corresponding to `function_name`, if available.
     pub fn get_function_definition(&self, function_name: &str) -> Option<&abi::FunctionDefinition> {
+        // TODO: Verify that when someone tries to invoke a non-public function, it should fail.
         self.dispatch_table.get_fn(function_name)
     }
 

crates/mun_runtime/tests/runtime.rs

@@ -5,7 +5,6 @@ use std::io;
 mod util;
 
 #[test]
-#[ignore]
 fn multiple_modules() {
     let driver = CompileAndRunTestDriver::from_fixture(
         r#"
@@ -27,6 +26,30 @@ fn multiple_modules() {
     assert_invoke_eq!(i32, 5, driver, "main");
 }
 
+#[test]
+fn cyclic_modules() {
+    let driver = CompileAndRunTestDriver::from_fixture(
+        r#"
+    //- /mun.toml
+    [package]
+    name="foo"
+    version="0.0.0"
+
+    //- /src/mod.mun
+    pub fn main() -> i32 { foo::foo() }
+
+    fn bar() -> i32 { 5 }
+
+    //- /src/foo.mun
+    pub fn foo() -> i32 { super::bar() }
+    "#,
+        |builder| builder,
+    )
+    .expect("Failed to build test driver");
+
+    assert_invoke_eq!(i32, 5, driver, "main");
+}
+
 #[test]
 fn from_fixture() {
     let driver = CompileAndRunTestDriver::from_fixture(
@@ -61,7 +84,7 @@ fn error_assembly_not_linkable() {
             "{}",
             io::Error::new(
                 io::ErrorKind::NotFound,
-                format!("Failed to link: function `dependency` is missing."),
+                format!("Failed to link due to missing dependencies."),
             )
         )
     );