fitzgen requested alexcrichton for a review on PR #8481.
fitzgen requested wasmtime-core-reviewers for a review on PR #8481.
fitzgen requested wasmtime-default-reviewers for a review on PR #8481.
fitzgen opened PR #8481 from fitzgen:gc-array-types
to bytecodealliance:main
:
This commit adds support for defining array types from Wasm or the host, and managing them inside the engine's types registry. It does not introduce support for allocating or manipulating array values. That functionality will come in future pull requests.
<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
fitzgen updated PR #8481.
github-actions[bot] commented on PR #8481:
Subscribe to Label Action
cc @fitzgen
<details>
This issue or pull request has been labeled: "fuzzing", "wasmtime:api", "wasmtime:c-api"Thus the following users have been cc'd because of the following labels:
- fitzgen: fuzzing
To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.
Learn more.
</details>
alexcrichton submitted PR review.
alexcrichton submitted PR review.
alexcrichton created PR review comment:
What about
Ref::null(ty: HeapType)
as a method to avoid needing to write it here? (I feel like we've wanted this in fuzzing too of "create an arbitrary value")(no need to do it here of course, a follow-up is fine)
alexcrichton created PR review comment:
Would it make sense to fold the upper case and this one together to share more code perhaps? There's some differences so it doesn't look trivial (or obviously worthwhile), but wanted to flag
alexcrichton created PR review comment:
This'd be another good location for the create-null-
Val
-helper (which may actually already exist having seen some code below)
alexcrichton created PR review comment:
Another good spot for a null-constructor on
Val
fitzgen submitted PR review.
fitzgen created PR review comment:
Yeah I attempted this and then aborted when I saw how it was shaping up. I think factoring things out so that you can still capture the differences with a function or trait or enum or set of option arguments or something would ultimately end up with the same amount of code but slightly more convoluted because now you have to understand how the different cases work out with the abstraction mechanism.
fitzgen submitted PR review.
fitzgen created PR review comment:
Yeah this is probably long overdue.
fitzgen submitted PR review.
fitzgen created PR review comment:
I'll go ahead and merge this and then submit a follow up introducing this new method.
fitzgen updated PR #8481.
fitzgen has enabled auto merge for PR #8481.
fitzgen merged PR #8481.
Last updated: Nov 22 2024 at 16:03 UTC