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

Fixes #2418: Enhance wiggle to generate its UserErrorConverstion trait with a function that returns Result<abi_err, String> #2419

Merged

Conversation

fst-crenshaw
Copy link
Contributor

@fst-crenshaw fst-crenshaw commented Nov 13, 2020

Long-time listener, first-time caller.

I am submitting this WIP PR for issue #2418. Please see issue #2418 for a description.

@github-actions github-actions bot added the wasi Issues pertaining to WASI label Nov 13, 2020
@github-actions
Copy link

Subscribe to Label Action

cc @kubkon

This issue or pull request has been labeled: "wasi"

Thus the following users have been cc'd because of the following labels:

  • kubkon: wasi

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

@peterhuene peterhuene added the wiggle Issues relating to the wiggle code generator. label Nov 14, 2020
…tion that returns

a Result<abi_err, String>.  This enhancement allows hostcall implementations using wiggle
to return an actionable error to the instance (the abi_err) or to terminate the instance
using the String as fatal error information.
…tion that returns

a Result<abi_err, String>.  This enhancement allows hostcall implementations using wiggle
to return an actionable error to the instance (the abi_err) or to terminate the instance
using the String as fatal error information.
…. Hostcall

implementations generated by wiggle now return an Result<abi_error, Trap>.  As a
result, hostcalls experiencing fatal errors may trap, thereby terminating the
wasmtime instance.  This enhancement has been performed for both wasi snapshot1
and wasi snapshot0.
@fst-crenshaw fst-crenshaw marked this pull request as ready for review November 21, 2020 03:15
}

#[no_mangle]
pub unsafe fn #c_abi_name(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For referenced, I recommended this get removed since it needed to get updated anyway and I don't think that anything is using this right now.

crates/wiggle/generate/src/funcs.rs Outdated Show resolved Hide resolved
crates/wasi-common/wig/src/wasi.rs Show resolved Hide resolved
crates/wiggle/generate/src/funcs.rs Outdated Show resolved Hide resolved
crates/wiggle/generate/src/funcs.rs Outdated Show resolved Hide resolved
crates/wiggle/wasmtime/macro/src/lib.rs Show resolved Hide resolved
Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you have anything else you wanted to add in? (just in case the "WIP" title is still active)

@fst-crenshaw fst-crenshaw changed the title [WIP] Fixes #2418: Enhance wiggle to generate its UserErrorConverstion trait with a function that returns Result<abi_err, String> Fixes #2418: Enhance wiggle to generate its UserErrorConverstion trait with a function that returns Result<abi_err, String> Nov 24, 2020
@fst-crenshaw
Copy link
Contributor Author

@alexcrichton -- I believe this is ready to merge. I appreciate you!

@alexcrichton
Copy link
Member

👍

@alexcrichton alexcrichton merged commit b06ed39 into bytecodealliance:main Nov 24, 2020
cfallin added a commit to bytecodealliance/lucet that referenced this pull request Nov 30, 2020
- Refer to a rebased branch of wasmtime that (temporarily) removes
  bytecodealliance/wasmtime#2419; that PR changed some APIs in a way
  that is not yet reflected in Lucet.
- Update a `GuestMemory` impl in a slightly hamfisted way (every
  shared/mut borrow is "just a borrow"; imprecise but maybe still
  sound?)

Both of these things should be fixed before merging; I just want to get
a green CI again for testing!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wasi Issues pertaining to WASI wiggle Issues relating to the wiggle code generator.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants