wasmtime / PR #12315 Fixed-length lists, host side · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / PR #12315 Fixed-length lists, host side

Wasmtime GitHub notifications bot (Jan 13 2026 at 21:44):

cpetig edited PR #12315.

Wasmtime GitHub notifications bot (Jan 13 2026 at 21:46):

cpetig commented on PR #12315:

TODO: Check whether the standard name fixed-length lists is used over fixed size lists.

Wasmtime GitHub notifications bot (Feb 05 2026 at 04:59):

AlbertMarashi commented on PR #12315:

What's remaining for this?

Wasmtime GitHub notifications bot (Feb 06 2026 at 14:37):

cpetig commented on PR #12315:

What's remaining for this?

I didn't find the time to add a proper test case. And I (non-blockingly) wait for a new wasm-tools release to do the renaming in one row.

Wasmtime GitHub notifications bot (Jun 11 2026 at 22:44):

cpetig updated PR #12315.

Wasmtime GitHub notifications bot (Jun 13 2026 at 22:53):

cpetig updated PR #12315.

Wasmtime GitHub notifications bot (Jun 13 2026 at 23:45):

cpetig updated PR #12315.

Wasmtime GitHub notifications bot (Jun 13 2026 at 23:55):

:memo: cpetig submitted PR review.

Wasmtime GitHub notifications bot (Jun 13 2026 at 23:55):

:speech_balloon: cpetig created PR review comment:

I found a last "size" which should be "length"

Wasmtime GitHub notifications bot (Jun 13 2026 at 23:55):

:speech_balloon: cpetig created PR review comment:

another "size" left over

Wasmtime GitHub notifications bot (Jun 13 2026 at 23:55):

:speech_balloon: cpetig created PR review comment:

should this become len() like Vec?

Wasmtime GitHub notifications bot (Jun 13 2026 at 23:55):

:speech_balloon: cpetig created PR review comment:

dito

Wasmtime GitHub notifications bot (Jun 14 2026 at 00:03):

cpetig updated PR #12315.

Wasmtime GitHub notifications bot (Jun 14 2026 at 00:14):

cpetig updated PR #12315.

Wasmtime GitHub notifications bot (Jun 14 2026 at 00:20):

cpetig has marked PR #12315 as ready for review.

Wasmtime GitHub notifications bot (Jun 14 2026 at 00:20):

cpetig requested fitzgen for a review on PR #12315.

Wasmtime GitHub notifications bot (Jun 14 2026 at 00:20):

cpetig requested wasmtime-fuzz-reviewers for a review on PR #12315.

Wasmtime GitHub notifications bot (Jun 14 2026 at 00:20):

cpetig requested wasmtime-core-reviewers for a review on PR #12315.

Wasmtime GitHub notifications bot (Jun 14 2026 at 12:46):

cpetig edited PR #12315:

Replace the todo's with proper code and add a host side test case.

Implements https://github.com/bytecodealliance/wasmtime/issues/12279

~~Includes all the commits on https://github.com/bytecodealliance/wasmtime/pull/10619 , so that one should be merged first.~~

Wasmtime GitHub notifications bot (Jun 14 2026 at 12:47):

cpetig assigned cpetig to PR #12315.

Wasmtime GitHub notifications bot (Jun 14 2026 at 16:06):

:memo: cpetig submitted PR review:

I am not sure whether the O(n) constraint applies to the host side as the user always controls the data types (and dimensions) added on the host side, but if so I will gladly investigate into a different solution.

Wasmtime GitHub notifications bot (Jun 14 2026 at 16:06):

:speech_balloon: cpetig created PR review comment:

I am not sure whether Alex doubt about O(n) at https://github.com/bytecodealliance/wasmtime/pull/10619#discussion_r2052556773 is applicable here. But it is worth linking the context here.

Wasmtime GitHub notifications bot (Jun 14 2026 at 16:06):

:speech_balloon: cpetig created PR review comment:

I am not sure whether Alex doubt about O(n) at https://github.com/bytecodealliance/wasmtime/pull/10619#discussion_r2052556773 is applicable here. But it is worth linking the context here.

Wasmtime GitHub notifications bot (Jun 14 2026 at 16:06):

:speech_balloon: cpetig created PR review comment:

I am not sure whether Alex doubt about O(n) at https://github.com/bytecodealliance/wasmtime/pull/10619#discussion_r2052556773 is applicable here. But it is worth linking the context here.

Wasmtime GitHub notifications bot (Jun 14 2026 at 16:21):

cpetig commented on PR #12315:

@Felix-El

Wasmtime GitHub notifications bot (Jun 15 2026 at 16:29):

fitzgen unassigned fitzgen from PR #12315 Fixed-length lists, host side.

Wasmtime GitHub notifications bot (Jun 15 2026 at 16:29):

fitzgen requested alexcrichton for a review on PR #12315.

Wasmtime GitHub notifications bot (Jun 16 2026 at 22:06):

:memo: alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Jun 16 2026 at 22:06):

:speech_balloon: alexcrichton created PR review comment:

Could this be moved to crates/core/src/array.rs so this could get miri-testing there, and additionally enable adding #[test] for this? Additionally, this doesn't currently handle panics of cb (leaks intermediate results) and a few more casts than I think are necessary.

I sketched out what I'm thinking here and feel free to copy that into this PR

Wasmtime GitHub notifications bot (Jun 16 2026 at 22:06):

:speech_balloon: alexcrichton created PR review comment:

This is fine yeah, it's proportional to what the guest has already done. Before this loop is done, though, can this call consume_fuel_array in the same manner as lists?

Wasmtime GitHub notifications bot (Jun 16 2026 at 22:06):

:speech_balloon: alexcrichton created PR review comment:

I've got some miscellaneous concerns about this function, so I'm going to list them here:

This is more-or-less duplicated with new_fixed_length_list_type in types_builder.rs. Could that call this function (or a similar signature?)

I'm worried about the unchecked multiplication here because the host could declare [SomeBigStruct; HUGE_NUMBER] which exceeds 4GiB of data, and I'd want that to be a panic/error/something as opposed to the silent wrap here.

I think the saturating_mul here should be checked_mul, and I also don't think the conversion here to u8 is right. If c is 1 and count is 256 then flat_count should be None, not Some(255).

I'm a little worried that in Rust below we've got N: usize where the parameter count: u32 has a different type here. Could there be a separate entrypoint which takes count: usize and performs a fallible/panicking conversion asserting that the count is less than u32::MAX?

Wasmtime GitHub notifications bot (Jun 16 2026 at 22:06):

:speech_balloon: alexcrichton created PR review comment:

Nah this is fine, this is where there's basically no other option.

Wasmtime GitHub notifications bot (Jun 16 2026 at 22:06):

:speech_balloon: alexcrichton created PR review comment:

This is fine yeah, but similar to above can this use consume_fuel_array beforehand?

Wasmtime GitHub notifications bot (Jun 20 2026 at 23:05):

cpetig updated PR #12315 (assigned to cpetig).

Wasmtime GitHub notifications bot (Jun 20 2026 at 23:14):

:memo: cpetig submitted PR review.

Wasmtime GitHub notifications bot (Jun 20 2026 at 23:14):

:speech_balloon: cpetig created PR review comment:

I thought about how to remove the duplication but this gets more difficult because one of the functions is const (this also makes the implementation less elegant as traits (and_then, try_from) don't work) and I didn't identify an obvious common dependency which is not core but component model specific.

I think I addressed all other concerns.

Wasmtime GitHub notifications bot (Jun 20 2026 at 23:27):

cpetig updated PR #12315 (assigned to cpetig).

Wasmtime GitHub notifications bot (Jun 20 2026 at 23:41):

:memo: cpetig submitted PR review.

Wasmtime GitHub notifications bot (Jun 20 2026 at 23:41):

:speech_balloon: cpetig created PR review comment:

Sadly that implementation is only possible with 1.95 up (AsMut on Uninit array, see https://doc.rust-lang.org/beta/core/mem/union.MaybeUninit.html#impl-AsMut%3C%5BMaybeUninit%3CT%3E%5D%3E-for-MaybeUninit%3C%5BT;+N%5D%3E )
:thinking:

Wasmtime GitHub notifications bot (Jun 21 2026 at 16:37):

cpetig updated PR #12315 (assigned to cpetig).

Wasmtime GitHub notifications bot (Jun 21 2026 at 16:38):

:memo: cpetig submitted PR review.

Wasmtime GitHub notifications bot (Jun 21 2026 at 16:38):

:speech_balloon: cpetig created PR review comment:

I found a way which still feels sound to me, hopefully you agree ...

Wasmtime GitHub notifications bot (Jun 21 2026 at 16:52):

cpetig updated PR #12315 (assigned to cpetig).

Wasmtime GitHub notifications bot (Jul 01 2026 at 20:55):

cpetig updated PR #12315 (assigned to cpetig).

Wasmtime GitHub notifications bot (Jul 01 2026 at 21:14):

cpetig commented on PR #12315:

@alexcrichton would you please take another look at my latest fixes?

Wasmtime GitHub notifications bot (Jul 02 2026 at 19:52):

:thumbs_up: alexcrichton submitted PR review:

Two minor thoughts from me but otherwise looks great, thanks!

Wasmtime GitHub notifications bot (Jul 02 2026 at 19:52):

:speech_balloon: alexcrichton created PR review comment:

I realize that the likelihood of getting this wrong is pretty low, but this as still worries me. Could fixed_length_list_static take a usize parameter directly? Either that or could this caluclation here double-check that N fits in u32? For example:
    const ABI: CanonicalAbiInfo = {
        if N > u32::MAX as usize { panic!() }
        CanonicalAbiInfo::fixed_length_list_static(&T::ABI, N as u32)
    };
I basically just don't want to have to think about the "what if as is lossy" even if it's a niche case that may never show up in practice

Wasmtime GitHub notifications bot (Jul 02 2026 at 19:52):

:speech_balloon: alexcrichton created PR review comment:

Could new_fixed_length_list_type be updated to call this function? I agree that the preexisting implementation of new_fixed_length_list_type is much easier to read but we'll need to have this const-compatible implementation anyway, so I'd prefer everything to funnel through the same function so there's still just one function to keep correct.

Wasmtime GitHub notifications bot (Jul 02 2026 at 22:24):

cpetig updated PR #12315 (assigned to cpetig).

Wasmtime GitHub notifications bot (Jul 02 2026 at 22:28):

:memo: cpetig submitted PR review.

Wasmtime GitHub notifications bot (Jul 02 2026 at 22:28):

:speech_balloon: cpetig created PR review comment:

Now a usize (at the cost of +10 lines of unlikely used error handling)

Wasmtime GitHub notifications bot (Jul 02 2026 at 22:30):

:memo: cpetig submitted PR review.

Wasmtime GitHub notifications bot (Jul 02 2026 at 22:30):

:speech_balloon: cpetig created PR review comment:

Now implemented as using the const function (which I upgraded to a usize argument).

Wasmtime GitHub notifications bot (Jul 06 2026 at 02:55):

:thumbs_up: alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Jul 06 2026 at 02:55):

alexcrichton added PR #12315 Fixed-length lists, host side to the merge queue.

Wasmtime GitHub notifications bot (Jul 06 2026 at 03:19):

github-merge-queue[bot] removed PR #12315 Fixed-length lists, host side from the merge queue.

Wasmtime GitHub notifications bot (Jul 07 2026 at 22:18):

cpetig updated PR #12315 (assigned to cpetig).

Wasmtime GitHub notifications bot (Jul 07 2026 at 23:13):

alexcrichton added PR #12315 Fixed-length lists, host side to the merge queue.

Wasmtime GitHub notifications bot (Jul 07 2026 at 23:38):

:check: alexcrichton merged PR #12315.

Wasmtime GitHub notifications bot (Jul 07 2026 at 23:38):

alexcrichton removed PR #12315 Fixed-length lists, host side from the merge queue.

Last updated: Jul 29 2026 at 05:03 UTC