refactor: improve comments, fix lint

Signed-off-by: Victor Adossi <[email protected]>

src/module/functions/mod.rs

@@ -21,7 +21,7 @@ use crate::parse::IndicesToIds;
 use crate::tombstone_arena::{Id, Tombstone, TombstoneArena};
 use crate::ty::TypeId;
 use crate::ty::ValType;
-use crate::{ExportItem, FunctionBuilder, Memory, MemoryId};
+use crate::{ExportItem, Memory, MemoryId};
 
 pub use self::local_function::LocalFunction;
 
@@ -450,8 +450,8 @@ impl Module {
     /// For example, if you wanted to replace an exported function with a no-op,
     ///
     /// ```ignore
-    /// // Since `FunctionBuilder` requires a mutable pointer to the module's types
-    /// // We must build it *outside* the closure and `move` it in
+    /// // Since `FunctionBuilder` requires a mutable pointer to the module's types,
+    /// // we must build it *outside* the closure and `move` it in
     /// let mut builder = FunctionBuilder::new(&mut module.types, &[], &[]);
     ///
     /// module.replace_exported_func(fid, move || {
@@ -478,11 +478,12 @@ impl Module {
                 let ty = self.types.get(lf.ty());
                 let (params, results) = (ty.params().to_vec(), ty.results().to_vec());
 
-                // Run the builder function to produce a new local function, or create a stub that is unreachable
-                let new_local_fn =
-                    fn_builder((&params, &results)).context("export fn builder failed")?;
+                // Add the function produced by `fn_builder` as a local function,
+                let new_fid = self.funcs.add_local(
+                    fn_builder((&params, &results)).context("export fn builder failed")?,
+                );
 
-                let new_fid = self.funcs.add_local(new_local_fn);
+                // Mutate the existing export to use the new local function
                 let export = self.exports.get_mut(exported_fn.id());
                 export.item = ExportItem::Function(new_fid);
 
@@ -496,8 +497,8 @@ impl Module {
     /// Replace a single imported function with the result of the provided builder function.
     ///
     /// ```ignore
-    /// // Since `FunctionBuilder` requires a mutable pointer to the module's types
-    /// // We must build it *outside* the closure and `move` it in
+    /// // Since `FunctionBuilder` requires a mutable pointer to the module's types,
+    /// // we must build it *outside* the closure and `move` it in
     /// let mut builder = FunctionBuilder::new(&mut module.types, &[], &[]);
     ///
     /// module.replace_imported_func(fid, move || {
@@ -525,12 +526,12 @@ impl Module {
                 let ty = self.types.get(*tid);
                 let (params, results) = (ty.params().to_vec(), ty.results().to_vec());
 
-                // Run the builder function to produce a new local function, or create a stub that is unreachable
-                let new_local_fn =
-                    fn_builder((&params, &results)).context("import fn builder failed")?;
-
+                // Mutate the existing function, changing it from a FunctionKind::ImportedFunction
+                // to the local function produced by running the provided `fn_builder`
                 let func = self.funcs.get_mut(fid);
-                func.kind = FunctionKind::Local(new_local_fn);
+                func.kind = FunctionKind::Local(
+                    fn_builder((&params, &results)).context("import fn builder failed")?,
+                );
 
                 self.imports.delete(original_imported_fn.id());
 
@@ -661,7 +662,7 @@ impl Emit for ModuleFunctions {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::{Export, Import, ImportKind, Module};
+    use crate::{Export, Module, FunctionBuilder};
 
     #[test]
     fn get_memory_id() {
@@ -682,8 +683,7 @@ mod tests {
         let original_fn_id: FunctionId = builder.finish(vec![], &mut module.funcs);
         let original_export_id = module.exports.add("dummy", original_fn_id);
 
-        // Since FunctionBuilder requires a mutable pointer to the module's types
-        // We must build it *outside* the builder closure before using it.
+        // Create builder to use inside closure
         let mut builder = FunctionBuilder::new(&mut module.types, &[], &[]);
 
         // Replace the existing function with a new one with a reversed const value
@@ -728,8 +728,7 @@ mod tests {
         let original_fn_id: FunctionId = builder.finish(vec![], &mut module.funcs);
         let original_export_id = module.exports.add("dummy", original_fn_id);
 
-        // Since FunctionBuilder requires a mutable pointer to the module's types
-        // We must build it *outside* the builder closure before using it.
+        // Create builder to use inside closure
         let mut builder = FunctionBuilder::new(&mut module.types, &[], &[]);
 
         // Replace the existing function with a new one with a reversed const value
@@ -774,8 +773,7 @@ mod tests {
         let types = module.types.add(&[], &[]);
         let (original_fn_id, original_import_id) = module.add_import_func("mod", "dummy", types);
 
-        // Since FunctionBuilder requires a mutable pointer to the module's types
-        // We must build it *outside* the builder closure before using it.
+        // Create builder to use inside closure
         let mut builder = FunctionBuilder::new(&mut module.types, &[], &[]);
 
         // Replace the existing function with a new one with a reversed const value
@@ -817,8 +815,7 @@ mod tests {
         let types = module.types.add(&[], &[]);
         let (original_fn_id, original_import_id) = module.add_import_func("mod", "dummy", types);
 
-        // Since FunctionBuilder requires a mutable pointer to the module's types
-        // We must build it *outside* the builder closure before using it.
+        // Create builder to use inside closure
         let mut builder = FunctionBuilder::new(&mut module.types, &[], &[]);
 
         // Replace the existing function with a new one with a reversed const value