Stream: git-wasmtime

Topic: wasmtime / PR #5397 Wasmtime component bindgen: opt-in tr...

view this post on Zulip Wasmtime GitHub notifications bot (Dec 08 2022 at 01:08):

pchickey opened PR #5397 from pch/component_bindgen_error_types to main:

This was gotten working over in wit-bindgen, and I ported it to the wasmtime repo since the generator moved.

My earlier failed experiment of a "HostError<T>" has been deleted, and bindgen macro users can now opt in to a special trappable_error_type which should work exactly how wiggle's trappable errors work.

I couldn't port the rust-guest dependent tests from wit-bindgen into this repo so currently I am stuck struggling to replace them with wat.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 08 2022 at 01:34):

pchickey updated PR #5397 from pch/component_bindgen_error_types to main.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 08 2022 at 19:18):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 08 2022 at 19:18):

alexcrichton created PR review comment:

Oh I thought the Error<T> was going to work which would avoid this result-of-result, but could you explain a bit more why it didn't work out?

view this post on Zulip Wasmtime GitHub notifications bot (Dec 08 2022 at 22:24):

pchickey submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 08 2022 at 22:24):

pchickey created PR review comment:

I just don't have any tests that actually use the functionality in yet.

We can't use Result<f64, Error<()>> in this case because Error<T> requires T impl std::error::Error. Additionally, all of this PR only supports substituting named types, and doesn't support substituting primitive types.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 08 2022 at 22:24):

pchickey edited PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 08 2022 at 22:55):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 08 2022 at 22:55):

alexcrichton created PR review comment:

Ah ok makes sense. I'm worried about the ergonomics of this, though. Are you thinking we'd special-case -> result<T, rich-error> in WIT to have a Result<T, E> return value here? For degenerate cases like -> result<T> with no error or primitive errors I'm fine with doing whatever.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 09 2022 at 00:41):

pchickey updated PR #5397 from pch/component_bindgen_error_types to main.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 12 2022 at 20:14):

pchickey updated PR #5397 from pch/component_bindgen_error_types to main.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 13 2022 at 22:18):

pchickey updated PR #5397 from pch/component_bindgen_error_types to main.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 13 2022 at 22:52):

pchickey updated PR #5397 from pch/component_bindgen_error_types to main.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 13 2022 at 23:13):

pchickey updated PR #5397 from pch/component_bindgen_error_types to main.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 13 2022 at 23:13):

pchickey has marked PR #5397 as ready for review.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 13 2022 at 23:15):

pchickey submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 13 2022 at 23:15):

pchickey created PR review comment:

I put the tests in below that show we can special case a interface i { ... result<t, e> } to Result<T, TrappableE> when the user invokes the macro with trappable_error_types: { i::e: TrappableE, ... }

view this post on Zulip Wasmtime GitHub notifications bot (Dec 13 2022 at 23:18):

pchickey edited PR #5397 from pch/component_bindgen_error_types to main:

This was gotten working over in wit-bindgen, and I ported it to the wasmtime repo since the generator moved.

My earlier failed experiment of a "HostError<T>" has been deleted, and bindgen macro users can now opt in to a special trappable_error_type which should work exactly how wiggle's trappable errors work.

When the user provides the macro argument 

trappable_error_type: { i::e: TrappableE }

the code generator looks in wit interfaces named i for a wit type named e, it will generate a type struct TrappableE which impl Error and impl From<E> for TrappableE. In any functions that return result<t, e>, it generates the return type Result<T, TrappableE>, rather than the much less idiomatic Result<Result<T, E>, anyhow::Error>.

I introduced new tests in tests/all/component-model/bindgen/results.rs that show this functionality in use.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 14 2022 at 15:28):

alexcrichton created PR review comment:

Oh all this clap stuff is a holdover from copying over from the wit-bindgen repository, I think all of it can be removed since clap isn't even a dependency in this repo.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 14 2022 at 15:28):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 14 2022 at 15:28):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 14 2022 at 18:08):

pchickey updated PR #5397 from pch/component_bindgen_error_types to main.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 14 2022 at 18:08):

pchickey submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 14 2022 at 18:08):

pchickey created PR review comment:

Oh! I didn't even think to look. Deleted.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 14 2022 at 18:12):

pchickey has enabled auto merge for PR #5397.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 14 2022 at 18:44):

pchickey merged PR #5397.

Last updated: Nov 22 2024 at 16:03 UTC