Skip to content

feat: Full support for GC extension#304

Open
logan-gatlin wants to merge 17 commits intomainfrom
full-gc-ext
Open

feat: Full support for GC extension#304
logan-gatlin wants to merge 17 commits intomainfrom
full-gc-ext

Conversation

@logan-gatlin
Copy link
Contributor

@logan-gatlin logan-gatlin commented Mar 12, 2026

Type System Overhaul

Type was previously a flat wrapper around function signatures. It's now a subtype node wrapping a CompositeType enum. Type::params() and Type::results() still work but panic on non-function types. New code should use kind(), as_function(), as_struct(), as_array().

Types can now reference each other by type index, or by rec group relative type index. HeapType::Concrete now holds a TypeId (arena allocated) instead of just a u32. Most functions that convert types now must pass a IdsToIndicies/IndicesToId, and sometimes a rec_group_start u32 index.

Recursive Type Groups

Rec groups are the hardest part of GC — types within a group can reference each other, requiring forward references. The solution is a placeholder-then-finalize pattern:

  1. Pre-allocate TypeIds with dummy placeholder values
  2. Parse all types in the group, using the pre-allocated IDs for forward references
  3. Overwrite placeholders with real definitions
    ArenaSet gained new methods to support this: alloc_unique() (allocate without dedup), replace_and_register() (overwrite + register in dedup map), find() (lookup without insert).

Singletons (every non-recursive type) are deduplicated, but explcit multi-type groups are never deduplicated per the spec. Emission is now rec-group-ordered instead of flat/sorted.

ModuleType::delete now has a warning that the user is responsible for checking if types in the same rec group reference the deleted type. Added two new delete methods:

  • try_delete - Same as delete, but fails if another type in the same rec group depends on the type to be deleted
  • delete_entire_group - Helper function that deletes a type, along with every other type in its rec group.

GC Liveness Pass

Significantly extended with three new phases:

  1. Manual visitor overrides for instructions with HeapType fields (ref.test, ref.cast, br_on_cast, etc.) and locals with ref types — marking concrete type IDs as used.
  2. Transitive type closure — after the main worklist drains, follows all TypeId references within used types (struct fields, array elements, function params/results, supertypes).
  3. Rec group atomicity — if any type in a rec group is used, all sibling types are marked used, matching the spec requirement that rec groups are indivisible.

Tests

All GC spec tests (except those that rely on other proposals) are now enabled. A new fuzzer wasm-smith has been added, since WABT does not have proper support for GC yet.

@logan-gatlin logan-gatlin marked this pull request as ready for review March 13, 2026 17:38
Copy link
Contributor

@guybedford guybedford left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI-Assisted Review: PR #304 — Full GC Extension Support

Note: This review was generated with AI assistance (Claude). Findings should be verified by a human reviewer.

Build & Test Status

All tests pass locally:

  • 30 unit tests
  • 105 round-trip tests (35 GC-specific)
  • 267 spec tests (6 known-failing, all justified)
  • 18 doc-tests

Overall Assessment

This is a high-quality, well-structured PR. The type system overhaul, rec group handling (placeholder-then-finalize pattern), and three-phase GC liveness pass are architecturally sound. Test coverage is excellent with 17+ dedicated GC round-trip WAT files, thorough builder API tests, and a new wasm-smith fuzzer. The removal of ~30 entries from the known-failing list is well-justified.

See inline comments for specific findings, summarized here by severity:

Bugs (should fix):

  1. ConstExpr::Extended silently ignored in element segments, data/elem offsets, and table init (GC liveness -- types/funcs/globals may be incorrectly collected)
  2. Ord impl on Type is inconsistent with PartialEq/Hash (missing is_for_function_entry)
  3. delete() doesn't clean up rec groups (partial deletion silently drops live siblings)

Design concerns (should address or document):
4. HeapType::Exact collapsed into Concrete -- loses exactness info on round-trip
5. shared flag silently discarded -- shared composite types accepted but emitted as unshared
6. #[walrus(skip_visit)] on HeapType fields is fragile -- new instructions need manual visitor overrides

Nits:

  • RecGroupId defined/exported but never used
  • rec_group_for_type is O(n*m) -- consider reverse index
  • Stale "Phase 3" comment in gc-types-parse.rs
  • Fuzzer fixed seed limits exploration; max_types=10 may be small for GC

@logan-gatlin
Copy link
Contributor Author

logan-gatlin commented Mar 16, 2026

Split the fuzzer runs into two jobs since adding wasm-smith caused it to consistently time out. On the bright side, wasm-smith is WAY faster than the other fuzzers. wasm-opt-ttf gets about 1000 runs a minute for me while wasm-smith does 150k.

@logan-gatlin
Copy link
Contributor Author

logan-gatlin commented Mar 19, 2026

Our very old version of WABT had a memory corruption bug with recursive calls, which I believe was causing watgen CI to run forever. I bumped the dependency version and added a manual timeout in Rust.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants