Merge pull request #1470 from bytecodealliance/pch/wiggle_error_model

Wiggle: changes to traits used for error conversion and reporting

Cargo.lock

@@ -255,17 +255,6 @@ version = "0.1.10"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "4785bdd1c96b2a846b2bd7cc02e86b6b3dbf14e7e53446c4f54c92a361040822"
 
-[[package]]
-name = "chrono"
-version = "0.4.11"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "80094f509cf8b5ae86a4966a39b3ff66cd7e2a3e594accec3743ff3fabeab5b2"
-dependencies = [
- "num-integer",
- "num-traits",
- "time",
-]
-
 [[package]]
 name = "clap"
 version = "2.33.0"
@@ -557,7 +546,7 @@ dependencies = [
  "file-per-thread-logger",
  "filecheck",
  "indicatif",
- "pretty_env_logger 0.4.0",
+ "pretty_env_logger",
  "serde",
  "target-lexicon",
  "term",
@@ -1294,17 +1283,6 @@ version = "0.2.6"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "74490b50b9fbe561ac330df47c08f3f33073d2d00c150f719147d7c54522fa1b"
 
-[[package]]
-name = "pretty_env_logger"
-version = "0.3.1"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "717ee476b1690853d222af4634056d830b5197ffd747726a9a1eee6da9f49074"
-dependencies = [
- "chrono",
- "env_logger 0.6.2",
- "log",
-]
-
 [[package]]
 name = "pretty_env_logger"
 version = "0.4.0"
@@ -1945,7 +1923,7 @@ dependencies = [
  "anyhow",
  "cfg-if",
  "os_pipe",
- "pretty_env_logger 0.4.0",
+ "pretty_env_logger",
  "target-lexicon",
  "tempfile",
  "wasi-common",
@@ -1992,17 +1970,6 @@ dependencies = [
  "lazy_static",
 ]
 
-[[package]]
-name = "time"
-version = "0.1.42"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "db8dcfca086c1143c9270ac42a2bbd8a7ee477b78ac8e45b19abfb0cbede4b6f"
-dependencies = [
- "libc",
- "redox_syscall",
- "winapi",
-]
-
 [[package]]
 name = "toml"
 version = "0.5.6"
@@ -2145,7 +2112,7 @@ dependencies = [
  "file-per-thread-logger",
  "lazy_static",
  "libc",
- "pretty_env_logger 0.4.0",
+ "pretty_env_logger",
  "rayon",
  "region",
  "rustc-demangle",
@@ -2192,7 +2159,7 @@ dependencies = [
  "filecheck",
  "libc",
  "more-asserts",
- "pretty_env_logger 0.4.0",
+ "pretty_env_logger",
  "rayon",
  "structopt",
  "target-lexicon",
@@ -2244,7 +2211,7 @@ dependencies = [
  "lightbeam",
  "log",
  "more-asserts",
- "pretty_env_logger 0.4.0",
+ "pretty_env_logger",
  "rand 0.7.3",
  "rayon",
  "serde",
@@ -2397,15 +2364,6 @@ dependencies = [
  "wast 11.0.0",
 ]
 
-[[package]]
-name = "wast"
-version = "9.0.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "ee7b16105405ca2aa2376ba522d8d4b1a11604941dd3bb7df9fd2ece60f8d16a"
-dependencies = [
- "leb128",
-]
-
 [[package]]
 name = "wast"
 version = "11.0.0"
@@ -2526,15 +2484,15 @@ dependencies = [
 
 [[package]]
 name = "witx"
-version = "0.8.4"
+version = "0.8.5"
 dependencies = [
  "anyhow",
  "diff",
  "log",
- "pretty_env_logger 0.3.1",
+ "pretty_env_logger",
  "structopt",
  "thiserror",
- "wast 9.0.0",
+ "wast 11.0.0",
 ]
 
 [[package]]

crates/wasi-common/src/wasi.rs

@@ -11,17 +11,17 @@ wiggle::from_witx!({
 pub use types::Errno;
 pub type Result<T> = std::result::Result<T, Errno>;
 
-impl<'a> wiggle::GuestErrorType<'a> for Errno {
+impl wiggle::GuestErrorType for Errno {
-    type Context = WasiCtx;
-
     fn success() -> Self {
         Self::Success
     }
+}
 
-    fn from_error(e: wiggle::GuestError, _ctx: &Self::Context) -> Self {
+impl types::GuestErrorConversion for WasiCtx {
+    fn into_errno(&self, e: wiggle::GuestError) -> Errno {
         eprintln!("Guest error: {:?}", e);
         // TODO proper error mapping
-        Self::Inval
+        Errno::Inval
     }
 }
 

crates/wasi-common/wig/Cargo.toml

@@ -17,7 +17,7 @@ proc-macro = true
 quote = "1.0.2"
 proc-macro2 = "1.0.6"
 heck = "0.3.1"
-witx = { path = "../WASI/tools/witx", version = "0.8.4" }
+witx = { path = "../WASI/tools/witx", version = "0.8.5" }
 
 [badges]
 maintenance = { status = "actively-developed" }

crates/wiggle/Cargo.toml

@@ -12,7 +12,7 @@ include = ["src/**/*", "LICENSE"]
 
 [dependencies]
 thiserror = "1"
-witx = { path = "../wasi-common/WASI/tools/witx", version = "0.8.4", optional = true }
+witx = { path = "../wasi-common/WASI/tools/witx", version = "0.8.5", optional = true }
 wiggle-macro = { path = "macro", version = "0.15.0" }
 
 [badges]

crates/wiggle/generate/Cargo.toml

@@ -14,7 +14,7 @@ include = ["src/**/*", "LICENSE"]
 [lib]
 
 [dependencies]
-witx = { version = "0.8.4", path = "../../wasi-common/WASI/tools/witx" }
+witx = { version = "0.8.5", path = "../../wasi-common/WASI/tools/witx" }
 quote = "1.0"
 proc-macro2 = "1.0"
 heck = "0.3"

crates/wiggle/generate/src/funcs.rs

@@ -62,9 +62,10 @@ pub fn define_func(
                 _ => unreachable!("err should always be passed by value"),
             };
             let err_typename = names.type_ref(&tref, anon_lifetime());
+            let err_method = names.guest_error_conversion_method(&tref);
             quote! {
                 let e = wiggle::GuestError::InFunc { funcname: #funcname, location: #location, err: Box::new(e.into()) };
-                let err: #err_typename = wiggle::GuestErrorType::from_error(e, ctx);
+                let err: #err_typename = GuestErrorConversion::#err_method(ctx, e); // XXX replace with conversion method on trait!
                 return #abi_ret::from(err);
             }
         } else {

crates/wiggle/generate/src/lib.rs

@@ -8,6 +8,8 @@ mod types;
 use proc_macro2::TokenStream;
 use quote::quote;
 
+use lifetimes::anon_lifetime;
+
 pub use config::Config;
 pub use funcs::define_func;
 pub use module_trait::define_module_trait;
@@ -21,6 +23,17 @@ pub fn generate(doc: &witx::Document, config: &Config) -> TokenStream {
 
     let types = doc.typenames().map(|t| define_datatype(&names, &t));
 
+    let guest_error_methods = doc.error_types().map(|t| {
+        let typename = names.type_ref(&t, anon_lifetime());
+        let err_method = names.guest_error_conversion_method(&t);
+        quote!(fn #err_method(&self, e: wiggle::GuestError) -> #typename;)
+    });
+    let guest_error_conversion = quote! {
+        pub trait GuestErrorConversion {
+            #(#guest_error_methods)*
+        }
+    };
+
     let modules = doc.modules().map(|module| {
         let modname = names.module(&module.name);
         let trait_name = names.trait_name(&module.name);
@@ -57,6 +70,7 @@ pub fn generate(doc: &witx::Document, config: &Config) -> TokenStream {
     quote!(
         pub mod types {
             #(#types)*
+            #guest_error_conversion
         }
         #(#modules)*
         #metadata

crates/wiggle/generate/src/names.rs

@@ -1,7 +1,7 @@
 use heck::{CamelCase, ShoutySnakeCase, SnakeCase};
 use proc_macro2::{Ident, TokenStream};
 use quote::{format_ident, quote};
-use witx::{AtomType, BuiltinType, Id, TypeRef};
+use witx::{AtomType, BuiltinType, Id, Type, TypeRef};
 
 use crate::lifetimes::LifetimeExt;
 
@@ -59,12 +59,12 @@ impl Names {
                 }
             }
             TypeRef::Value(ty) => match &**ty {
-                witx::Type::Builtin(builtin) => self.builtin_type(*builtin, lifetime.clone()),
+                Type::Builtin(builtin) => self.builtin_type(*builtin, lifetime.clone()),
-                witx::Type::Pointer(pointee) | witx::Type::ConstPointer(pointee) => {
+                Type::Pointer(pointee) | Type::ConstPointer(pointee) => {
                     let pointee_type = self.type_ref(&pointee, lifetime.clone());
                     quote!(wiggle::GuestPtr<#lifetime, #pointee_type>)
                 }
-                witx::Type::Array(pointee) => {
+                Type::Array(pointee) => {
                     let pointee_type = self.type_ref(&pointee, lifetime.clone());
                     quote!(wiggle::GuestPtr<#lifetime, [#pointee_type]>)
                 }
@@ -139,4 +139,28 @@ impl Names {
     pub fn func_len_binding(&self, id: &Id) -> Ident {
         format_ident!("{}_len", id.as_str().to_snake_case())
     }
+
+    pub fn guest_error_conversion_method(&self, tref: &TypeRef) -> Ident {
+        match tref {
+            TypeRef::Name(nt) => format_ident!("into_{}", nt.name.as_str().to_snake_case()),
+            TypeRef::Value(ty) => match &**ty {
+                Type::Builtin(b) => match b {
+                    BuiltinType::String => unreachable!("error type must be atom"),
+                    BuiltinType::U8 => format_ident!("into_u8"),
+                    BuiltinType::U16 => format_ident!("into_u16"),
+                    BuiltinType::U32 => format_ident!("into_u32"),
+                    BuiltinType::U64 => format_ident!("into_u64"),
+                    BuiltinType::S8 => format_ident!("into_i8"),
+                    BuiltinType::S16 => format_ident!("into_i16"),
+                    BuiltinType::S32 => format_ident!("into_i32"),
+                    BuiltinType::S64 => format_ident!("into_i64"),
+                    BuiltinType::F32 => format_ident!("into_f32"),
+                    BuiltinType::F64 => format_ident!("into_f64"),
+                    BuiltinType::Char8 => format_ident!("into_char8"),
+                    BuiltinType::USize => format_ident!("into_usize"),
+                },
+                _ => panic!("unexpected anonymous error type: {:?}", ty),
+            },
+        }
+    }
 }

crates/wiggle/macro/Cargo.toml

@@ -15,7 +15,7 @@ proc-macro = true
 
 [dependencies]
 wiggle-generate = { path = "../generate", version = "0.15.0" }
-witx = { path = "../../wasi-common/WASI/tools/witx", version = "0.8.4" }
+witx = { path = "../../wasi-common/WASI/tools/witx", version = "0.8.5" }
 syn = { version = "1.0", features = ["full"] }
 
 [dev-dependencies]

crates/wiggle/macro/src/lib.rs

@@ -66,14 +66,20 @@ use syn::parse_macro_input;
 ///
 /// /// For all types used in the `Error` position of a `Result` in the module
 /// /// traits, you must implement `GuestErrorType` which tells wiggle-generated
-/// /// code how to determine if a method call has been successful, as well as
+/// /// code what value to return when the method returns Ok(...).
-/// /// how to translate a wiggle runtime error into an ABI-level error.
+/// impl GuestErrorType for types::Errno {
-/// impl<'a> GuestErrorType<'a> for types::Errno {
-///     type Context = YourCtxType;
 ///     fn success() -> Self {
 ///         unimplemented!()
 ///     }
-///     fn from_error(_e: wiggle::GuestError, _c: &Self::Context) -> Self {
+/// }
+///
+/// /// The `types::GuestErrorConversion` trait is also generated with a method for
+/// /// each type used in the `Error` position. This trait allows wiggle-generated
+/// /// code to convert a `wiggle::GuestError` into the right error type. The trait
+/// /// must be implemented for the user's `ctx` type.
+///
+/// impl types::GuestErrorConversion for YourCtxType {
+///     fn into_errno(&self, _e: wiggle::GuestError) -> types::Errno {
 ///         unimplemented!()
 ///     }
 /// }

crates/wiggle/src/guest_type.rs

@@ -1,10 +1,14 @@
 use crate::{GuestError, GuestPtr};
 use std::mem;
 
-pub trait GuestErrorType<'a> {
+/// A trait for types which are used to report errors. Each type used in the
-    type Context;
+/// first result position of an interface function is used, by convention, to
+/// indicate whether the function was successful and subsequent results are valid,
+/// or whether an error occured. This trait allows wiggle to return the correct
+/// value when the interface function's idiomatic Rust method returns
+/// Ok(<rest of return values>).
+pub trait GuestErrorType {
     fn success() -> Self;
-    fn from_error(e: GuestError, ctx: &Self::Context) -> Self;
 }
 
 /// A trait for types that are intended to be pointees in `GuestPtr<T>`.

crates/wiggle/test-helpers/src/lib.rs

@@ -313,16 +313,16 @@ impl<'a> WasiCtx<'a> {
 // with these errors. We just push them to vecs.
 #[macro_export]
 macro_rules! impl_errno {
-    ( $errno:ty ) => {
+    ( $errno:ty, $convert:path ) => {
-        impl<'a> wiggle::GuestErrorType<'a> for $errno {
+        impl wiggle::GuestErrorType for $errno {
-            type Context = WasiCtx<'a>;
             fn success() -> $errno {
                 <$errno>::Ok
             }
-            fn from_error(e: GuestError, ctx: &WasiCtx) -> $errno {
+        }
-                eprintln!("GUEST ERROR: {:?}", e);
+        impl<'a> $convert for WasiCtx<'a> {
-                ctx.guest_errors.borrow_mut().push(e);
+            fn into_errno(&self, e: wiggle::GuestError) -> $errno {
-                types::Errno::InvalidArg
+                eprintln!("GuestError: {:?}", e);
+                <$errno>::InvalidArg
             }
         }
     };

crates/wiggle/tests/arrays.rs

@@ -1,5 +1,5 @@
 use proptest::prelude::*;
-use wiggle::{GuestError, GuestMemory, GuestPtr};
+use wiggle::{GuestMemory, GuestPtr};
 use wiggle_test::{impl_errno, HostMemory, MemArea, WasiCtx};
 
 wiggle::from_witx!({
@@ -7,7 +7,7 @@ wiggle::from_witx!({
     ctx: WasiCtx,
 });
 
-impl_errno!(types::Errno);
+impl_errno!(types::Errno, types::GuestErrorConversion);
 
 impl<'a> arrays::Arrays for WasiCtx<'a> {
     fn reduce_excuses(

crates/wiggle/tests/atoms.rs

@@ -1,5 +1,5 @@
 use proptest::prelude::*;
-use wiggle::{GuestError, GuestMemory};
+use wiggle::GuestMemory;
 use wiggle_test::{impl_errno, HostMemory, MemArea, WasiCtx};
 
 wiggle::from_witx!({
@@ -7,7 +7,7 @@ wiggle::from_witx!({
     ctx: WasiCtx,
 });
 
-impl_errno!(types::Errno);
+impl_errno!(types::Errno, types::GuestErrorConversion);
 
 impl<'a> atoms::Atoms for WasiCtx<'a> {
     fn int_float_args(&self, an_int: u32, an_float: f32) -> Result<(), types::Errno> {

crates/wiggle/tests/flags.rs

@@ -1,6 +1,6 @@
 use proptest::prelude::*;
 use std::convert::TryFrom;
-use wiggle::{GuestError, GuestMemory, GuestPtr};
+use wiggle::{GuestMemory, GuestPtr};
 use wiggle_test::{impl_errno, HostMemory, MemArea, WasiCtx};
 
 wiggle::from_witx!({
@@ -8,7 +8,7 @@ wiggle::from_witx!({
     ctx: WasiCtx,
 });
 
-impl_errno!(types::Errno);
+impl_errno!(types::Errno, types::GuestErrorConversion);
 
 impl<'a> flags::Flags for WasiCtx<'a> {
     fn configure_car(

crates/wiggle/tests/handles.rs

@@ -1,5 +1,5 @@
 use proptest::prelude::*;
-use wiggle::{GuestError, GuestMemory, GuestType};
+use wiggle::{GuestMemory, GuestType};
 use wiggle_test::{impl_errno, HostMemory, MemArea, WasiCtx};
 
 const FD_VAL: u32 = 123;
@@ -9,7 +9,7 @@ wiggle::from_witx!({
     ctx: WasiCtx,
 });
 
-impl_errno!(types::Errno);
+impl_errno!(types::Errno, types::GuestErrorConversion);
 
 impl<'a> handle_examples::HandleExamples for WasiCtx<'a> {
     fn fd_create(&self) -> Result<types::Fd, types::Errno> {

crates/wiggle/tests/ints.rs

@@ -1,6 +1,6 @@
 use proptest::prelude::*;
 use std::convert::TryFrom;
-use wiggle::{GuestError, GuestMemory};
+use wiggle::GuestMemory;
 use wiggle_test::{impl_errno, HostMemory, MemArea, WasiCtx};
 
 wiggle::from_witx!({
@@ -8,7 +8,7 @@ wiggle::from_witx!({
     ctx: WasiCtx,
 });
 
-impl_errno!(types::Errno);
+impl_errno!(types::Errno, types::GuestErrorConversion);
 
 impl<'a> ints::Ints for WasiCtx<'a> {
     fn cookie_cutter(&self, init_cookie: types::Cookie) -> Result<types::Bool, types::Errno> {

crates/wiggle/tests/pointers.rs

@@ -1,5 +1,5 @@
 use proptest::prelude::*;
-use wiggle::{GuestError, GuestMemory, GuestPtr};
+use wiggle::{GuestMemory, GuestPtr};
 use wiggle_test::{impl_errno, HostMemory, MemArea, WasiCtx};
 
 wiggle::from_witx!({
@@ -7,7 +7,7 @@ wiggle::from_witx!({
     ctx: WasiCtx,
 });
 
-impl_errno!(types::Errno);
+impl_errno!(types::Errno, types::GuestErrorConversion);
 
 impl<'a> pointers::Pointers for WasiCtx<'a> {
     fn pointers_and_enums<'b>(

crates/wiggle/tests/strings.rs

@@ -1,5 +1,5 @@
 use proptest::prelude::*;
-use wiggle::{GuestBorrows, GuestError, GuestMemory, GuestPtr};
+use wiggle::{GuestBorrows, GuestMemory, GuestPtr};
 use wiggle_test::{impl_errno, HostMemory, MemArea, MemAreas, WasiCtx};
 
 wiggle::from_witx!({
@@ -7,7 +7,7 @@ wiggle::from_witx!({
     ctx: WasiCtx,
 });
 
-impl_errno!(types::Errno);
+impl_errno!(types::Errno, types::GuestErrorConversion);
 
 impl<'a> strings::Strings for WasiCtx<'a> {
     fn hello_string(&self, a_string: &GuestPtr<str>) -> Result<u32, types::Errno> {

crates/wiggle/tests/structs.rs

@@ -1,5 +1,5 @@
 use proptest::prelude::*;
-use wiggle::{GuestError, GuestMemory, GuestPtr};
+use wiggle::{GuestMemory, GuestPtr};
 use wiggle_test::{impl_errno, HostMemory, MemArea, WasiCtx};
 
 wiggle::from_witx!({
@@ -7,7 +7,7 @@ wiggle::from_witx!({
     ctx: WasiCtx,
 });
 
-impl_errno!(types::Errno);
+impl_errno!(types::Errno, types::GuestErrorConversion);
 
 impl<'a> structs::Structs for WasiCtx<'a> {
     fn sum_of_pair(&self, an_pair: &types::PairInts) -> Result<i64, types::Errno> {

crates/wiggle/tests/union.rs

@@ -1,5 +1,5 @@
 use proptest::prelude::*;
-use wiggle::{GuestError, GuestMemory, GuestType};
+use wiggle::{GuestMemory, GuestType};
 use wiggle_test::{impl_errno, HostMemory, MemArea, WasiCtx};
 
 wiggle::from_witx!({
@@ -7,7 +7,7 @@ wiggle::from_witx!({
     ctx: WasiCtx,
 });
 
-impl_errno!(types::Errno);
+impl_errno!(types::Errno, types::GuestErrorConversion);
 
 // Avoid panics on overflow
 fn mult_lose_overflow(a: i32, b: u32) -> i32 {

crates/wiggle/tests/wasi.rs

@@ -23,17 +23,16 @@ fn document_equivelant() {
 
 type Result<T> = std::result::Result<T, types::Errno>;
 
-impl<'a> GuestErrorType<'a> for types::Errno {
+impl GuestErrorType for types::Errno {
-    type Context = WasiCtx<'a>;
-
     fn success() -> types::Errno {
         types::Errno::Success
     }
+}
 
-    fn from_error(e: GuestError, ctx: &Self::Context) -> types::Errno {
+impl<'a> types::GuestErrorConversion for WasiCtx<'a> {
-        eprintln!("GUEST ERROR: {:?}", e);
+    fn into_errno(&self, e: GuestError) -> types::Errno {
-        ctx.guest_errors.borrow_mut().push(e);
+        eprintln!("GuestError {:?}", e);
-        types::Errno::Io
+        types::Errno::Badf
     }
 }