From 1151f630b8f37e4e4458eceee63e4d6dd0c6221a Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Tue, 23 Mar 2021 22:03:25 -0700 Subject: [PATCH 1/7] wiggle GuestError: improve Display of InFunc, InDataField --- crates/wiggle/generate/src/funcs.rs | 2 ++ crates/wiggle/src/error.rs | 5 +++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/crates/wiggle/generate/src/funcs.rs b/crates/wiggle/generate/src/funcs.rs index af15709d0b..4fcdcba474 100644 --- a/crates/wiggle/generate/src/funcs.rs +++ b/crates/wiggle/generate/src/funcs.rs @@ -139,10 +139,12 @@ impl witx::Bindgen for Rust<'_> { ) { let rt = self.rt; let wrap_err = |location: &str| { + let modulename = self.module.name.as_str(); let funcname = self.funcname; quote! { |e| { #rt::GuestError::InFunc { + modulename: #modulename, funcname: #funcname, location: #location, err: Box::new(#rt::GuestError::from(e)), diff --git a/crates/wiggle/src/error.rs b/crates/wiggle/src/error.rs index 5cfab66675..965a88e816 100644 --- a/crates/wiggle/src/error.rs +++ b/crates/wiggle/src/error.rs @@ -19,14 +19,15 @@ pub enum GuestError { BorrowCheckerOutOfHandles, #[error("Slice length mismatch")] SliceLengthsDiffer, - #[error("In func {funcname}:{location}:")] + #[error("In func {modulename}::{funcname} at {location}: {err}")] InFunc { + modulename: &'static str, funcname: &'static str, location: &'static str, #[source] err: Box, }, - #[error("In data {typename}.{field}:")] + #[error("In data {typename}.{field}: {err}")] InDataField { typename: String, field: String, From 4a9ce90d346e7de785a9393f3e9a2b6d312c4848 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Tue, 23 Mar 2021 22:04:34 -0700 Subject: [PATCH 2/7] GuestError::InDataField never constructed, so delete it --- crates/wiggle/src/error.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/crates/wiggle/src/error.rs b/crates/wiggle/src/error.rs index 965a88e816..e8dd0f5a3b 100644 --- a/crates/wiggle/src/error.rs +++ b/crates/wiggle/src/error.rs @@ -27,13 +27,6 @@ pub enum GuestError { #[source] err: Box, }, - #[error("In data {typename}.{field}: {err}")] - InDataField { - typename: String, - field: String, - #[source] - err: Box, - }, #[error("Invalid UTF-8 encountered: {0:?}")] InvalidUtf8(#[from] ::std::str::Utf8Error), #[error("Int conversion error: {0:?}")] From f74b0291adbdc94552bc014fc2c9497ef1e12a1a Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Tue, 23 Mar 2021 22:14:49 -0700 Subject: [PATCH 3/7] dead code: remove GuestErrorConversion, it now is never called --- crates/wiggle/generate/src/lib.rs | 12 ------------ crates/wiggle/generate/src/names.rs | 5 ----- 2 files changed, 17 deletions(-) diff --git a/crates/wiggle/generate/src/lib.rs b/crates/wiggle/generate/src/lib.rs index 90cca766d1..cecd610b50 100644 --- a/crates/wiggle/generate/src/lib.rs +++ b/crates/wiggle/generate/src/lib.rs @@ -38,17 +38,6 @@ pub fn generate(doc: &witx::Document, names: &Names, settings: &CodegenSettings) } }); - 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: #rt::GuestError) -> #typename;) - }); - let guest_error_conversion = quote! { - pub trait GuestErrorConversion { - #(#guest_error_methods)* - } - }; - let user_error_methods = settings.errors.iter().map(|errtype| { let abi_typename = names.type_ref(&errtype.abi_type(), anon_lifetime()); let user_typename = errtype.typename(); @@ -82,7 +71,6 @@ pub fn generate(doc: &witx::Document, names: &Names, settings: &CodegenSettings) #(#types)* #(#constants)* - #guest_error_conversion #user_error_conversion } #(#modules)* diff --git a/crates/wiggle/generate/src/names.rs b/crates/wiggle/generate/src/names.rs index ae3b6bc74b..087b753d18 100644 --- a/crates/wiggle/generate/src/names.rs +++ b/crates/wiggle/generate/src/names.rs @@ -196,11 +196,6 @@ impl Names { } } - pub fn guest_error_conversion_method(&self, tref: &TypeRef) -> Ident { - let suffix = Self::snake_typename(tref); - format_ident!("into_{}", suffix) - } - pub fn user_error_conversion_method(&self, user_type: &UserErrorType) -> Ident { let abi_type = Self::snake_typename(&user_type.abi_type()); format_ident!( From 1c4af27f2d23730c1ed2133550ed90066d2c6889 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Tue, 23 Mar 2021 22:20:29 -0700 Subject: [PATCH 4/7] delete GuestErrorConversion from docs, tests --- crates/wiggle/macro/src/lib.rs | 11 ------- .../wiggle/test-helpers/examples/tracing.rs | 4 --- crates/wiggle/test-helpers/src/lib.rs | 9 +----- crates/wiggle/tests/atoms.rs | 2 +- crates/wiggle/tests/atoms_async.rs | 2 +- crates/wiggle/tests/errors.rs | 29 +++---------------- crates/wiggle/tests/flags.rs | 2 +- crates/wiggle/tests/handles.rs | 2 +- crates/wiggle/tests/ints.rs | 2 +- crates/wiggle/tests/lists.rs | 2 +- crates/wiggle/tests/pointers.rs | 2 +- crates/wiggle/tests/records.rs | 2 +- crates/wiggle/tests/strings.rs | 2 +- crates/wiggle/tests/variant.rs | 2 +- crates/wiggle/tests/wasi.rs | 9 +----- 15 files changed, 16 insertions(+), 66 deletions(-) diff --git a/crates/wiggle/macro/src/lib.rs b/crates/wiggle/macro/src/lib.rs index 09ab2f6353..4892a9420e 100644 --- a/crates/wiggle/macro/src/lib.rs +++ b/crates/wiggle/macro/src/lib.rs @@ -113,17 +113,6 @@ use syn::parse_macro_input; /// } /// } /// -/// /// 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!() -/// } -/// } -/// /// /// If you specify a `error` mapping to the macro, you must implement the /// /// `types::UserErrorConversion` for your ctx type as well. This trait gives /// /// you an opportunity to store or log your rich error type, while returning diff --git a/crates/wiggle/test-helpers/examples/tracing.rs b/crates/wiggle/test-helpers/examples/tracing.rs index 1e1517d8d9..63358a4da6 100644 --- a/crates/wiggle/test-helpers/examples/tracing.rs +++ b/crates/wiggle/test-helpers/examples/tracing.rs @@ -27,10 +27,6 @@ witx_literal: " errors: { errno => RichError }, }); -// The impl of GuestErrorConversion works just like in every other test where -// we have a single error type with witx `$errno` with the success called `$ok` -impl_errno!(types::Errno, types::GuestErrorConversion); - /// When the `errors` mapping in witx is non-empty, we need to impl the /// types::UserErrorConversion trait that wiggle generates from that mapping. impl<'a> types::UserErrorConversion for WasiCtx<'a> { diff --git a/crates/wiggle/test-helpers/src/lib.rs b/crates/wiggle/test-helpers/src/lib.rs index 99571367a5..fa36bee5de 100644 --- a/crates/wiggle/test-helpers/src/lib.rs +++ b/crates/wiggle/test-helpers/src/lib.rs @@ -347,18 +347,11 @@ impl<'a> WasiCtx<'a> { // with these errors. We just push them to vecs. #[macro_export] macro_rules! impl_errno { - ( $errno:ty, $convert:path ) => { + ( $errno:ty ) => { impl wiggle::GuestErrorType for $errno { fn success() -> $errno { <$errno>::Ok } } - impl<'a> $convert for WasiCtx<'a> { - fn into_errno(&self, e: wiggle::GuestError) -> $errno { - eprintln!("GuestError: {:?}", e); - self.guest_errors.borrow_mut().push(e); - <$errno>::InvalidArg - } - } }; } diff --git a/crates/wiggle/tests/atoms.rs b/crates/wiggle/tests/atoms.rs index 9aabbc74c5..8cc2acaa40 100644 --- a/crates/wiggle/tests/atoms.rs +++ b/crates/wiggle/tests/atoms.rs @@ -6,7 +6,7 @@ wiggle::from_witx!({ witx: ["$CARGO_MANIFEST_DIR/tests/atoms.witx"], }); -impl_errno!(types::Errno, types::GuestErrorConversion); +impl_errno!(types::Errno); impl<'a> atoms::Atoms for WasiCtx<'a> { fn int_float_args(&self, an_int: u32, an_float: f32) -> Result<(), types::Errno> { diff --git a/crates/wiggle/tests/atoms_async.rs b/crates/wiggle/tests/atoms_async.rs index bdaf32d796..41abbdebfb 100644 --- a/crates/wiggle/tests/atoms_async.rs +++ b/crates/wiggle/tests/atoms_async.rs @@ -12,7 +12,7 @@ wiggle::from_witx!({ } }); -impl_errno!(types::Errno, types::GuestErrorConversion); +impl_errno!(types::Errno); #[wiggle::async_trait(?Send)] impl<'a> atoms::Atoms for WasiCtx<'a> { diff --git a/crates/wiggle/tests/errors.rs b/crates/wiggle/tests/errors.rs index ec9291ff53..98cd30edf7 100644 --- a/crates/wiggle/tests/errors.rs +++ b/crates/wiggle/tests/errors.rs @@ -26,9 +26,7 @@ mod convert_just_errno { errors: { errno => RichError }, }); - // The impl of GuestErrorConversion works just like in every other test where - // we have a single error type with witx `$errno` with the success called `$ok` - impl_errno!(types::Errno, types::GuestErrorConversion); + impl_errno!(types::Errno); /// When the `errors` mapping in witx is non-empty, we need to impl the /// types::UserErrorConversion trait that wiggle generates from that mapping. @@ -104,7 +102,7 @@ mod convert_just_errno { /// we use two distinct error types. mod convert_multiple_error_types { pub use super::convert_just_errno::RichError; - use wiggle_test::WasiCtx; + use wiggle_test::{impl_errno, WasiCtx}; /// Test that we can map multiple types of errors. #[derive(Debug, thiserror::Error)] @@ -135,27 +133,8 @@ mod convert_multiple_error_types { errors: { errno => RichError, errno2 => AnotherRichError }, }); - // Can't use the impl_errno! macro as usual here because the conversion - // trait ends up having two methods. - // We aren't going to execute this code, so the bodies are elided. - impl<'a> types::GuestErrorConversion for WasiCtx<'a> { - fn into_errno(&self, _e: wiggle::GuestError) -> types::Errno { - unimplemented!() - } - fn into_errno2(&self, _e: wiggle::GuestError) -> types::Errno2 { - unimplemented!() - } - } - impl wiggle::GuestErrorType for types::Errno { - fn success() -> types::Errno { - ::Ok - } - } - impl wiggle::GuestErrorType for types::Errno2 { - fn success() -> types::Errno2 { - ::Ok - } - } + impl_errno!(types::Errno); + impl_errno!(types::Errno2); // The UserErrorConversion trait will also have two methods for this test. They correspond to // each member of the `errors` mapping. diff --git a/crates/wiggle/tests/flags.rs b/crates/wiggle/tests/flags.rs index 1a1f98c6c5..e29cfdeab7 100644 --- a/crates/wiggle/tests/flags.rs +++ b/crates/wiggle/tests/flags.rs @@ -7,7 +7,7 @@ wiggle::from_witx!({ witx: ["$CARGO_MANIFEST_DIR/tests/flags.witx"], }); -impl_errno!(types::Errno, types::GuestErrorConversion); +impl_errno!(types::Errno); impl<'a> flags::Flags for WasiCtx<'a> { fn configure_car( diff --git a/crates/wiggle/tests/handles.rs b/crates/wiggle/tests/handles.rs index 15fb0d36be..6dce915286 100644 --- a/crates/wiggle/tests/handles.rs +++ b/crates/wiggle/tests/handles.rs @@ -8,7 +8,7 @@ wiggle::from_witx!({ witx: ["$CARGO_MANIFEST_DIR/tests/handles.witx"], }); -impl_errno!(types::Errno, types::GuestErrorConversion); +impl_errno!(types::Errno); impl<'a> handle_examples::HandleExamples for WasiCtx<'a> { fn fd_create(&self) -> Result { diff --git a/crates/wiggle/tests/ints.rs b/crates/wiggle/tests/ints.rs index 23310ee561..eb54b02f4e 100644 --- a/crates/wiggle/tests/ints.rs +++ b/crates/wiggle/tests/ints.rs @@ -7,7 +7,7 @@ wiggle::from_witx!({ witx: ["$CARGO_MANIFEST_DIR/tests/ints.witx"], }); -impl_errno!(types::Errno, types::GuestErrorConversion); +impl_errno!(types::Errno); impl<'a> ints::Ints for WasiCtx<'a> { fn cookie_cutter(&self, init_cookie: types::Cookie) -> Result { diff --git a/crates/wiggle/tests/lists.rs b/crates/wiggle/tests/lists.rs index d2f76fd152..e797b5f0ca 100644 --- a/crates/wiggle/tests/lists.rs +++ b/crates/wiggle/tests/lists.rs @@ -6,7 +6,7 @@ wiggle::from_witx!({ witx: ["$CARGO_MANIFEST_DIR/tests/lists.witx"], }); -impl_errno!(types::Errno, types::GuestErrorConversion); +impl_errno!(types::Errno); impl<'a> lists::Lists for WasiCtx<'a> { fn reduce_excuses( diff --git a/crates/wiggle/tests/pointers.rs b/crates/wiggle/tests/pointers.rs index 96e33a734d..7988733074 100644 --- a/crates/wiggle/tests/pointers.rs +++ b/crates/wiggle/tests/pointers.rs @@ -6,7 +6,7 @@ wiggle::from_witx!({ witx: ["$CARGO_MANIFEST_DIR/tests/pointers.witx"], }); -impl_errno!(types::Errno, types::GuestErrorConversion); +impl_errno!(types::Errno); impl<'a> pointers::Pointers for WasiCtx<'a> { fn pointers_and_enums<'b>( diff --git a/crates/wiggle/tests/records.rs b/crates/wiggle/tests/records.rs index fa448c3dc3..3c61e621a3 100644 --- a/crates/wiggle/tests/records.rs +++ b/crates/wiggle/tests/records.rs @@ -6,7 +6,7 @@ wiggle::from_witx!({ witx: ["$CARGO_MANIFEST_DIR/tests/records.witx"], }); -impl_errno!(types::Errno, types::GuestErrorConversion); +impl_errno!(types::Errno); impl<'a> records::Records for WasiCtx<'a> { fn sum_of_pair(&self, an_pair: &types::PairInts) -> Result { diff --git a/crates/wiggle/tests/strings.rs b/crates/wiggle/tests/strings.rs index 420a5c0a62..e6eab2a9e2 100644 --- a/crates/wiggle/tests/strings.rs +++ b/crates/wiggle/tests/strings.rs @@ -6,7 +6,7 @@ wiggle::from_witx!({ witx: ["$CARGO_MANIFEST_DIR/tests/strings.witx"], }); -impl_errno!(types::Errno, types::GuestErrorConversion); +impl_errno!(types::Errno); impl<'a> strings::Strings for WasiCtx<'a> { fn hello_string(&self, a_string: &GuestPtr) -> Result { diff --git a/crates/wiggle/tests/variant.rs b/crates/wiggle/tests/variant.rs index 5bddfb6aab..d367938106 100644 --- a/crates/wiggle/tests/variant.rs +++ b/crates/wiggle/tests/variant.rs @@ -6,7 +6,7 @@ wiggle::from_witx!({ witx: ["$CARGO_MANIFEST_DIR/tests/variant.witx"], }); -impl_errno!(types::Errno, types::GuestErrorConversion); +impl_errno!(types::Errno); // Avoid panics on overflow fn mult_lose_overflow(a: i32, b: u32) -> i32 { diff --git a/crates/wiggle/tests/wasi.rs b/crates/wiggle/tests/wasi.rs index 6dc6d84ea5..d95f008449 100644 --- a/crates/wiggle/tests/wasi.rs +++ b/crates/wiggle/tests/wasi.rs @@ -1,4 +1,4 @@ -use wiggle::{GuestError, GuestErrorType, GuestPtr, GuestSlice}; +use wiggle::{GuestErrorType, GuestPtr, GuestSlice}; use wiggle_test::WasiCtx; // This test file exists to make sure that the entire `wasi.witx` file can be @@ -31,13 +31,6 @@ impl GuestErrorType for types::Errno { } } -impl<'a> types::GuestErrorConversion for WasiCtx<'a> { - fn into_errno(&self, e: GuestError) -> types::Errno { - eprintln!("GuestError {:?}", e); - types::Errno::Badf - } -} - impl<'a> crate::wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx<'a> { fn args_get(&self, _argv: &GuestPtr>, _argv_buf: &GuestPtr) -> Result<()> { unimplemented!("args_get") From e6c7e00a5220b48c6d23b9ffd38078dc103775a5 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Wed, 24 Mar 2021 10:39:06 -0700 Subject: [PATCH 5/7] wiggle-using crates: delete GuestErrorConversion --- crates/wasi-common/src/snapshots/preview_0.rs | 8 -------- crates/wasi-common/src/snapshots/preview_1.rs | 7 ------- crates/wasi-crypto/src/wiggle_interfaces/error.rs | 7 ------- crates/wasi-nn/src/witx.rs | 10 ---------- 4 files changed, 32 deletions(-) diff --git a/crates/wasi-common/src/snapshots/preview_0.rs b/crates/wasi-common/src/snapshots/preview_0.rs index 909c228599..02e6289cd2 100644 --- a/crates/wasi-common/src/snapshots/preview_0.rs +++ b/crates/wasi-common/src/snapshots/preview_0.rs @@ -24,14 +24,6 @@ impl wiggle::GuestErrorType for types::Errno { } } -impl types::GuestErrorConversion for WasiCtx { - fn into_errno(&self, e: wiggle::GuestError) -> types::Errno { - debug!("Guest error: {:?}", e); - let snapshot1_errno: snapshot1_types::Errno = e.into(); - snapshot1_errno.into() - } -} - impl types::UserErrorConversion for WasiCtx { fn errno_from_error(&self, e: Error) -> Result { debug!("Error: {:?}", e); diff --git a/crates/wasi-common/src/snapshots/preview_1.rs b/crates/wasi-common/src/snapshots/preview_1.rs index c451856207..494ba9111e 100644 --- a/crates/wasi-common/src/snapshots/preview_1.rs +++ b/crates/wasi-common/src/snapshots/preview_1.rs @@ -30,13 +30,6 @@ impl wiggle::GuestErrorType for types::Errno { } } -impl types::GuestErrorConversion for WasiCtx { - fn into_errno(&self, e: wiggle::GuestError) -> types::Errno { - debug!("Guest error: {:?}", e); - e.into() - } -} - impl types::UserErrorConversion for WasiCtx { fn errno_from_error(&self, e: Error) -> Result { debug!("Error: {:?}", e); diff --git a/crates/wasi-crypto/src/wiggle_interfaces/error.rs b/crates/wasi-crypto/src/wiggle_interfaces/error.rs index e3651f6db9..fd13a87331 100644 --- a/crates/wasi-crypto/src/wiggle_interfaces/error.rs +++ b/crates/wasi-crypto/src/wiggle_interfaces/error.rs @@ -52,13 +52,6 @@ impl<'a> wiggle::GuestErrorType for guest_types::CryptoErrno { } } -impl guest_types::GuestErrorConversion for WasiCryptoCtx { - fn into_crypto_errno(&self, e: wiggle::GuestError) -> guest_types::CryptoErrno { - eprintln!("GuestError (witx) {:?}", e); - guest_types::CryptoErrno::GuestError - } -} - impl From for guest_types::CryptoErrno { fn from(e: wiggle::GuestError) -> Self { eprintln!("GuestError (impl) {:?}", e); diff --git a/crates/wasi-nn/src/witx.rs b/crates/wasi-nn/src/witx.rs index 72120276b7..32cc0167d8 100644 --- a/crates/wasi-nn/src/witx.rs +++ b/crates/wasi-nn/src/witx.rs @@ -10,16 +10,6 @@ wiggle::from_witx!({ use types::NnErrno; -/// Wiggle generates code that performs some input validation on the arguments passed in by users of -/// wasi-nn. Here we convert the validation error into one (or more, eventually) of the error -/// variants defined in the witx. -impl types::GuestErrorConversion for WasiNnCtx { - fn into_nn_errno(&self, e: wiggle::GuestError) -> NnErrno { - eprintln!("Guest error: {:?}", e); - NnErrno::InvalidArgument - } -} - impl<'a> types::UserErrorConversion for WasiNnCtx { fn nn_errno_from_wasi_nn_error(&self, e: WasiNnError) -> Result { eprintln!("Host error: {:?}", e); From 1d663bfd71b587ce76a1f9aba2479bf3e1eba98a Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Wed, 24 Mar 2021 11:19:12 -0700 Subject: [PATCH 6/7] delete straggler InDataField --- crates/wasi-common/src/snapshots/preview_1.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/wasi-common/src/snapshots/preview_1.rs b/crates/wasi-common/src/snapshots/preview_1.rs index 494ba9111e..0ee4044c37 100644 --- a/crates/wasi-common/src/snapshots/preview_1.rs +++ b/crates/wasi-common/src/snapshots/preview_1.rs @@ -96,7 +96,6 @@ impl From for types::Errno { InvalidUtf8 { .. } => Self::Ilseq, TryFromIntError { .. } => Self::Overflow, InFunc { err, .. } => types::Errno::from(*err), - InDataField { err, .. } => types::Errno::from(*err), SliceLengthsDiffer { .. } => Self::Fault, BorrowCheckerOutOfHandles { .. } => Self::Fault, } From df18b44c532f93624b3b76971667a385827cf7cb Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Wed, 24 Mar 2021 11:35:20 -0700 Subject: [PATCH 7/7] oops --- crates/wiggle/test-helpers/examples/tracing.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/wiggle/test-helpers/examples/tracing.rs b/crates/wiggle/test-helpers/examples/tracing.rs index 63358a4da6..9930f705c0 100644 --- a/crates/wiggle/test-helpers/examples/tracing.rs +++ b/crates/wiggle/test-helpers/examples/tracing.rs @@ -27,6 +27,8 @@ witx_literal: " errors: { errno => RichError }, }); +impl_errno!(types::Errno); + /// When the `errors` mapping in witx is non-empty, we need to impl the /// types::UserErrorConversion trait that wiggle generates from that mapping. impl<'a> types::UserErrorConversion for WasiCtx<'a> {