fix: support named fixed-length list types in code generation#1537
Conversation
sumleo
force-pushed
the
fix/named-fixed-length-list
branch
2 times, most recently
from
b929b31 to
9bc1675
Compare
crates/core/src/lib.rs
Outdated
| fn type_fixed_length_list( | ||
| &mut self, | ||
| id: TypeId, | ||
| name: &str, | ||
| ty: &Type, | ||
| size: u32, | ||
| docs: &Docs, | ||
| ) { | ||
| let _ = (id, name, ty, size, docs); | ||
| todo!("named fixed-length list types are not yet supported in this backend") | ||
| } |
There was a problem hiding this comment.
Could this default implementation be omitted and the todo! placed in bindings generators downstream that don't support this?
There was a problem hiding this comment.
Done. Removed the default implementation and added explicit type_fixed_length_list to all backends:
- Rust: generates
pub type Name = [T; N];(already existed) - Go: generates
type Name = [N]Type - Markdown: delegates to
type_alias - Moonbit: no-op (maps to
FixedArray[T]natively) - C, C++, C#: explicit
todo!()with expected test failures
crates/rust/tests/codegen.rs
Outdated
| "#, | ||
| generate_all, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Could this be added to tests/codegen/*.wit?
There was a problem hiding this comment.
Done. Moved the test to tests/codegen/named-fixed-length-list.wit and removed the inline version from crates/rust/tests/codegen.rs. Added expected failures in the test infrastructure for C, C++, and C# backends that don't yet support this.
sumleo
force-pushed
the
fix/named-fixed-length-list
branch
from
9bc1675 to
de6a089
Compare
alexcrichton
left a comment
alexcrichton
left a comment
There was a problem hiding this comment.
Thanks! Mind rebasing this as well to get this added to the merge queue?
sumleo
force-pushed
the
fix/named-fixed-length-list
branch
2 times, most recently
from
f56e45a to
41a75aa
Compare
| } | ||
|
|
||
| // Named fixed-length lists don't work with async yet. | ||
| if name == "named-fixed-length-list.wit-async" { |
There was a problem hiding this comment.
I would be curious to see the error (message) as these features should be orthogonal. Should be easy to fix (as should be a C++ implementation using std::array<>, C might be more tricky as it could require adding a struct).
There was a problem hiding this comment.
The error is a todo\!() panic in deallocate for FixedLengthList at crates/core/src/abi.rs:2407 - the async code path calls deallocate_lists_in_types which hits the unimplemented deallocation. This is addressed by #1538, so once that lands this expected failure can be removed.
thread 'main' panicked at crates/core/src/abi.rs:2407:53:
not yet implemented
3: wit_bindgen_core::abi::Generator<B>::deallocate
6: wit_bindgen_rust::interface::InterfaceGenerator::deallocate_lists
7: wit_bindgen_rust::interface::InterfaceGenerator::generate_guest_import_body_async
Re C++: agreed, std::array<> would work directly. I marked it as expected failure for now since this PR focuses on the named type alias dispatch, but a follow-up to implement it in C/C++/C# would be straightforward.
The `define_type` function dispatches to `type_fixed_length_list` on the `InterfaceGenerator` trait for named fixed-length list types like `type my-array = list<u32, 4>`. This makes `type_fixed_length_list` a required trait method (no default) and adds implementations in all backends: - Rust: generates `pub type Name = [T; N];` - Go: generates `type Name = [N]Type` - Markdown: delegates to `type_alias` - Moonbit: no-op (maps to `FixedArray[T]` natively) - C, C++, C#: explicit `todo!()` (not yet supported) Also adds a shared codegen test in `tests/codegen/named-fixed-length-list.wit` with expected failures for backends that don't yet support this.
sumleo
force-pushed
the
fix/named-fixed-length-list
branch
from
41a75aa to
3251160
Compare
3 participants
Summary
define_typefunction in the coreInterfaceGeneratortrait had atodo!()forFixedLengthList, causing a panic when WIT files contained named fixed-length list types liketype my-array = list<u32, 4>type_fixed_length_listmethod to theInterfaceGeneratortrait with a default implementationtype_listpattern, generating type aliases likepub type MyArray = [u32; 4];define_typedispatch to call the new methodTest plan
tests/codegen/named-fixed-length-list.witwith named fixed-length list types in both import and export positionscrates/rust/tests/codegen.rsverifying Rust codegen compiles