Add misc. fixes and updates from feedback

lib/api-tests/tests/high_level_api.rs

@@ -19,17 +19,16 @@ static TEST_WAT: &str = r#"
 "#;
 
 fn append_custom_section(
-    wasm: &[u8],
+    wasm: &mut Vec<u8>,
     custom_section_name: &str,
     custom_section_contents: &[u8],
-) -> Vec<u8> {
+) {
-    let mut out = Vec::with_capacity(
+    wasm.reserve(
         // 1 for custom section id, 5 for max length of custom section, 5 for max length of name
-        wasm.len() + custom_section_name.len() + custom_section_contents.len() + 1 + 5 + 5,
+        custom_section_name.len() + custom_section_contents.len() + 1 + 5 + 5,
     );
 
-    out.extend(wasm);
+    wasm.push(0);
-    out.push(0);
 
     let name_length = custom_section_name.as_bytes().len() as u32;
     let mut name_length_as_leb128 = vec![];
@@ -42,82 +41,117 @@ fn append_custom_section(
     let mut custom_section_length_as_leb128 = vec![];
     write_u32_leb128(&mut custom_section_length_as_leb128, custom_section_length);
 
-    out.extend(&custom_section_length_as_leb128);
+    wasm.extend(&custom_section_length_as_leb128);
-    out.extend(&name_length_as_leb128);
+    wasm.extend(&name_length_as_leb128);
-    out.extend(custom_section_name.as_bytes());
+    wasm.extend(custom_section_name.as_bytes());
-    out.extend(custom_section_contents);
+    wasm.extend(custom_section_contents);
-
-    out
 }
 
 #[test]
-fn it_works() {
+fn custom_section_parsing_works() {
-    use wasmer::wasm::{Global, Value};
+    use wasmer::{CompiledModule, Module};
-    use wasmer::{export, func, imports, CompiledModule, Func, Module, Table};
+    let wasm = {
-    let wasm_no_custom_section = wabt::wat2wasm(TEST_WAT).unwrap();
+        let mut wasm = wabt::wat2wasm(TEST_WAT).unwrap();
-    let wasm = append_custom_section(
+        append_custom_section(&mut wasm, "test_custom_section", b"hello, world!");
-        &wasm_no_custom_section,
+        append_custom_section(&mut wasm, "test_custom_section", b"goodbye, world!");
-        "test_custom_section",
+        append_custom_section(&mut wasm, "different_name", b"different value");
-        b"hello, world!",
+        wasm
-    );
+    };
-    let wasm = append_custom_section(&wasm, "test_custom_section", b"goodbye, world!");
+
-    // TODO: review error messages when `CompiledModule` is not in scope. My hypothesis is that they'll be
-    // misleading, if so we may want to do something about it.
     let module = Module::new(wasm).unwrap();
-    let imports = module.imports();
-    assert_eq!(imports.len(), 2);
-    let num_exports = module.exports().count();
-    assert_eq!(num_exports, 6);
-    let (
-        mut test_table_seen,
-        mut test_global_seen,
-        mut test_ret_2_seen,
-        mut test_ret_4_seen,
-        mut set_test_global_seen,
-        mut update_outside_global_seen,
-    ) = (false, false, false, false, false, false);
-    for export in module.exports() {
-        match (export.name.as_ref(), export.kind) {
-            ("test-table", export::ExportKind::Table) => {
-                assert!(!test_table_seen);
-                test_table_seen = true;
-            }
-            ("test-global", export::ExportKind::Global) => {
-                assert!(!test_global_seen);
-                test_global_seen = true;
-            }
-            ("ret_2", export::ExportKind::Function) => {
-                assert!(!test_ret_2_seen);
-                test_ret_2_seen = true;
-            }
-            ("ret_4", export::ExportKind::Function) => {
-                assert!(!test_ret_4_seen);
-                test_ret_4_seen = true;
-            }
-            ("set_test_global", export::ExportKind::Function) => {
-                assert!(!set_test_global_seen);
-                set_test_global_seen = true;
-            }
-            ("update_outside_global", export::ExportKind::Function) => {
-                assert!(!update_outside_global_seen);
-                update_outside_global_seen = true;
-            }
-            _ => {
-                assert!(false, "Unknown export found!");
-            }
-        }
-    }
-    assert!(test_table_seen);
-    assert!(test_global_seen);
-    assert!(test_ret_2_seen);
-    assert!(test_ret_4_seen);
-    assert!(set_test_global_seen);
-    assert!(update_outside_global_seen);
 
     assert_eq!(
         module.custom_sections("test_custom_section"),
         Some(&[b"hello, world!".to_vec(), b"goodbye, world!".to_vec()][..])
     );
+}
+
+#[test]
+fn module_exports_are_ordered() {
+    use wasmer::{export, CompiledModule, Module};
+
+    let wasm = wabt::wat2wasm(TEST_WAT).unwrap();
+    // TODO: review error messages when `CompiledModule` is not in scope. My hypothesis is that they'll be
+    // misleading, if so we may want to do something about it.
+    let module = Module::new(wasm).unwrap();
+    let exports = module.exports().collect::<Vec<_>>();
+    assert_eq!(
+        exports,
+        vec![
+            export::ExportDescriptor {
+                name: "test-table",
+                kind: export::ExportKind::Table,
+            },
+            export::ExportDescriptor {
+                name: "test-global",
+                kind: export::ExportKind::Global,
+            },
+            export::ExportDescriptor {
+                name: "ret_2",
+                kind: export::ExportKind::Function,
+            },
+            export::ExportDescriptor {
+                name: "ret_4",
+                kind: export::ExportKind::Function,
+            },
+            export::ExportDescriptor {
+                name: "set_test_global",
+                kind: export::ExportKind::Function,
+            },
+            export::ExportDescriptor {
+                name: "update_outside_global",
+                kind: export::ExportKind::Function,
+            },
+        ]
+    );
+}
+
+#[test]
+fn module_imports_are_correct() {
+    use wasmer::{CompiledModule, Module};
+
+    let wasm = wabt::wat2wasm(TEST_WAT).unwrap();
+    // TODO: review error messages when `CompiledModule` is not in scope. My hypothesis is that they'll be
+    // misleading, if so we may want to do something about it.
+    let module = Module::new(wasm).unwrap();
+
+    // TODO: test this more later
+    let imports = module.imports();
+    assert_eq!(imports.len(), 2);
+}
+
+#[test]
+fn module_can_be_instantiated() {
+    use wasmer::wasm::{Global, Value};
+    use wasmer::{func, imports, CompiledModule, Module};
+
+    let wasm = wabt::wat2wasm(TEST_WAT).unwrap();
+    // TODO: review error messages when `CompiledModule` is not in scope. My hypothesis is that they'll be
+    // misleading, if so we may want to do something about it.
+    let module = Module::new(wasm).unwrap();
+
+    let outside_global = Global::new_mutable(Value::I32(15));
+    let import_object = imports! {
+        "env" => {
+            "update-func" => func!(|x: i32| x * 2),
+            "outside-global" => outside_global.clone(),
+        }
+    };
+    let instance = module.instantiate(&import_object);
+
+    assert!(instance.is_ok());
+}
+
+#[test]
+fn exports_work() {
+    use wasmer::wasm::{Global, Value};
+    use wasmer::{func, imports, CompiledModule, Func, Module, Table};
+
+    let wasm = wabt::wat2wasm(TEST_WAT).unwrap();
+    // TODO: review error messages when `CompiledModule` is not in scope. My hypothesis is that they'll be
+    // misleading, if so we may want to do something about it.
+    let module = Module::new(wasm).unwrap();
+
     let outside_global = Global::new_mutable(Value::I32(15));
 
     let import_object = imports! {

lib/runtime-core/src/instance.rs

@@ -181,7 +181,10 @@ impl Instance {
     /// # Ok(())
     /// # }
     /// ```
-    #[deprecated(since = "0.17.0", note = "Please use `instance.exports.get()` instead")]
+    #[deprecated(
+        since = "0.17.0",
+        note = "Please use `instance.exports.get(name)` instead"
+    )]
     pub fn func<Args, Rets>(&self, name: &str) -> ResolveResult<Func<Args, Rets, Wasm>>
     where
         Args: WasmTypeList,
@@ -192,23 +195,7 @@ impl Instance {
 
     /// Resolve a function by name.
     pub fn resolve_func(&self, name: &str) -> ResolveResult<usize> {
-        let export_index =
+        resolve_func_index(&*self.module, name).map(|fi| fi.index())
-            self.module
-                .info
-                .exports
-                .get(name)
-                .ok_or_else(|| ResolveError::ExportNotFound {
-                    name: name.to_string(),
-                })?;
-
-        if let ExportIndex::Func(func_index) = export_index {
-            Ok(func_index.index())
-        } else {
-            Err(ResolveError::ExportWrongType {
-                name: name.to_string(),
-            }
-            .into())
-        }
     }
 
     /// This returns the representation of a function that can be called
@@ -225,7 +212,10 @@ impl Instance {
     /// # Ok(())
     /// # }
     /// ```
-    #[deprecated(since = "0.17.0", note = "Please use `instance.exports.get()` instead")]
+    #[deprecated(
+        since = "0.17.0",
+        note = "Please use `instance.exports.get(name)` instead"
+    )]
     pub fn dyn_func(&self, name: &str) -> ResolveResult<DynFunc> {
         self.exports.get(name)
     }
@@ -324,16 +314,8 @@ impl Instance {
     }
 }
 
-/// Private function implementing the inner logic of `Instance::func`
+/// Private function used to find the [`FuncIndex`] of a given export.
-fn func<'a, Args, Rets>(
+fn resolve_func_index(module: &ModuleInner, name: &str) -> ResolveResult<FuncIndex> {
-    module: &'a ModuleInner,
-    inst_inner: &'a InstanceInner,
-    name: &str,
-) -> ResolveResult<Func<'a, Args, Rets, Wasm>>
-where
-    Args: WasmTypeList,
-    Rets: WasmTypeList,
-{
     let export_index =
         module
             .info
@@ -344,95 +326,7 @@ where
             })?;
 
     if let ExportIndex::Func(func_index) = export_index {
-        let sig_index = *module
+        Ok(*func_index)
-            .info
-            .func_assoc
-            .get(*func_index)
-            .expect("broken invariant, incorrect func index");
-        let signature = SigRegistry.lookup_signature_ref(&module.info.signatures[sig_index]);
-
-        if signature.params() != Args::types() || signature.returns() != Rets::types() {
-            Err(ResolveError::Signature {
-                expected: (*signature).clone(),
-                found: Args::types().to_vec(),
-            })?;
-        }
-
-        let ctx = match func_index.local_or_import(&module.info) {
-            LocalOrImport::Local(_) => inst_inner.vmctx,
-            LocalOrImport::Import(imported_func_index) => unsafe {
-                inst_inner.import_backing.vm_functions[imported_func_index]
-                    .func_ctx
-                    .as_ref()
-            }
-            .vmctx
-            .as_ptr(),
-        };
-
-        let func_wasm_inner = module
-            .runnable_module
-            .get_trampoline(&module.info, sig_index)
-            .unwrap();
-
-        let (func_ptr, func_env) = match func_index.local_or_import(&module.info) {
-            LocalOrImport::Local(local_func_index) => (
-                module
-                    .runnable_module
-                    .get_func(&module.info, local_func_index)
-                    .unwrap(),
-                None,
-            ),
-            LocalOrImport::Import(import_func_index) => {
-                let imported_func = &inst_inner.import_backing.vm_functions[import_func_index];
-
-                (
-                    NonNull::new(imported_func.func as *mut _).unwrap(),
-                    unsafe { imported_func.func_ctx.as_ref() }.func_env,
-                )
-            }
-        };
-
-        let typed_func: Func<Args, Rets, Wasm> =
-            unsafe { Func::from_raw_parts(func_wasm_inner, func_ptr, func_env, ctx) };
-
-        Ok(typed_func)
-    } else {
-        Err(ResolveError::ExportWrongType {
-            name: name.to_string(),
-        }
-        .into())
-    }
-}
-
-/// Private function implementing the inner logic of `Instance::dyn_func`
-fn dyn_func<'a>(
-    module: &'a ModuleInner,
-    inst_inner: &'a InstanceInner,
-    name: &str,
-) -> ResolveResult<DynFunc<'a>> {
-    let export_index =
-        module
-            .info
-            .exports
-            .get(name)
-            .ok_or_else(|| ResolveError::ExportNotFound {
-                name: name.to_string(),
-            })?;
-
-    if let ExportIndex::Func(func_index) = export_index {
-        let sig_index = *module
-            .info
-            .func_assoc
-            .get(*func_index)
-            .expect("broken invariant, incorrect func index");
-        let signature = SigRegistry.lookup_signature_ref(&module.info.signatures[sig_index]);
-
-        Ok(DynFunc {
-            signature,
-            module: &module,
-            instance_inner: &inst_inner,
-            func_index: *func_index,
-        })
     } else {
         Err(ResolveError::ExportWrongType {
             name: name.to_string(),
@@ -910,14 +804,82 @@ impl<'a> Exportable<'a> for Global {
 impl<'a> Exportable<'a> for DynFunc<'a> {
     fn get_self(exports: &'a Exports, name: &str) -> ResolveResult<Self> {
         let (inst_inner, module) = exports.get_inner();
-        dyn_func(module, inst_inner, name)
+        let func_index = resolve_func_index(module, name)?;
+
+        let sig_index = *module
+            .info
+            .func_assoc
+            .get(func_index)
+            .expect("broken invariant, incorrect func index");
+        let signature = SigRegistry.lookup_signature_ref(&module.info.signatures[sig_index]);
+
+        Ok(DynFunc {
+            signature,
+            module: &module,
+            instance_inner: &inst_inner,
+            func_index: func_index,
+        })
     }
 }
 
 impl<'a, Args: WasmTypeList, Rets: WasmTypeList> Exportable<'a> for Func<'a, Args, Rets, Wasm> {
     fn get_self(exports: &'a Exports, name: &str) -> ResolveResult<Self> {
         let (inst_inner, module) = exports.get_inner();
-        func(module, inst_inner, name)
+
+        let func_index = resolve_func_index(module, name)?;
+
+        let sig_index = *module
+            .info
+            .func_assoc
+            .get(func_index)
+            .expect("broken invariant, incorrect func index");
+        let signature = SigRegistry.lookup_signature_ref(&module.info.signatures[sig_index]);
+
+        if signature.params() != Args::types() || signature.returns() != Rets::types() {
+            Err(ResolveError::Signature {
+                expected: (*signature).clone(),
+                found: Args::types().to_vec(),
+            })?;
+        }
+
+        let ctx = match func_index.local_or_import(&module.info) {
+            LocalOrImport::Local(_) => inst_inner.vmctx,
+            LocalOrImport::Import(imported_func_index) => unsafe {
+                inst_inner.import_backing.vm_functions[imported_func_index]
+                    .func_ctx
+                    .as_ref()
+            }
+            .vmctx
+            .as_ptr(),
+        };
+
+        let func_wasm_inner = module
+            .runnable_module
+            .get_trampoline(&module.info, sig_index)
+            .unwrap();
+
+        let (func_ptr, func_env) = match func_index.local_or_import(&module.info) {
+            LocalOrImport::Local(local_func_index) => (
+                module
+                    .runnable_module
+                    .get_func(&module.info, local_func_index)
+                    .unwrap(),
+                None,
+            ),
+            LocalOrImport::Import(import_func_index) => {
+                let imported_func = &inst_inner.import_backing.vm_functions[import_func_index];
+
+                (
+                    NonNull::new(imported_func.func as *mut _).unwrap(),
+                    unsafe { imported_func.func_ctx.as_ref() }.func_env,
+                )
+            }
+        };
+
+        let typed_func: Func<Args, Rets, Wasm> =
+            unsafe { Func::from_raw_parts(func_wasm_inner, func_ptr, func_env, ctx) };
+
+        Ok(typed_func)
     }
 }
 

lib/runtime-core/src/macros.rs

@@ -137,11 +137,12 @@ macro_rules! namespace {
 
 #[cfg(test)]
 mod test {
-    #[test]
-    fn imports_macro_allows_trailing_comma_and_none() {
     fn func(arg: i32) -> i32 {
         arg + 1
     }
+
+    #[test]
+    fn imports_macro_allows_trailing_comma_and_none() {
         let _ = imports! {
             "env" => {
                 "func" => func!(func),
@@ -182,7 +183,12 @@ mod test {
                 "func2" => func!(func),
             }
         };
+    }
+
+    #[test]
+    fn imports_macro_allows_trailing_comma_and_none_with_state() {
         use std::{ffi, ptr};
+
         fn dtor(_arg: *mut ffi::c_void) {}
         fn state_creator() -> (*mut ffi::c_void, fn(*mut ffi::c_void)) {
             (ptr::null_mut() as *mut ffi::c_void, dtor)

lib/runtime-core/src/module.rs

@@ -54,6 +54,10 @@ pub struct ModuleInfo {
     pub imported_globals: Map<ImportedGlobalIndex, (ImportName, GlobalDescriptor)>,
 
     /// Map of string to export index.
+    // Implementation note: this should maintain the order that the exports appear in the
+    // Wasm module.  Be careful not to use APIs that may break the order!
+    // Side note, because this is public we can't actually guarantee that it will remain
+    // in order.
     pub exports: IndexMap<String, ExportIndex>,
 
     /// Vector of data initializers.
@@ -193,7 +197,7 @@ impl Module {
             .exports
             .iter()
             .map(|(name, &ei)| ExportDescriptor {
-                name: name.clone(),
+                name,
                 kind: ei.into(),
             })
     }
@@ -282,9 +286,9 @@ impl Module {
 // TODO: review this vs `ExportIndex`
 /// Type describing an export that the [`Module`] provides.
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
-pub struct ExportDescriptor {
+pub struct ExportDescriptor<'a> {
     /// The name identifying the export.
-    pub name: String,
+    pub name: &'a str,
     /// The type of the export.
     pub kind: ExportKind,
 }

src/bin/wasmer.rs

@@ -31,7 +31,7 @@ use wasmer_llvm_backend::{
 };
 use wasmer_runtime::{
     cache::{Cache as BaseCache, FileSystemCache, WasmHash},
-    Backend, DynFunc, Func, Value, VERSION,
+    Backend, DynFunc, Value, VERSION,
 };
 #[cfg(feature = "managed")]
 use wasmer_runtime_core::tiering::{run_tiering, InteractiveShellContext, ShellExitOperation};
@@ -437,7 +437,7 @@ fn execute_wasi(
         .instantiate(&import_object)
         .map_err(|e| format!("Can't instantiate WASI module: {:?}", e))?;
 
-    let start: Func<(), ()> = instance
+    let start: wasmer_runtime::Func<(), ()> = instance
         .exports
         .get("_start")
         .map_err(|e| format!("{:?}", e))?;