Add Component Model type-hierarchy skeleton + reorganize tests#282
Conversation
Per the slice plan worked out on PR bytecodealliance#281's review thread, this lays the public Go-side type-information surface so that a future slice can expose ComponentFunc + Call as a thin consumer of these public types rather than calling wasmtime_component_func_type directly. Adds (skeleton only — downcasts kept narrow): - Component now stores its [Engine] in a public `Engine *Engine` field, matching the precedent set by Linker and Store. Required because the component-type APIs (`wasmtime_component_type_import_count` etc.) take an engine. - Component.Type() returns a *ComponentType. - ComponentType: ImportCount / ExportCount / ImportNth / ExportNth. Each Nth returns (name, *ComponentItem). Out-of-range yields ("", nil). - ComponentItem: Kind() returns one of seven ComponentItemKind* constants. TypeAlias() downcasts to a *ComponentValType when the kind is Type; returns nil for any other kind. Downcasts for the remaining six kinds (component / instance / module / func / resource / core-func) are deferred to follow-up slices. - ComponentValType: Kind() returns one of 27 ComponentValTypeKind* constants covering both primitive and composite valtype kinds. Composite-kind sub-type wrappers (list / record / tuple / variant / enum / option / result / flags / map / future / stream / own / borrow / error-context) are deferred to follow-up slices. Tests added: TestComponentType / ImportExportCount / ExportNth / ItemTypeAlias / ItemTypeAliasReturnsNilForNonTypeKind. The last exercises ComponentValType through a `(component (type $a u32) (export "my-id" (type $a)))` fixture, which is the only public path to a ComponentValType in this slice. All tests pass under default and -tags debug; gofmt and vet clean.
fdb3a48 to
e3315bd
Compare
| (func (export "hello") (canon lift (core func $i "hello")))) | ||
| ` | ||
|
|
||
| func TestComponentNew(t *testing.T) { |
There was a problem hiding this comment.
moved to: component_feat_component_model.go
| // the engine must outlive the component. Held privately so the field | ||
| // does not commit the package to a public lifecycle contract; can be | ||
| // promoted later if a user-facing need appears. | ||
| engine *Engine |
There was a problem hiding this comment.
Flagging one design choice that wasn't obvious:
Component now holds a private engine *Engine field. The type-info C APIs (wasmtime_component_type_import_count etc.) require an engine,
so it has to live somewhere. Three options I considered:
- Private field on
Component← chosen. Keeps the user-facing API quiet; promoting to public later is non-breaking. - Method parameter (wasmtime-py's approach). I went back and forth on this one — speaking as a Go user, having
enginethread through
every type-info call (c.Type().ImportCount(engine)) felt unwieldy versus keeping it as constructor-time state. That was the deciding factor. - Public
Engine *Engine, matching theLinker/Storeprecedent. Closest analog but commits to a public lifecycle contract before a
concrete user need.
Related: the same question applies to Linker.Engine and Store.Engine. Considering handling those in this PR for consistency, pending the
call on Component here.
There was a problem hiding this comment.
Agreed yeah putting it in a field is reasonable, while the C API has various constraints I think it's fine for languages to extend that and refine the constraints as best fit, and here in Go there's little cost to carrying more references to the engine around.
alexcrichton
left a comment
There was a problem hiding this comment.
Thanks! I'll go ahead and merge, but I'd like you to ask your local LLM to dial back on the verbosity here. The comments are a bit too precise about the exact structure of code and not always applicable. The testing here has quite a lot of verbose comments and some (personally I find) strange structures. Tests and comments are important, don't get me wrong, but to me it's a bit over-the-top ehre.
| cases := []struct { | ||
| name string | ||
| wat string | ||
| wantExportName string | ||
| }{ | ||
| { | ||
| "hello_export", | ||
| `(component | ||
| (core module $m (func (export "hello"))) | ||
| (core instance $i (instantiate $m)) | ||
| (func (export "hello") (canon lift (core func $i "hello"))))`, | ||
| "hello", | ||
| }, | ||
| { | ||
| "world_export", | ||
| `(component | ||
| (core module $m (func (export "world"))) | ||
| (core instance $i (instantiate $m)) | ||
| (func (export "world") (canon lift (core func $i "world"))))`, | ||
| "world", | ||
| }, | ||
| } | ||
| for _, tc := range cases { | ||
| t.Run(tc.name, func(t *testing.T) { | ||
| engine := newComponentEngine() | ||
| component := newComponent(t, engine, tc.wat) | ||
| defer component.Close() | ||
|
|
||
| bytes, err := component.Serialize() | ||
| require.NoError(t, err) | ||
| require.NotEmpty(t, bytes) | ||
|
|
||
| round, err := NewComponentDeserialize(engine, bytes) | ||
| require.NoError(t, err) | ||
| defer round.Close() | ||
| require.NotNil(t, round.GetExportIndex(nil, tc.wantExportName)) | ||
| }) | ||
| } |
There was a problem hiding this comment.
For future PRs and this isn't necessarily a huge issue, but personally I find this style of test to be pretty confusing. Each test has a cases list and a set of fields/sub-tests that needs to be understood in addition to reading over the test itself. Personally I think it'd be simpler to just go ahead and duplicate the body of the test as necessary as separate test cases or similar. I don't see much need in being super-DRY here for example and some duplication is fine.
| wasm, err := Wat2Wasm(helloComponent) | ||
| // newComponent compiles `wat` into a fresh [Component] for tests. The | ||
| // returned component must be closed by the caller. | ||
| func newComponent(t *testing.T, engine *Engine, wat string) *Component { |
There was a problem hiding this comment.
Similar to above, could this be inlined directly within tests? I find it a bit confusing to have newComponent and NewComponent and trying to remember which to call.
|
Thank you for the review. |
Pre-existing wasmtime-go test files (e.g. functype_test.go, importtype_test.go, valtype_test.go) follow a "one test per type, multiple inline assertions" pattern. The previous V1 layout (14 tests split by kind x variation axis with OutOfRange / ReturnsNilForNon<X>Kind suffixes) was modeled on the recently-merged bytecodealliance#282 component tests, which have diverged from that pre-existing canonical style. Collapse to 5 tests, one per composite kind. Each test covers happy path + index-out-of-range (where applicable) + wrong-kind-returns-nil inline. Net: 224 -> 65 lines. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…m/flags Continues the type-hierarchy work from bytecodealliance#282 by exposing payload accessors for the product/enum composite kinds: list, record, tuple, enum, flags. Each ships as a Go wrapper type with the C API's count/nth-style accessors (plus list/record's element/field types via independent ComponentValType clones, matching ComponentItem.TypeAlias()): - ComponentListType (Element) - ComponentRecordType (FieldCount / FieldNth) - ComponentTupleType (TypesCount / TypesNth) - ComponentEnumType (NamesCount / NamesNth) - ComponentFlagsType (NamesCount / NamesNth) ComponentValType gains a downcast method per kind (List, Record, Tuple, Enum, Flags) returning nil for any other kind. The corresponding five ComponentValTypeKind* constants -- commented out in bytecodealliance#282 -- are now uncommented. Out of scope for this slice (per the roadmap split): variant / option / result (sum types, deferred to share a focused PR with their value marshaling), resource own/borrow, future/stream/error-context/map, and value-side marshaling for the kinds added here. Verified: go build ./..., go test ./..., go test -tags debug ./..., go vet ./..., gofmt -l . -- all clean. 14 new test functions.
Pre-existing wasmtime-go test files (e.g. functype_test.go, importtype_test.go, valtype_test.go) follow a "one test per type, multiple inline assertions" pattern. The previous V1 layout (14 tests split by kind x variation axis with OutOfRange / ReturnsNilForNon<X>Kind suffixes) was modeled on the recently-merged bytecodealliance#282 component tests, which have diverged from that pre-existing canonical style. Collapse to 5 tests, one per composite kind. Each test covers happy path + index-out-of-range (where applicable) + wrong-kind-returns-nil inline. Net: 224 -> 65 lines.
This PR does two things:
Component.Type(),ComponentType,ComponentItem,ComponentValType.Kind()accessors only; primitive ValType kinds only.<source>_test.gofiles now mirror production 1:1, no cross-file fixture sharing.Follow-up to #281 (merged). Tracks #280.
What's added
Component.Type()returns a*ComponentTypefor walking imports/exports.ComponentItemis a discriminated union (kind tag + payload).Kind()discriminates 7 item kinds;TypeAlias()returns the embeddedComponentValTypewhen the kind isType(nil otherwise). Payload accessors for the remaining six kinds (component / component-instance / module / component-func / resource / core-func) are deferred to later slices.ComponentValType.Kind()exposes the 13 primitive WIT kinds (bool,s8–s64,u8–u64,f32/f64,char,string). Constants for the 14 composite kinds are commented out — they will be uncommented when each gets a payload accessor that returns the corresponding sub-type wrapper, with a test path.Out of scope (next slices)
Payload accessors for non-
TypeComponentItemkinds, sub-type wrappers for compositeComponentValTypekinds, andComponentFunc+ value marshaling — deferred to follow-up slices.Verified
go build ./...,go test ./...(default +-tags debug),gofmt -l .,go vet ./...all clean. 14 test functions / 28 sub-tests including a 13-caseTestComponentValTypeKindForEachPrimitiveand a 3-case multi-type walk.