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

Host-wasmtime-rust: import functions are able to Trap execution #388

Merged
merged 11 commits into from
Oct 21, 2022

Conversation

pchickey
Copy link
Contributor

@pchickey pchickey commented Oct 21, 2022

Pat Hickey added 9 commits October 19, 2022 13:26
this is implied since main is the default branch, and means that in other places
that depend on the default branch, the wasmtime brought in by these
crates is not the same dependency

additionally, drop the dep on wasmtime-wasi altogether, it is not used.
just to functions with a single result which is a result<a, e> and e is
a defined type. This ensures we can generate the std::error::Error impl
for those types, otherwise we cant wrap them up into an anyhow::Error.
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.

Looks good to me!

I think CI can be fixed by copying this block for the new test.

At least that's how existing tests don't trip up over this error. It may also be fixable by using #[cfg] on the include! macro invocation so only run the include! on wasm which would avoid the need to update Cargo.toml as well (which would be nice!)

Pat Hickey added 2 commits October 21, 2022 09:36
@pchickey pchickey marked this pull request as ready for review October 21, 2022 16:36
@alexcrichton alexcrichton merged commit 3cae985 into bytecodealliance:main Oct 21, 2022
@pchickey pchickey deleted the wasmtime_trapping branch October 21, 2022 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants