Improve consistency and add misc clean ups

lib/api-tests/tests/high_level_api.rs

@@ -80,27 +80,27 @@ fn module_exports_are_ordered() {
         vec![
             export::ExportDescriptor {
                 name: "test-table",
-                kind: export::ExportKind::Table,
+                ty: export::ExportType::Table,
             },
             export::ExportDescriptor {
                 name: "test-global",
-                kind: export::ExportKind::Global,
+                ty: export::ExportType::Global,
             },
             export::ExportDescriptor {
                 name: "ret_2",
-                kind: export::ExportKind::Function,
+                ty: export::ExportType::Function,
             },
             export::ExportDescriptor {
                 name: "ret_4",
-                kind: export::ExportKind::Function,
+                ty: export::ExportType::Function,
             },
             export::ExportDescriptor {
                 name: "set_test_global",
-                kind: export::ExportKind::Function,
+                ty: export::ExportType::Function,
             },
             export::ExportDescriptor {
                 name: "update_outside_global",
-                kind: export::ExportKind::Function,
+                ty: export::ExportType::Function,
             },
         ]
     );

lib/api/src/lib.rs

@@ -22,15 +22,12 @@
 //!
 //! more info, what to do if you run into problems
 
-/// Commonly used types and functions.
+pub use crate::module::*;
-pub mod prelude {
+pub use wasmer_runtime_core::instance::{DynFunc, Instance};
-    pub use crate::module::*;
+pub use wasmer_runtime_core::memory::Memory;
-    pub use wasmer_runtime_core::instance::{DynFunc, Instance};
+pub use wasmer_runtime_core::table::Table;
-    pub use wasmer_runtime_core::memory::Memory;
+pub use wasmer_runtime_core::Func;
-    pub use wasmer_runtime_core::table::Table;
+pub use wasmer_runtime_core::{func, imports};
-    pub use wasmer_runtime_core::Func;
-    pub use wasmer_runtime_core::{func, imports};
-}
 
 pub mod module {
     //! Types and functions for WebAssembly modules.
@@ -51,7 +48,7 @@ pub mod module {
     // TODO: verify that this is the type we want to export, with extra methods on it
     pub use wasmer_runtime_core::module::Module;
     // should this be in here?
-    pub use wasmer_runtime_core::module::{ExportDescriptor, ExportKind, Import, ImportType};
+    pub use wasmer_runtime_core::module::{ExportDescriptor, ExportType, Import, ImportDescriptor};
     // TODO: implement abstract module API
 }
 
@@ -69,7 +66,7 @@ pub mod wasm {
     //!
     //! # Tables
     pub use wasmer_runtime_core::global::Global;
-    pub use wasmer_runtime_core::module::{ExportDescriptor, ExportKind, Import, ImportType};
+    pub use wasmer_runtime_core::module::{ExportDescriptor, ExportType, Import, ImportDescriptor};
     pub use wasmer_runtime_core::table::Table;
     pub use wasmer_runtime_core::types::{
         FuncSig, GlobalDescriptor, MemoryDescriptor, TableDescriptor, Type, Value,
@@ -78,13 +75,13 @@ pub mod wasm {
 
 pub mod import {
     //! Types and functions for Wasm imports.
-    pub use wasmer_runtime_core::module::{Import, ImportType};
+    pub use wasmer_runtime_core::module::{Import, ImportDescriptor};
     pub use wasmer_runtime_core::{func, imports};
 }
 
 pub mod export {
     //! Types and functions for Wasm exports.
-    pub use wasmer_runtime_core::module::{ExportDescriptor, ExportKind};
+    pub use wasmer_runtime_core::module::{ExportDescriptor, ExportType};
 }
 
 pub mod units {
@@ -100,16 +97,49 @@ pub mod types {
 pub mod error {
     //! Various error types returned by Wasmer APIs.
     pub use wasmer_runtime_core::error::{CompileError, CompileResult};
-}
 
-pub use prelude::*;
+    #[derive(Debug)]
+    pub enum CompileFromFileError {
+        CompileError(CompileError),
+        IoError(std::io::Error),
+    }
+
+    impl std::fmt::Display for CompileFromFileError {
+        fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+            match self {
+                CompileFromFileError::CompileError(ce) => write!(f, "{}", ce),
+                CompileFromFileError::IoError(ie) => write!(f, "{}", ie),
+            }
+        }
+    }
+
+    impl std::error::Error for CompileFromFileError {
+        fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+            match self {
+                CompileFromFileError::CompileError(ce) => Some(ce),
+                CompileFromFileError::IoError(ie) => Some(ie),
+            }
+        }
+    }
+
+    impl From<CompileError> for CompileFromFileError {
+        fn from(other: CompileError) -> Self {
+            CompileFromFileError::CompileError(other)
+        }
+    }
+    impl From<std::io::Error> for CompileFromFileError {
+        fn from(other: std::io::Error) -> Self {
+            CompileFromFileError::IoError(other)
+        }
+    }
+}
 
 /// Idea for generic trait; consider rename; it will need to be moved somewhere else
 pub trait CompiledModule {
     fn new(bytes: impl AsRef<[u8]>) -> error::CompileResult<Module>;
     fn from_binary(bytes: impl AsRef<[u8]>) -> error::CompileResult<Module>;
     fn from_binary_unchecked(bytes: impl AsRef<[u8]>) -> error::CompileResult<Module>;
-    fn from_file(file: impl AsRef<std::path::Path>) -> error::CompileResult<Module>;
+    fn from_file(file: impl AsRef<std::path::Path>) -> Result<Module, error::CompileFromFileError>;
 
     fn validate(bytes: impl AsRef<[u8]>) -> error::CompileResult<()>;
 }
@@ -156,30 +186,31 @@ impl CompiledModule for Module {
         wasmer_runtime_core::compile_with(bytes, &default_compiler())
     }
 
-    fn from_binary(_bytes: impl AsRef<[u8]>) -> error::CompileResult<Module> {
+    fn from_binary(bytes: impl AsRef<[u8]>) -> error::CompileResult<Module> {
-        todo!("from_binary: how is this different from `new`?")
+        let bytes = bytes.as_ref();
+        wasmer_runtime_core::compile_with(bytes, &default_compiler())
     }
-    fn from_binary_unchecked(_bytes: impl AsRef<[u8]>) -> error::CompileResult<Module> {
+
-        todo!("from_binary_unchecked")
+    fn from_binary_unchecked(bytes: impl AsRef<[u8]>) -> error::CompileResult<Module> {
+        // TODO: optimize this
+        Self::from_binary(bytes)
     }
-    fn from_file(_file: impl AsRef<std::path::Path>) -> error::CompileResult<Module> {
+
-        todo!("from_file");
+    fn from_file(file: impl AsRef<std::path::Path>) -> Result<Module, error::CompileFromFileError> {
-        /*
         use std::fs;
         use std::io::Read;
         let path = file.as_ref();
-        let mut f =
+        let mut f = fs::File::open(path)?;
-            fs::File::open(path).map_err(|_| todo!("Current error enum can't handle this case"))?;
         // TODO: ideally we can support a streaming compilation API and not have to read in the entire file
         let mut bytes = vec![];
-        f.read_to_end(&mut bytes)
+        f.read_to_end(&mut bytes)?;
-            .map_err(|_| todo!("Current error enum can't handle this case"))?;
 
-        Module::from_binary(bytes.as_slice())
+        Ok(Module::from_binary(bytes.as_slice())?)
-        */
     }
 
-    fn validate(_bytes: impl AsRef<[u8]>) -> error::CompileResult<()> {
+    fn validate(bytes: impl AsRef<[u8]>) -> error::CompileResult<()> {
-        todo!("validate")
+        // TODO: optimize this
+        let _ = Self::from_binary(bytes)?;
+        Ok(())
     }
 }

lib/runtime-core/src/instance.rs

@@ -56,7 +56,7 @@ pub struct Instance {
     /// Reference to the module used to instantiate this instance.
     pub module: Arc<ModuleInner>,
     inner: Pin<Box<InstanceInner>>,
-    /// The exports of this instance. TODO: document this
+    /// The exports of this instance.
     pub exports: Exports,
     #[allow(dead_code)]
     import_object: ImportObject,
@@ -894,7 +894,8 @@ impl<'a, Args: WasmTypeList, Rets: WasmTypeList> Exportable<'a> for Func<'a, Arg
 pub struct Exports {
     // We want to avoid the borrow checker here.
     // This is safe because
-    // 1. `Exports` can't be constructed or copied (so can't safely outlive `Instance`)
+    // 1. `Exports` can't be constructed, its fields inspected (directly or via methods),
+    //    or copied outside of this module/in Instance, so it can't safely outlive `Instance`.
     // 2. `InstanceInner` is `Pin<Box<>>`, thus we know that it will not move.
     instance_inner: *const InstanceInner,
     module: Arc<ModuleInner>,
@@ -931,7 +932,7 @@ impl Exports {
         T::get_self(self, name)
     }
 
-    /// This method must remain private.
+    /// This method must remain private for `Exports` to be sound.
     fn get_inner(&self) -> (&InstanceInner, &ModuleInner) {
         let inst_inner = unsafe { &*self.instance_inner };
         let module = self.module.borrow();

lib/runtime-core/src/module.rs

@@ -176,18 +176,18 @@ impl Module {
     /// ```
     /// # use wasmer_runtime_core::module::*;
     /// # fn example(module: &Module) {
-    /// // We can filter by `ExportKind` to get only certain types of exports.
+    /// // We can filter by `ExportType` to get only certain types of exports.
     /// // For example, here we get all the names of the functions exported by this module.
     /// let function_names =
     ///     module.exports()
-    ///           .filter(|ed| ed.kind == ExportKind::Function)
+    ///           .filter(|ed| ed.kind == ExportType::Function)
     ///           .map(|ed| ed.name.to_string())
     ///           .collect::<Vec<String>>();
     ///
     /// // And here we count the number of global variables exported by this module.
     /// let num_globals =
     ///     module.exports()
-    ///           .filter(|ed| ed.kind == ExportKind::Global)
+    ///           .filter(|ed| ed.kind == ExportType::Global)
     ///           .count();
     /// # }
     /// ```
@@ -198,7 +198,7 @@ impl Module {
             .iter()
             .map(|(name, &ei)| ExportDescriptor {
                 name,
-                kind: ei.into(),
+                ty: ei.into(),
             })
     }
 
@@ -236,7 +236,7 @@ impl Module {
             Import {
                 namespace,
                 name,
-                ty: sig.into(),
+                descriptor: sig.into(),
             }
         });
         let imported_memories =
@@ -247,7 +247,7 @@ impl Module {
                     Import {
                         namespace,
                         name,
-                        ty: memory_descriptor.into(),
+                        descriptor: memory_descriptor.into(),
                     }
                 });
         let imported_tables =
@@ -258,7 +258,7 @@ impl Module {
                     Import {
                         namespace,
                         name,
-                        ty: table_descriptor.into(),
+                        descriptor: table_descriptor.into(),
                     }
                 });
         let imported_globals =
@@ -269,7 +269,7 @@ impl Module {
                     Import {
                         namespace,
                         name,
-                        ty: global_descriptor.into(),
+                        descriptor: global_descriptor.into(),
                     }
                 });
 
@@ -287,20 +287,18 @@ impl Module {
     }
 }
 
-// TODO: review this vs `ExportIndex`
 /// Type describing an export that the [`Module`] provides.
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct ExportDescriptor<'a> {
     /// The name identifying the export.
     pub name: &'a str,
     /// The type of the export.
-    pub kind: ExportKind,
+    pub ty: ExportType,
 }
 
-// TODO: kind vs type
 /// Tag indicating the type of the export.
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
-pub enum ExportKind {
+pub enum ExportType {
     /// The export is a function.
     Function,
     /// The export is a global variable.
@@ -311,7 +309,7 @@ pub enum ExportKind {
     Table,
 }
 
-impl From<ExportIndex> for ExportKind {
+impl From<ExportIndex> for ExportType {
     fn from(other: ExportIndex) -> Self {
         match other {
             ExportIndex::Func(_) => Self::Function,
@@ -330,12 +328,9 @@ impl Clone for Module {
     }
 }
 
-/// The type of import. This indicates whether the import is a function, global, memory, or table.
+/// Information about an import such as its type and metadata about the import.
-// TODO: discuss and research Kind vs Type;
-// `Kind` has meaning to me from Haskell as an incomplete type like
-// `List` which is of kind `* -> *`.
 #[derive(Debug, Clone, PartialEq, Eq)]
-pub enum ImportType {
+pub enum ImportDescriptor {
     /// The import is a function.
     Function(FuncDescriptor),
     /// The import is a global variable.
@@ -346,45 +341,45 @@ pub enum ImportType {
     Table(TableDescriptor),
 }
 
-impl From<FuncDescriptor> for ImportType {
+impl From<FuncDescriptor> for ImportDescriptor {
     fn from(other: FuncDescriptor) -> Self {
-        ImportType::Function(other)
+        ImportDescriptor::Function(other)
     }
 }
-impl From<&FuncDescriptor> for ImportType {
+impl From<&FuncDescriptor> for ImportDescriptor {
     fn from(other: &FuncDescriptor) -> Self {
-        ImportType::Function(other.clone())
+        ImportDescriptor::Function(other.clone())
     }
 }
-impl From<MemoryDescriptor> for ImportType {
+impl From<MemoryDescriptor> for ImportDescriptor {
     fn from(other: MemoryDescriptor) -> Self {
-        ImportType::Memory(other)
+        ImportDescriptor::Memory(other)
     }
 }
-impl From<&MemoryDescriptor> for ImportType {
+impl From<&MemoryDescriptor> for ImportDescriptor {
     fn from(other: &MemoryDescriptor) -> Self {
-        ImportType::Memory(*other)
+        ImportDescriptor::Memory(*other)
     }
 }
 
-impl From<TableDescriptor> for ImportType {
+impl From<TableDescriptor> for ImportDescriptor {
     fn from(other: TableDescriptor) -> Self {
-        ImportType::Table(other)
+        ImportDescriptor::Table(other)
     }
 }
-impl From<&TableDescriptor> for ImportType {
+impl From<&TableDescriptor> for ImportDescriptor {
     fn from(other: &TableDescriptor) -> Self {
-        ImportType::Table(*other)
+        ImportDescriptor::Table(*other)
     }
 }
-impl From<GlobalDescriptor> for ImportType {
+impl From<GlobalDescriptor> for ImportDescriptor {
     fn from(other: GlobalDescriptor) -> Self {
-        ImportType::Global(other)
+        ImportDescriptor::Global(other)
     }
 }
-impl From<&GlobalDescriptor> for ImportType {
+impl From<&GlobalDescriptor> for ImportDescriptor {
     fn from(other: &GlobalDescriptor) -> Self {
-        ImportType::Global(*other)
+        ImportDescriptor::Global(*other)
     }
 }
 
@@ -396,7 +391,7 @@ pub struct Import {
     /// The name of the import.
     pub name: String,
     /// The type of the import.
-    pub ty: ImportType,
+    pub descriptor: ImportDescriptor,
 }
 
 impl ModuleInner {}