Rec group builder#13687
Conversation
It previously matched the `I16` variant instead of `ValType`, so it returned the wrong answer for every storage type. The method is otherwise unused inside the tree, which is why this went unnoticed.
somdoron
requested review from
alexcrichton and
pchickey
and removed request for
a team
Adds `RecGroupBuilder`, which lets embedders declare kind-typed labels (`PendingStructId`/`PendingArrayId`/`PendingFuncId`), use them as forward references while defining other types via a small build-time "template" family, and register the whole group at once with `build()`. This makes it possible to construct self-referential and mutually-recursive struct/array/func types directly from the embedder API, which previously required plucking such types out of a module's imports/exports. The builder lowers its members to module-canonical `WasmSubType`s (using 0-based `Module` indices for intra-group references and `Engine` indices for already-registered types) and reuses the existing rec-group registration path, so hash-consing, runtime canonicalization, supertype lists, and GC layouts all come for free. Implements the embedder API requested in bytecodealliance#10176.
somdoron
force-pushed
the
rec-group-builder
branch
from
e4e67d9 to
ff45908
Compare
github-actions
Bot
added
wasmtime:api
fitzgen
added
the
wasm-proposal:gc
fitzgen
requested review from
fitzgen
and removed request for
alexcrichton and
pchickey
|
@somdoron given that you just posted an AI plan verbatim in this PR's associated issue, I am assuming you also used AI to implement this PR. Can you confirm that you have already reviewed the AI tool's work in detail before posting it here and asking reviewers to spend their time on it? And are you taking full responsibility for the code in this PR? https://github.com/bytecodealliance/governance/blob/main/AI_TOOL_POLICY.md |
|
@fitzgen I confirmed that I reviewed the AI tool. I reviewed the code, and I'm taking full responsibility for the code in this PR. The PR is for a real problem I'm suffering from, and is focused and small. |
Subscribe to Label Actioncc @fitzgen DetailsThis issue or pull request has been labeled: "wasm-proposal:gc", "wasmtime:api", "wasmtime:docs"Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
2 participants
Implements the embedder API requested in issue #10176.
Embedders can currently only build one-off struct/array/func types, so types that reference
themselves or each other can't be constructed directly. This adds RecGroupBuilder: declare
kind-typed labels, use them as forward references, and build() the whole recursion group at
once. It lowers to module-canonical WasmSubTypes and reuses the existing rec-group
registration path.
Also fixes an unrelated pre-existing bug in StorageType::is_val_type (matched I16 instead of
ValType), in its own commit.