Conversation
| #[cfg(feature = "std")] | ||
| pub type Map<K, V> = std::collections::HashMap<K, V>; | ||
| #[cfg(not(feature = "std"))] | ||
| pub type Map<K, V> = alloc::collections::BTreeMap<K, V>; |
There was a problem hiding this comment.
This I'm hesitant to do myself, specifically switching based on std. That's not API-compatible unfortunately and puts this crate in a weird situation.
One possibility perhaps is to force some sort of configuration in bindgen! if map<K, V> is encountered which specifies which map type to use. Another is to always fall back to the more general BTreeMap in terms of availability. Another is to support both a fallback and a configuration. Another yet might be to avoid using HashMap and BTreeMap entirely and instead use Vec<(T, U)> by default. Overall I'm not sure what the best option is...
There was a problem hiding this comment.
@alexcrichton honestly, I do not feel qualify to decide, until now, I have been "pattern matching" and coding based on the List type. Digging more into Rust details, here is the finding thus far based on AI, so I need to triple verify, I may delay this topic and try to make things to work first, and swap this later
I think BTreeMap is actually the right default, and the current choice is fine for now. Here's my reasoning:
WIT key types are constrained. Keys in a WIT map won't be arbitrary Rust types -- they'll be primitives, strings, enums, or simple records. All of these naturally implement Ord (and Hash + Eq). So the trait bound difference between BTreeMap and HashMap is largely theoretical for WIT-generated code.
It provides real map semantics. Unlike Vec<(K, V)>, you get deduplication and O(log n) lookup. Calling something a "map" but handing users a Vec would be misleading and push complexity onto every consumer.
It works everywhere. no_std + alloc is the baseline for Wasm guest code, and BTreeMap lives in alloc. No extra dependencies, no feature flag gymnastics.
Configuration can come later. A map_type option in bindgen! (similar to runtime_path) could be added as a non-breaking enhancement if someone has a concrete need for HashMap or a custom type. But it's not needed to land map support.
The one thing I'd avoid is Vec<(K, V)> -- even though it matches the ABI, it sacrifices the whole point of having a map type in WIT. If users wanted list-of-tuples semantics, they'd write list<tuple<K, V>> in their WIT.
So my suggestion: keep BTreeMap as-is, and note in the PR that a configurable map_type option is a possible future enhancement.
Implement map type rendering plus lowering/lifting/deallocation support across the C, C++, C#, Go, MoonBit, and Markdown backends, and add map codegen/runtime tests. This aligns non-Rust generators with core map ABI support and fixes the Go test harness module replacement path needed for map codegen verification. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
d9c32e0 to
0a302d8
Compare
Defer map codegen for MoonBit, Go, C#, C++, and C to future PRs that include runtime tests and review from language-specific maintainers. Only todo!() stubs for the new MapLower/MapLift/ IterMapKey/IterMapValue/GuestDeallocateMap instruction variants are kept so exhaustive matches compile.
This change was a fix for a latent bug from the vanity imports migration but is unrelated to map support. Removing to keep the PR focused on Rust.
- Make `InterfaceGenerator::type_map` a required trait method instead of
providing a default `todo!()` impl, so missing implementations are
caught at compile time rather than runtime.
- Remove `RuntimeItem::MapType` indirection in the Rust backend; reference
`{rt}::Map` directly instead of emitting a `pub use` re-export.
- Fix borrowed map rendering to use `&Map<K, V>` instead of the
incorrect `&[(K, V)]` slice syntax that was copy-pasted from lists.
- Add explanatory comments on map ABI methods that reuse list
read/write helpers (maps share the list<tuple<K, V>> memory layout).
- Add explicit `type_map` todo stubs to C, C++, C#, Go, and MoonBit
backends.
- Make `anonymous_type_map` a required method on `AnonymousTypeGenerator` trait (no default impl), consistent with all other methods in the trait. - Add explicit `anonymous_type_map` todo!() implementation to C backend. - Fix map runner test to construct proper Map types instead of passing slice literals, matching the generated `&Map<K, V>` parameter signatures.
The map.wit codegen test hits todo!() panics in backends that don't yet implement map support (C, C++, C#, Go, MoonBit). Add map.wit to each backend's should_fail_verify so CI treats these as expected failures.
- Run cargo fmt to fix formatting in c/lib.rs, test/c.rs, test/moonbit.rs.
- Use starts_with("map.wit") for the C backend's should_fail_verify to
catch all codegen test variants (no-sig-flattening, autodrop, async).
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Add runtime tests for empty maps, option values, maps inside records, inline anonymous map types, and a 100-entry stress test. Expand codegen tests with option values, nested maps, record values, bool keys, and char keys.
Exercise additional ABI code paths: nested map<K, map<K2, V>> with recursive lower/lift blocks, multiple map parameters with independent ptr/len pairs, map as a variant arm payload, map inside result<ok, err>, and tuple-with-map in codegen.
Exercise map inside an anonymous tuple (filter_mode_preserve_top path) and the single-entry boundary case.
Summary
map<K, V>support inwit-bindgencore ABI and backend codegen paths (C, C++, C#, Go, MoonBit), plus Markdown type renderingtests/codegen/map.witandtests/runtime/map/*crates/test/src/go.rsso generated map bindings resolvego.bytecodealliance.org/pkg/wit/*correctlyTest plan
cargo fmtcargo clippy --workspace --all-targets -- -D warningscargo checkcargo check -p wit-bindgen-corecargo check -p wit-bindgen-ccargo check -p wit-bindgen-cppcargo check -p wit-bindgen-csharpcargo check -p wit-bindgen-gocargo check -p wit-bindgen-moonbitcargo check -p wit-bindgen-markdowncargo run test --artifacts target/artifacts --runner cargo --languages go --filter map.wit tests/codegen