Add support for map type#12216
Conversation
6b6f675 to
cd2282b
Compare
Subscribe to Label Actioncc @fitzgen DetailsThis issue or pull request has been labeled: "wasmtime:api", "wasmtime:c-api", "wasmtime:config", "wizer"Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
Label Messager: wasmtime:configIt looks like you are changing Wasmtime's configuration options. Make sure to
DetailsTo modify this label's message, edit the To add new label messages or remove existing label messages, edit the |
|
On a skim this looks like it's all in the right direction, thanks! As as a heads up the wasm-tools deps will be updated in #12254 which'll avoid the need for git deps. I'll take a closer look once this is further along in CI passing tests |
db45a3a to
83dd7a7
Compare
Subscribe to Label Actioncc @fitzgen DetailsThis issue or pull request has been labeled: "fuzzing"Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
83dd7a7 to
b9ca740
Compare
alexcrichton
left a comment
There was a problem hiding this comment.
This is looking quite good to me, thanks for the thorough tests!
I haven't scrutinized the trampoline generation nor the lifting/lowering yet, but I can do that once the tests added here are passing (the #[ignore] ones at least).
If you can one thing I'd also recommend is modeling as many tests as possible as a *.wast test since that's generally the easiest to run and share (albeit difficult to write and debug)
301e19b to
d291adf
Compare
ddac63a to
b6376cb
Compare
alexcrichton
left a comment
There was a problem hiding this comment.
Mostly some thoughts about deduplication/sharing of the gnarliest bits below --
b6376cb to
e644b8b
Compare
|
@alexcrichton addressed the comments, about the sharing stuff, feedback welcome there, I am not sure if it is the best way or not; not sure what you had in mind |
This commit introduces support for HashMap<K, V> in the component model, allowing maps to be represented as list<tuple<K, V>> in the canonical ABI. It includes implementations for the ComponentType, Lower, and Lift traits for HashMap, enabling type checking, lowering to flat representations, and lifting from memory. Additionally, the maximum depth for type generation in the fuzzing utility is updated to accommodate the new map type.
This commit removes the previous wasm features configuration and adds new functions for creating a map-configured engine. The `map_config` and `map_engine` functions are introduced to facilitate the use of the component model with maps in tests, ensuring that the engine is properly configured for map types in the component model.
…existing tests This commit introduces a new WAST test file specifically for testing various map types in the component model. Additionally, it removes the redundant map type definitions from the existing types.wast file to streamline the test suite.
…ved iteration and memory management. Introduce new implementations for ComponentType, Lower, and Lift traits for std::collections::HashMap, enhancing support for map types in the component model.
The translate_map function had two categories of bugs preventing map adapter trampolines from working: 1. Wasm stack discipline: local_set_new_tmp emits LocalSet which pops from the stack, but was called when the stack was empty (to "pre-allocate" locals). Fixed by computing values first, then calling local_set_new_tmp to consume them—matching translate_list's pattern. Also removed an erroneous LocalTee that left an orphan value on the stack. Affected: src_byte_len, dst_byte_len, cur_src_ptr, cur_dst_ptr. 2. Pointer advancement: after value translation, the pointer still points at the value start. The code only advanced by trailing padding instead of value_size + trailing_padding, causing every loop iteration to re-read the same memory. Also fixes entry layout to use proper record alignment rules (entry align = max(key_align, value_align), value at aligned offset).
Val::Map already holds Vec<(Val, Val)> which derefs to &[(Val, Val)], matching lower_map's signature directly. The intermediate Vec allocation and deep clone of every key/value pair was redundant.
…er_map_to_memory helpers
- component_fuzz: use saturating_sub in generate_hashable_key to prevent underflow when fuel is 0 and Enum variant is chosen - typed: remove incorrect ? operators in lift_map for hashbrown::HashMap (with_capacity and insert don't return Result)
Compute map entry ABI and value offsets once during type building, and reuse that metadata in runtime map lift/lower paths instead of recalculating tuple layout at each call site.
Bundle map lift/lower layout and type metadata into a small MapAbi32 helper so map helper calls stay concise without changing behavior.
1802a87 to
2898eee
Compare
…rary_val The fuzzer's component_api oracle was generating map types but the engine didn't have the map feature enabled, and arbitrary_val had no arm for Type::Map. Enable component_model_map in the store helper (matching how component_model_async is forced on) and implement arbitrary value generation for map types.
2898eee to
36008b6
Compare
In reading over bytecodealliance#12216 I found a few related but also somewhat orthogonal refactors that I wanted to implement, and thus this commit. Changes here are: * Handling pointer pairs for lists/maps/strings is now more uniform with a single set of functions doing the lift/lower to flat/memory. Less "FIXME 4311" spread throughout effectively. * Conditional defines and organization of the `HashMap`-related impls were tweaked, for example impls on `HashMap` are now unconditional as well as `TryHashMap`. Some internals were refactored to in theory reduce complexity, but this is also subjective. * Lifting a `map<K, V>` now consumes fuel, a recent change, to ensure that resource exhaustion in the host is limited.
Context
This is adding support for
mapbased on WebAssembly/component-model#554References