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

Result alias for convienient use of anyhow::Error without depending on anyhow #5853

Merged
merged 6 commits into from
Feb 24, 2023

Conversation

rylev
Copy link
Contributor

@rylev rylev commented Feb 22, 2023

As discussed on Zulip, the bindgen! macro assumes the usage of anyhow. This adds a type alias to wasmtime for std::result::Result<T, anyhow::Error> that allows users to use wasmtime::Result where a std::result::Result<T, anyhow::Error> is expected.

In particular, this means that users of bindgen! can use wasmtime::Result when they are implementing an import function.

One caveat: after making this change, I realized that the bindgen! macro has the trappable_error_type option which could be used to change the error type assume. While some changes need to be made no matter what (as many of the methods generated by the macro still assume anyhow and don't respect that option), we might want to consider not adding this type alias and just making it clearer that users should use the trappable_error_type option if they are not using anyhow.

r? @alexcrichton

Copy link
Contributor

@pchickey pchickey left a comment

Choose a reason for hiding this comment

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

I'm fine with this, Alex what do you think?

crates/wasmtime/src/lib.rs Show resolved Hide resolved
crates/wasmtime/src/lib.rs Outdated Show resolved Hide resolved
@github-actions github-actions bot added the wasmtime:api Related to the API of the `wasmtime` crate itself label Feb 22, 2023
@github-actions
Copy link

Subscribe to Label Action

cc @peterhuene

This issue or pull request has been labeled: "wasmtime:api"

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

  • peterhuene: wasmtime:api

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

Learn more.

crates/wasmtime/src/lib.rs Outdated Show resolved Hide resolved
tests/all/memory_creator.rs Outdated Show resolved Hide resolved
@github-actions github-actions bot added the fuzzing Issues related to our fuzzing infrastructure label Feb 24, 2023
@github-actions
Copy link

Subscribe to Label Action

cc @fitzgen

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

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

  • fitzgen: fuzzing

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

Learn more.

@rylev
Copy link
Contributor Author

rylev commented Feb 24, 2023

@alexcrichton I applied your suggestion!

@alexcrichton alexcrichton added this pull request to the merge queue Feb 24, 2023
Merged via the queue into bytecodealliance:main with commit 6d6bd0e Feb 24, 2023
@rylev rylev deleted the result-alias branch September 7, 2023 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fuzzing Issues related to our fuzzing infrastructure wasmtime:api Related to the API of the `wasmtime` crate itself
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants