wasmtime / PR #13687 Rec group builder · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / PR #13687 Rec group builder

Wasmtime GitHub notifications bot (Jun 18 2026 at 13:08):

somdoron opened PR #13687 from somdoron:rec-group-builder to bytecodealliance:main:

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.

Wasmtime GitHub notifications bot (Jun 18 2026 at 13:08):

somdoron requested pchickey for a review on PR #13687.

Wasmtime GitHub notifications bot (Jun 18 2026 at 13:08):

somdoron requested wasmtime-core-reviewers for a review on PR #13687.

Wasmtime GitHub notifications bot (Jun 18 2026 at 13:08):

somdoron requested alexcrichton for a review on PR #13687.

Wasmtime GitHub notifications bot (Jun 18 2026 at 13:08):

somdoron requested wasmtime-default-reviewers for a review on PR #13687.

Wasmtime GitHub notifications bot (Jun 18 2026 at 13:19):

somdoron updated PR #13687.

Wasmtime GitHub notifications bot (Jun 18 2026 at 15:54):

github-actions[bot] added the label wasmtime:api on PR #13687.

Wasmtime GitHub notifications bot (Jun 18 2026 at 15:54):

github-actions[bot] added the label wasmtime:docs on PR #13687.

Wasmtime GitHub notifications bot (Jun 18 2026 at 16:56):

fitzgen added the label wasm-proposal:gc on PR #13687.

Wasmtime GitHub notifications bot (Jun 18 2026 at 16:57):

fitzgen unassigned pchickey from PR #13687 Rec group builder.

Wasmtime GitHub notifications bot (Jun 18 2026 at 16:57):

fitzgen unassigned alexcrichton from PR #13687 Rec group builder.

Wasmtime GitHub notifications bot (Jun 18 2026 at 16:57):

fitzgen requested fitzgen for a review on PR #13687.

Wasmtime GitHub notifications bot (Jun 18 2026 at 17:03):

fitzgen commented on PR #13687:

@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

Wasmtime GitHub notifications bot (Jun 18 2026 at 17:18):

somdoron commented on PR #13687:

@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.

Wasmtime GitHub notifications bot (Jun 18 2026 at 17:45):

github-actions[bot] commented on PR #13687:

Subscribe to Label Action

cc @fitzgen

<details>
This 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:

fitzgen: wasm-proposal:gc

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

Wasmtime GitHub notifications bot (Jun 24 2026 at 15:58):

:memo: fitzgen submitted PR review:

First of all, I want to say that this feature is going to involve a lot of API design, and probably won't land super quickly because of that. If you really need to be able to define rec groups in the host now, you can do that via building Modules (either via WAT strings or via wasm_encoder) that define the rec groups you need and export a global of each of the rec groups types so you can exfiltrate the GlobalType and the global type's ValType on the host via Module::exports. So that should unblock you for the time being, if necessary.

Some high-level thoughts on the design:
I don't think we need to distinguish between pending structs versus pending arrays at the type level. I don't think it is buying us much in terms of correctness by construction or whatever, but it makes ramping up on the API that much harder.
I don't love adding FieldTemplate/HeapTypeTemplate/etc... types to Wasmtime's public API. I think we could avoid that (and also not force callers to construct slices of them, potentially allocating when they otherwise wouldn't) by having a struct type builder that borrows the rec group builder and exposes a method to define fields one at a time, something like this:

```rust
impl RecGroupBuilder<'_> {
pub fn define_struct(&mut self, ty: PendingType) -> StructTypeBuilder<'_> { ... }
}

pub struct StructTypeBuilder<'a> {
rec_group: &'a mut RecGroupBuilder,
// ...
}

impl StructTypeBuilder<'_> {
/// Define a normal, non-forward-reference field.
pub fn field(&mut self, ty: FieldType) { ... }
/// Define a field that is a forward reference to another type defined in this rec group.
pub fn forward_ref_field(&mut self, ty: PendingType) -> ForwardRefFieldBuilder<'_> { ... }
}

pub struct ForwardRefFieldBuilder<'a> {
parent: StructOrArrayTypeBuilder<'a>,
// ...
}

impl ForwardRefFieldBuilder<'_> {
pub fn nullable(&mut self, nullable: bool) -> &mut Self { ... }
pub fn shared(&mut self, shared: bool) -> &mut Self { ... }

pub fn finish(self) -> &mut StructTypeBuilder<'_> { ... }
}
```

Wasmtime GitHub notifications bot (Jun 29 2026 at 09:39):

somdoron updated PR #13687.

Wasmtime GitHub notifications bot (Jun 29 2026 at 09:49):

somdoron commented on PR #13687:

Thanks, I have a workaround for now (similar to what you suggested).

I applied your suggestion, using a builder API for struct/array/func. I haven't built the ForwardRefFieldBuilder yet, I went with an API similar to StructType::new, taking both nullability and mutability as parameters. It does lack the shared field at the moment. Let me know if you think the field builder API is preferred.

Wasmtime GitHub notifications bot (Jun 29 2026 at 16:19):

fitzgen commented on PR #13687:

I applied your suggestion, using a builder API for struct/array/func. I haven't built the ForwardRefFieldBuilder yet, I went with an API similar to StructType::new, taking both nullability and mutability as parameters. It does lack the shared field at the moment. Let me know if you think the field builder API is preferred.

Yeah the builder API is preferred because it is less brittle as the Wasm spec evolves (e.g. with the addition of shared to reference types).

Wasmtime GitHub notifications bot (Jun 29 2026 at 16:38):

somdoron updated PR #13687.

Wasmtime GitHub notifications bot (Jun 29 2026 at 16:42):

somdoron updated PR #13687.

Wasmtime GitHub notifications bot (Jun 29 2026 at 16:53):

somdoron updated PR #13687.

Wasmtime GitHub notifications bot (Jun 29 2026 at 16:58):

somdoron commented on PR #13687:

Done, now with the builder API for fields / array elements / function params.

I didn't add shared to any of the builders yet, as it isn't supported yet and I think it depends on the shared-everything-threads proposal.

Wasmtime GitHub notifications bot (Jun 30 2026 at 18:51):

:memo: fitzgen submitted PR review:

Thanks, I think we are on the right track. Some more comments below.

(Don't want to start getting too detailed with the review until the skeleton / overall design is in the final shape.)

Wasmtime GitHub notifications bot (Jun 30 2026 at 18:51):

:speech_balloon: fitzgen created PR review comment:

This isn't true at the spec level, and is easy to support (just remove this assertion and everything should work, AFAICT) so I don't think we should require it.

Wasmtime GitHub notifications bot (Jun 30 2026 at 18:51):

:speech_balloon: fitzgen created PR review comment:

Rather than duplicating all these types for the forward-or-not cases, could we reuse the types from wasmtime_environ::types::* and have the not-forward cases be EngineOrModuleTypeIndex::Engine and references within the rec group be EngineOrModuleTypeIndex::RecGroup or something like that?

Wasmtime GitHub notifications bot (Jun 30 2026 at 18:51):

:speech_balloon: fitzgen created PR review comment:

Re: my previous comment, we would ideally make this become
    inner: WasmRecGroup,
Although unfortunately that would require defining all struct vs array vs function types up front for all the members, leading to an awkward API for the builder, so I guess we probably actually want something like this:
    members: Vec<Option<WasmSubType>>,
Although I guess we could do the first if we had different RecGroupBuilder::declare_struct vs RecGroupBuilder::declare_array vs RecGroupBuilder::decalre_func, as you originally formulated it... I didn't realize the implications that change would have here. I think we actually do want to have those separately-typed declare methods so we can have this builder wrap a WasmRecGroup. Sorry for the churn.

Wasmtime GitHub notifications bot (Jun 30 2026 at 18:51):

:speech_balloon: fitzgen created PR review comment:

This should document that the order of declare calls determines the order that the types are defined within the rec group, which has semantically visible implications. In the following example, $f != $f' and $s != $s', but you might otherwise not realize this when using the RecGroupBuilder, declare, and PendingType.
(rec (type $f (func))
     (type $s (struct)))

(rec (type $s' (struct))
     (type $f' (func)))

Wasmtime GitHub notifications bot (Jun 30 2026 at 20:41):

:memo: somdoron submitted PR review.

Wasmtime GitHub notifications bot (Jun 30 2026 at 20:41):

:speech_balloon: somdoron created PR review comment:

I think Vec<Option<WasmSubType>> might be better. The reason is that TypeRegistryInner::register_rec_group_types takes an iterator of WasmSubType, so we don't get much benefit from wrapping a WasmRecGroup here.

In any case, we can't construct the underlying WasmSubTypes for the WasmRecGroup until the struct/array/func builder finishes. We could create a placeholder of the correct composite kind when declare_* is called, but we'd still have to replace it once the struct/array/func builder finishes. Since that overwrite is needed either way, I don't think the separately-typed declare methods are required to wrap a WasmRecGroup — unless you'd prefer them for API reasons.

Wasmtime GitHub notifications bot (Jun 30 2026 at 20:46):

:memo: somdoron submitted PR review.

Wasmtime GitHub notifications bot (Jun 30 2026 at 20:46):

:speech_balloon: somdoron created PR review comment:

Correction: we do need the separate declare functions — they're required to build the WasmHeapType for a forward reference, whose concrete variant (ConcreteStruct / ConcreteArray / ConcreteFunc) encodes the composite kind. So the kind has to be known at declare time, even though the body isn't filled in yet.

Wasmtime GitHub notifications bot (Jun 30 2026 at 21:13):

:memo: somdoron submitted PR review.

Wasmtime GitHub notifications bot (Jun 30 2026 at 21:13):

:speech_balloon: somdoron created PR review comment:

Some conclusions after digging in:

We can use multiple declare functions, one per type.

We can use WasmHeapType with EngineOrModuleTypeIndex::Engine and EngineOrModuleTypeIndex::Module for forward references. That would remove FieldDef, ValDef, and SuperDef.

Using WasmRecGroup or Vec<WasmSubType> for the RecGroupBuilder's internal type isn't very elegant, since they aren't mutable.

We can use Vec<WasmSubType>: create a placeholder on declare_*, and create the real type when we call RecGroupBuilder::build or Array/Func/StructBuilder::finish.

Anyway, I think keeping the MemberDef enum is more elegant than trying to use WasmRecGroup or Vec<WasmSubType>, since it's mutable and allows accumulation.

Wasmtime GitHub notifications bot (Jul 01 2026 at 07:46):

somdoron updated PR #13687.

Wasmtime GitHub notifications bot (Jul 01 2026 at 07:58):

somdoron commented on PR #13687:

The new commit stores Vec<Option<WasmSubType>> instead of a MemberDef. Each StructTypeBuilder/ArrayTypeBuilder/FuncTypeBuilder must have its finish method called to commit the type to the RecGroupBuilder, consistent with ForwardRefFieldBuilder. If finish is never called on one of these builders, that type's definition is silently dropped.

At the moment only RecGroupBuilder::build reports errors: the first error is stored on the RecGroupBuilder and returned when build is called. We could instead have StructTypeBuilder/ArrayTypeBuilder/FuncTypeBuilder::finish return the error directly.

For example, two structs where $a has a forward reference to $b:

```rust
let mut builder = RecGroupBuilder::new(&engine);

let a = builder.declare_struct();
let b = builder.declare_struct();

builder
.define_struct(a)
.forward_ref_field(b)
.nullable(true)
.finish() // commit the field, back to the struct builder
.finish(); // commit the struct to the rec group

builder
.define_struct(b)
.field(FieldType::new(
Mutability::Const,
StorageType::ValType(ValType::I32),
))
.finish();

let group = builder.build()?;
let a: StructType = group.get_struct(a).unwrap();
let b: StructType = group.get_struct(b).unwrap();
```

Wasmtime GitHub notifications bot (Jul 01 2026 at 07:58):

somdoron edited a comment on PR #13687:

The new commit stores Vec<Option<WasmSubType>> instead of a MemberDef. Each StructTypeBuilder/ArrayTypeBuilder/FuncTypeBuilder must have its finish method called to commit the type to the RecGroupBuilder, consistent with ForwardRefFieldBuilder. If finish is never called on one of these builders, that type's definition is silently dropped.

At the moment only RecGroupBuilder::build reports errors: the first error is stored on the RecGroupBuilder and returned when build is called. We could instead have StructTypeBuilder/ArrayTypeBuilder/FuncTypeBuilder::finish return the error directly.

For example, two structs where $a has a forward reference to $b:
let mut builder = RecGroupBuilder::new(&engine);

let a = builder.declare_struct();
let b = builder.declare_struct();

builder
    .define_struct(a)
    .forward_ref_field(b)
    .nullable(true)
    .finish()  // commit the field, back to the struct builder
    .finish(); // commit the struct to the rec group

builder
    .define_struct(b)
    .field(FieldType::new(
        Mutability::Const,
        StorageType::ValType(ValType::I32),
    ))
    .finish();

let group = builder.build()?;
let a: StructType = group.get_struct(a).unwrap();
let b: StructType = group.get_struct(b).unwrap();

Wasmtime GitHub notifications bot (Jul 01 2026 at 08:03):

:memo: somdoron submitted PR review.

Wasmtime GitHub notifications bot (Jul 01 2026 at 08:03):

:speech_balloon: somdoron created PR review comment:

done, added documentation to each declare function and RecGroupBuilder.

Wasmtime GitHub notifications bot (Jul 01 2026 at 08:04):

:memo: somdoron submitted PR review.

Wasmtime GitHub notifications bot (Jul 01 2026 at 08:04):

:speech_balloon: somdoron created PR review comment:

fixed

Wasmtime GitHub notifications bot (Jul 07 2026 at 15:04):

:memo: fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Jul 07 2026 at 15:04):

:speech_balloon: fitzgen created PR review comment:

Using WasmRecGroup or Vec<WasmSubType> for the RecGroupBuilder's internal type isn't very elegant, since they aren't mutable.

We can add mutation methods to these types as necessary and change Box<[T]> to Vec<T> where needed. Given that, I'd prefer some combination of (3) and (4) over effectively duplicating the type hierarchy and adding mutation methods to one of the duplicate sets of types.

Wasmtime GitHub notifications bot (Jul 07 2026 at 15:12):

fitzgen commented on PR #13687:

If finish is never called on one of these builders, that type's definition is silently dropped.

I think this should become an error when RecGroupBuilder::build is called, rather than silently dropping types.

Also we should probably be consistent with finish vs build for RecGroupBuilder and StructTypeBuilder etc... I'm leaning towards build because we use that for MemoryTypeBuilder already.

At the moment only RecGroupBuilder::build reports errors: the first error is stored on the RecGroupBuilder and returned when build is called. We could instead have StructTypeBuilder/ArrayTypeBuilder/FuncTypeBuilder::finish return the error directly.

This is fine. Intermediate builder errors end up being annoying in practice, and can't cover everything so you need the final error anyways, so its often best to just delay all validation to the end and only have the final method be fallible.

Wasmtime GitHub notifications bot (Jul 07 2026 at 15:22):

:memo: fitzgen submitted PR review:

Thanks for your ongoing patience with this PR!

Wasmtime GitHub notifications bot (Jul 07 2026 at 15:22):

:speech_balloon: fitzgen created PR review comment:

Let's share a constant rather than have a comment saying it is / should be the same.

Wasmtime GitHub notifications bot (Jul 07 2026 at 15:22):

:speech_balloon: fitzgen created PR review comment:

Is there a reason we can't just delay all error checking until RecGroupBuilder::build? That would be cleaner.

Wasmtime GitHub notifications bot (Jul 07 2026 at 15:22):

:speech_balloon: fitzgen created PR review comment:

If we have members: Vec<WasmSubType> in RecGroupBuilder and initialize the entries to the appropriate default WasmCompositeTypeInner variant, then we shouldn't need this anymore. We can just match on the WasmCompositeTypeInner in define_struct et al instead.

Wasmtime GitHub notifications bot (Jul 08 2026 at 18:37):

:memo: somdoron submitted PR review.

Wasmtime GitHub notifications bot (Jul 08 2026 at 18:37):

:speech_balloon: somdoron created PR review comment:

The accumulated errors are only the cross-engine checks, and those can't be delayed: the builder immediately lowers everything into WasmSubType, where an engine reference is a bare VMSharedTypeIndex with no engine identity — by build() time a foreign engine's index is indistinguishable from a valid one in this engine. Delaying the check would mean holding onto the FieldType/ValType/supertype handles until build(), which contradicts building directly into the wasmtime-environ types instead of keeping our own builder-side representation.

Wasmtime GitHub notifications bot (Jul 08 2026 at 18:37):

somdoron requested fitzgen for a review on PR #13687.

Wasmtime GitHub notifications bot (Jul 08 2026 at 18:37):

somdoron requested wasmtime-compiler-reviewers for a review on PR #13687.

Wasmtime GitHub notifications bot (Jul 08 2026 at 18:37):

somdoron updated PR #13687.

Wasmtime GitHub notifications bot (Jul 08 2026 at 18:39):

:memo: somdoron submitted PR review.

Wasmtime GitHub notifications bot (Jul 08 2026 at 18:39):

:speech_balloon: somdoron created PR review comment:

done, I added mutation to those types. A separate commit just for those changes:
https://github.com/bytecodealliance/wasmtime/pull/13687/changes/57b22da48968a9f136f606c328405defed8b8511

Wasmtime GitHub notifications bot (Jul 08 2026 at 18:39):

:memo: somdoron submitted PR review.

Wasmtime GitHub notifications bot (Jul 08 2026 at 18:39):

:speech_balloon: somdoron created PR review comment:

done

Wasmtime GitHub notifications bot (Jul 08 2026 at 18:39):

:memo: somdoron submitted PR review.

Wasmtime GitHub notifications bot (Jul 08 2026 at 18:39):

:speech_balloon: somdoron created PR review comment:

done

Wasmtime GitHub notifications bot (Jul 08 2026 at 18:42):

somdoron commented on PR #13687:

I think this should become an error when RecGroupBuilder::build is called, rather than silently dropping types.

How do you feel about define_struct/forward_ref_field/etc. taking ownership of the builder? Then not calling build would be a compile error rather than a runtime one, and it all reads as a single chain.

The one downside is the config methods become self -> Self, so building fields in a loop needs x = x.field(..).

let mut builder = RecGroupBuilder::new(&engine);
let a = builder.declare_struct();
let b = builder.declare_struct();
let group = builder
    .define_struct(a).forward_ref_field(b).nullable(true).build().build()
    .define_struct(b).forward_ref_field(a).nullable(false).build().build()
    .build()?;
 ```

Anyway, now that the environ types are mutable, `build` actually does nothing — it only returns the parent builder to continue the chain. I didn't apply the ownership change.

> Also we should probably be consistent with finish vs build for RecGroupBuilder and StructTypeBuilder etc... I'm leaning towards build because we use that for MemoryTypeBuilder already.

I changed it to `build` across the board, but I think there's a small difference: only `RecGroupBuilder` is actually building something. For the intermediate builders, `finish` reads nicer — all they do is return the parent builder, and even if they did add the type/field themselves, they still wouldn't be building the full type yet. Anyway, whatever you prefer.


````

Wasmtime GitHub notifications bot (Jul 08 2026 at 18:43):

somdoron updated PR #13687.

Wasmtime GitHub notifications bot (Jul 08 2026 at 19:05):

somdoron updated PR #13687.

Last updated: Jul 29 2026 at 05:03 UTC