Stream: git-wasmtime

Topic: wasmtime / PR #5853 Result alias for convienient use of a...

Wasmtime GitHub notifications bot (Feb 22 2023 at 17:24):

rylev opened PR #5853 from result-alias to main:

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->
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

Wasmtime GitHub notifications bot (Feb 22 2023 at 17:38):

pchickey submitted PR review.

Wasmtime GitHub notifications bot (Feb 22 2023 at 17:38):

pchickey submitted PR review.

Wasmtime GitHub notifications bot (Feb 22 2023 at 17:38):

pchickey created PR review comment:

I'd add a note saying that the type is identical to anyhow::Result as well

Wasmtime GitHub notifications bot (Feb 22 2023 at 17:56):

abrown submitted PR review.

Wasmtime GitHub notifications bot (Feb 22 2023 at 17:56):

abrown submitted PR review.

Wasmtime GitHub notifications bot (Feb 22 2023 at 17:56):

abrown created PR review comment:

/// A convenience wrapper for `Result<T, anyhow::Error>`.

Wasmtime GitHub notifications bot (Feb 22 2023 at 18:06):

rylev updated PR #5853 from result-alias to main.

Wasmtime GitHub notifications bot (Feb 23 2023 at 15:27):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Feb 23 2023 at 15:27):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Feb 23 2023 at 15:27):

alexcrichton created PR review comment:

Could this be pub use anyhow::{Result, Error} perhaps? One key feature this is currently missing is the E generic defaulting to anyhow::Error but still allowing two generics.

Wasmtime GitHub notifications bot (Feb 23 2023 at 15:27):

alexcrichton created PR review comment:

e.g. from my comment above this shouldn't need to use std::result:: ideally

Wasmtime GitHub notifications bot (Feb 24 2023 at 09:33):

rylev updated PR #5853 from result-alias to main.

Wasmtime GitHub notifications bot (Feb 24 2023 at 10:50):

rylev updated PR #5853 from result-alias to main.

Wasmtime GitHub notifications bot (Feb 24 2023 at 15:37):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Feb 24 2023 at 17:02):

alexcrichton merged PR #5853.

Last updated: Nov 22 2024 at 16:03 UTC