Refactor reference types in the C/C++ API#13154
Merged
alexcrichton merged 8 commits intobytecodealliance:mainfrom Apr 21, 2026
Merged
Refactor reference types in the C/C++ API#13154alexcrichton merged 8 commits intobytecodealliance:mainfrom
alexcrichton merged 8 commits intobytecodealliance:mainfrom
Conversation
fitzgen
approved these changes
Apr 21, 2026
Member
fitzgen
left a comment
There was a problem hiding this comment.
LGTM
I think in the few other C++ codebases I've touched, the _foo.hh would be foo.hh and the existing foo.hh would be foo-inlines.hh but I don't really care what color we paint the bikeshed.
In updating `wasmtime-py` to Wasmtime 44 I noticed a few discrepancies in the C API, and then C++ API, in addition to what was found in bytecodealliance#13149. Notably we had both `wasmtime_exn_t`/`Exn` and `wasmtime_exnref_t`/`ExnRef`. In sorting this out with some deeper fixes in bytecodealliance#13149 I got stuck again in the rat's nest of circular definitions of classes in C++. In the end I gave up trying to incrementally improve things and decided to go with a larger refactoring. The goal here is to more cleanly separate out responsibilities and improve how circular definitions in C++ are handled: * All reference types now live in their own file. Reference types aren't in a mix of `val.h` or `gc.h` or `exn.h`, but instead each type lives in its own file. This means that `gc.h` is now gone and is replaced by `anyref.h`, `externref.h`, `exnref.h`, `eqref.h`, ... * C++ class definitions are split into separate headers from their main files. For example there's now `wasmtime/_anyref_class.hh` instead of just `wasmtime/anyref.hh`. This new method of resolving circular dependencies has some nice properties where headers that depend on the `AnyRef` type, for example, include `_anyref_class.h`. The `anyref.h` header then is able to include all other headers necessary for defining methods. Notably this means that methods now always live in the header of the type as opposed to sprinkled about in various locations. Note that this is applied to preexisting headers like `func.hh` and `store.hh` out of necessity, too. * Reference types in the C++ now share core methods via a macro to avoid duplicating code and to ensure they have a uniform API. * Some `#define`s for `WASMTIME_FEATURE_GC` guards were adjusted in a few locations. For example `wasmtime_valraw_t` no longer has GC fields when the GC proposal is disabled. * Reorganization was reflected on the Rust side as well. This means the previous `ref.rs` is now broken up into files such as `anyref.rs`. Some logic of core functions was also deduplicated within the `ref_wrapper!` macro. The old definitions of `wasmtime_exn_t` and `wasmtime_exnref_t` were also both merged. Overall this PR contains quite a lot of code movement and things were shuffled around, as well as adjustments to the `exnref` C API (removing duplication). Overall though the API is largely as it was before, although for the C++ API if individual headers are being included some new ones may need to be included to ensure all methods are defined. My hope is that the end result here is a bit more maintainable, types are easier to modify/extend, there's no longer any soups to deal with in C++, and everything should have a uniform API.
prtest:full
e989d4f to
03d074c
Compare
Member
Author
|
Oh nice! I'll be honest in that I don't really have much experience in C++ codebases. I'm going to flag this for merge mostly out of inertia, but I'm also happy to adjust to that pattern next time I'm in these parts. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In updating
wasmtime-pyto Wasmtime 44 I noticed a few discrepancies in the C API, and then C++ API, in addition to what was found in #13149. Notably we had bothwasmtime_exn_t/Exnandwasmtime_exnref_t/ExnRef. In sorting this out with some deeper fixes in #13149 I got stuck again in the rat's nest of circular definitions of classes in C++. In the end I gave up trying to incrementally improve things and decided to go with a larger refactoring. The goal here is to more cleanly separate out responsibilities and improve how circular definitions in C++ are handled:All reference types now live in their own file. Reference types aren't in a mix of
val.horgc.horexn.h, but instead each type lives in its own file. This means thatgc.his now gone and is replaced byanyref.h,externref.h,exnref.h,eqref.h, ...C++ class definitions are split into separate headers from their main files. For example there's now
wasmtime/_anyref_class.hhinstead of justwasmtime/anyref.hh. This new method of resolving circular dependencies has some nice properties where headers that depend on theAnyReftype, for example, include_anyref_class.h. Theanyref.hheader then is able to include all other headers necessary for defining methods. Notably this means that methods now always live in the header of the type as opposed to sprinkled about in various locations. Note that this is applied to preexisting headers likefunc.hhandstore.hhout of necessity, too.Reference types in the C++ now share core methods via a macro to avoid duplicating code and to ensure they have a uniform API.
Some
#defines forWASMTIME_FEATURE_GCguards were adjusted in a few locations. For examplewasmtime_valraw_tno longer has GC fields when the GC proposal is disabled.Reorganization was reflected on the Rust side as well. This means the previous
ref.rsis now broken up into files such asanyref.rs. Some logic of core functions was also deduplicated within theref_wrapper!macro. The old definitions ofwasmtime_exn_tandwasmtime_exnref_twere also both merged.Overall this PR contains quite a lot of code movement and things were shuffled around, as well as adjustments to the
exnrefC API (removing duplication). Overall though the API is largely as it was before, although for the C++ API if individual headers are being included some new ones may need to be included to ensure all methods are defined.My hope is that the end result here is a bit more maintainable, types are easier to modify/extend, there's no longer any soups to deal with in C++, and everything should have a uniform API.