Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wiggle: delete GuestErrorConversion, improve some error reporting #2760

Merged
merged 7 commits into from
Mar 24, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions crates/wiggle/generate/src/funcs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
Expand Down
12 changes: 0 additions & 12 deletions crates/wiggle/generate/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -82,7 +71,6 @@ pub fn generate(doc: &witx::Document, names: &Names, settings: &CodegenSettings)

#(#types)*
#(#constants)*
#guest_error_conversion
#user_error_conversion
}
#(#modules)*
Expand Down
5 changes: 0 additions & 5 deletions crates/wiggle/generate/src/names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down
11 changes: 0 additions & 11 deletions crates/wiggle/macro/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 2 additions & 8 deletions crates/wiggle/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,14 @@ 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<GuestError>,
},
#[error("In data {typename}.{field}:")]
InDataField {
typename: String,
field: String,
#[source]
err: Box<GuestError>,
},
#[error("Invalid UTF-8 encountered: {0:?}")]
InvalidUtf8(#[from] ::std::str::Utf8Error),
#[error("Int conversion error: {0:?}")]
Expand Down
4 changes: 0 additions & 4 deletions crates/wiggle/test-helpers/examples/tracing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
9 changes: 1 addition & 8 deletions crates/wiggle/test-helpers/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
};
}
2 changes: 1 addition & 1 deletion crates/wiggle/tests/atoms.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
2 changes: 1 addition & 1 deletion crates/wiggle/tests/atoms_async.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
29 changes: 4 additions & 25 deletions crates/wiggle/tests/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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 {
<types::Errno>::Ok
}
}
impl wiggle::GuestErrorType for types::Errno2 {
fn success() -> types::Errno2 {
<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.
Expand Down
2 changes: 1 addition & 1 deletion crates/wiggle/tests/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion crates/wiggle/tests/handles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<types::Fd, types::Errno> {
Expand Down
2 changes: 1 addition & 1 deletion crates/wiggle/tests/ints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<types::Bool, types::Errno> {
Expand Down
2 changes: 1 addition & 1 deletion crates/wiggle/tests/lists.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion crates/wiggle/tests/pointers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>(
Expand Down
2 changes: 1 addition & 1 deletion crates/wiggle/tests/records.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<i64, types::Errno> {
Expand Down
2 changes: 1 addition & 1 deletion crates/wiggle/tests/strings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<str>) -> Result<u32, types::Errno> {
Expand Down
2 changes: 1 addition & 1 deletion crates/wiggle/tests/variant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
9 changes: 1 addition & 8 deletions crates/wiggle/tests/wasi.rs
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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<GuestPtr<u8>>, _argv_buf: &GuestPtr<u8>) -> Result<()> {
unimplemented!("args_get")
Expand Down