Stream: git-wasmtime

Topic: wasmtime / PR #10765 c-api: new wasmtime_trap_new_code fu...


view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 12:41):

dotcarmen opened PR #10765 from dotcarmen:c-api-wasmtime-new-trap-code to bytecodealliance:main:

<!--
Please make sure you include the following information:

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
right now, returning wasmtime-recognized traps from functions is only available to the Rust API. this PR adds a new function wasmtime_trap_new_code to the c api that returns a new wasm_trap_t with the Trap value associated with the given code.

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 12:41):

dotcarmen requested wasmtime-core-reviewers for a review on PR #10765.

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 12:41):

dotcarmen requested fitzgen for a review on PR #10765.

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 12:41):

dotcarmen requested wasmtime-default-reviewers for a review on PR #10765.

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 12:46):

dotcarmen updated PR #10765.

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 12:47):

dotcarmen updated PR #10765.

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 12:50):

dotcarmen updated PR #10765.

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 13:02):

dotcarmen updated PR #10765.

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 13:06):

dotcarmen updated PR #10765.

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 13:07):

dotcarmen submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 13:07):

dotcarmen created PR review comment:

i kinda figured this is the appropriate way to handle this, if a different way is preferred i can change it :)

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 13:07):

dotcarmen deleted PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 13:09):

dotcarmen updated PR #10765.

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 13:12):

dotcarmen submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 13:12):

dotcarmen created PR review comment:

i'm not sure if this is a good idea or if it's desired, i can change it if not :)

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 13:12):

dotcarmen deleted PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 13:13):

dotcarmen submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 13:13):

dotcarmen created PR review comment:

i'm not sure if this is a good idea or if it's desired, i can change it if not :)

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 13:13):

dotcarmen created PR review comment:

same as above, i'm not sure if assert is the preferred way to handle this

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 13:13):

dotcarmen created PR review comment:

returning a null pointer is kinda an anti-pattern in the wasmtime c-api afaict, but i also think it makes sense here. lmk if a different pattern is preferred

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 13:24):

dotcarmen updated PR #10765.

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 13:24):

dotcarmen edited PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 19:49):

alexcrichton submitted PR review:

Thanks! Would you be up for sync-ing the implementation in C and Rust? The trap code listing here already doesn't match the Rust definition, so using from_u8 I don't think will work for the out-of-fuel trap for example. The convert-to-u8 can probably use trap as u8 nowadays, I'm not sure it was possible to do that when it was originally written. That should make the from_u8 conversion work as well.

Additionally would you be up for adding a test for this? Somewhere within this file is probably a good place to go.

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 19:49):

alexcrichton created PR review comment:

Mind leaving this to your own personal ~/.gitignore?

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 19:49):

alexcrichton created PR review comment:

Another alternative might be to .unwrap() here, effectively asserting that the code is valid?

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 20:26):

dotcarmen updated PR #10765.

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 21:10):

dotcarmen updated PR #10765.

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 21:18):

dotcarmen updated PR #10765.

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 21:23):

dotcarmen updated PR #10765.

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 21:24):

dotcarmen submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 21:24):

dotcarmen created PR review comment:

hmmm apparently my test command doesn't run the tests in this dir, @alexcrichton what's the command to run these tests? i can document it in c-api/README as well :)

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 21:54):

dotcarmen submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 21:54):

dotcarmen created PR review comment:

nvm, found it from ci :)

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 22:18):

dotcarmen updated PR #10765.

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 22:25):

dotcarmen updated PR #10765.

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 22:27):

dotcarmen updated PR #10765.

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 22:28):

dotcarmen updated PR #10765.

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 22:35):

dotcarmen updated PR #10765.

view this post on Zulip Wasmtime GitHub notifications bot (May 11 2025 at 22:46):

dotcarmen requested alexcrichton for a review on PR #10765.

view this post on Zulip Wasmtime GitHub notifications bot (May 13 2025 at 05:11):

alexcrichton created PR review comment:

I'm not actually sure how this works, it probably means that our include paths are set up incorrectly? This should already be taken care of below by <wasmtime/trap.h> I think though?

view this post on Zulip Wasmtime GitHub notifications bot (May 13 2025 at 05:11):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (May 13 2025 at 08:00):

dotcarmen submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (May 13 2025 at 08:00):

dotcarmen created PR review comment:

Ugh I think my LSP keeps sneaking that in. I removed this line before

view this post on Zulip Wasmtime GitHub notifications bot (May 13 2025 at 10:38):

dotcarmen updated PR #10765.

view this post on Zulip Wasmtime GitHub notifications bot (May 13 2025 at 10:39):

dotcarmen updated PR #10765.

view this post on Zulip Wasmtime GitHub notifications bot (May 13 2025 at 10:39):

dotcarmen submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (May 13 2025 at 10:39):

dotcarmen created PR review comment:

removed both accidental includes in 5e2dc3e

view this post on Zulip Wasmtime GitHub notifications bot (May 15 2025 at 05:56):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (May 15 2025 at 06:18):

alexcrichton merged PR #10765.


Last updated: Dec 06 2025 at 06:05 UTC