Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add optional fixed length to lists #304

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

lukewagner
Copy link
Member

This PR adds fixed-length lists as a "gated feature" (i.e., features spec'd and ready to implement, but not yet included in a Preview release), as proposed and discussed in #181.

@lukewagner lukewagner mentioned this pull request Feb 19, 2024
@badeend
Copy link
Contributor

badeend commented Feb 19, 2024

I'm not too familiar with the ABI, but can a large fixed-size List blow the stack? If so, it might be an idea to fall back to heap allocation after a certain size.

@lukewagner
Copy link
Member Author

The MAX_FLAT_PARAMS/RESULTS limits already in place end up protecting the number of "flattened" scalar params/results that go on the native stack (similar to how they limit the flattening of tuples), falling back to the heap, as you suggest.

@ydnar
Copy link

ydnar commented Feb 20, 2024

I’m glad to have fixed-width arrays added to WIT.

However from the perspective of an ABI implementer, I struggle with the representation of fixed-length arrays as a variation of list<T>, rather than as a new standalone type that shares some similarities to both tuple and list.

  • A fixed-length list shares nothing with list other than name (ABI representation, size, alignment, flattening and lift/lower rules).
  • Where previously the size and alignment of list were constant (8 and 4, respectively), this adds complexity that every WIT/Component Model implementer will need to address.

Concretely, I’d vote for a new type, let‘s call it array, that works exactly as described in this PR, but decouples its logic from list, leaving list unchanged from the existing spec. An array would unequivocally be a value type with non-optional type and len, leaving list as a type with a pointer to allocated storage.

The WIT syntax for array could be anything:

  • array<u8, 4>
  • [4]u8
  • [u8, 4]

@lukewagner
Copy link
Member Author

I think we shouldn't let the CABI representational concerns determine the WIT- and Component-Model-level names we choose; rather we should just focus on what's the least confusing things to folks reading/writing WIT.

Perhaps there is an argument that having both list<u8> and list<u8,4> share the same type-name "list" is confusing and so having different type names for the variable- and fixed-length cases is less confusing? I don't think so, but this is admittedly subjective. There is some precedent we can observe in Go and Rust, though: the syntax of the fixed-length thing ([T; N] in Rust, [N]T in Go) is a syntactic extension of the dynamic-length thing ([T] in Rust, []T in Go). Thus, there is evidence that having the fixed-length construct syntactically refine the variable-length construct is not confusing. But I haven't done an exhaustive survey of languages or anything.

Overall, while I'm not opposed to having array<T,N>, I think less names is better, as long as we're not introducing confusion. But I'd be interested to hear more thoughts from folks on this.

@oovm
Copy link

oovm commented Mar 2, 2024

I don't think list contains any mutability constraints, neither capacity mutability nor internal element mutability.

Without any numbers, it means that this is a list with unknown capacity (unknown at compile time)

That is:

list<T>     = list<T, ?>
list<u8, 4> = tuple<u8, u8, u8, u8>

Mutability needs to be passed via abi options and verified

For example list<u8, 4> can be:

  • dynamic passing: (ptr: i32, len: i32)
  • static passing: (ptr: i32)
  • inline passing: (i32, i32, i32, i32)
  • pass by reference: (arrayref)

Even though length and size_of(u8) are known at compile time, they will still be passed in by host if required.


Therefore I suggest adding an abi option static-length, requiring the language to interpret it as an array, to be compatible with some languages ​​that do not have the concept of array(fixed length list).

Considering that array is a feature widely present in popular languages, it should require static by default, and then add the reverse dynamic-length

@ydnar
Copy link

ydnar commented Mar 2, 2024

Therefore I suggest adding an abi option static-length, requiring the language to interpret it as an array, to be compatible with some languages ​​that do not have the concept of array(fixed length list).

Considering that array is a feature widely present in popular languages, it should require static by default, and then add the reverse dynamic-length

How are tuple handled in those languages today?

@oovm
Copy link

oovm commented Mar 5, 2024

@ydnar

For tuples, according to my classification, the current working mode is inline.

In other words, if list<u8, 4> is the syntactic sugar of tuple<u8, u8, u8, u8>, (i32, i32, i32, i32) will be passed in, instead of (ptr: i32).

Unless the input parameters are greater than 16 or the return value is greater than 1, static-length mode will be used.

package examples: tests;

world imports {
  import tuples;
}

interface tuples {
  tuple0: func(x: tuple<>);
  tuple1: func(x: tuple<u8>);
  tuple2: func(x: tuple<u8, u8>);
  tuple4: func(x: tuple<u8, u8, u8, u8>);
  tuple16: func(x: tuple<u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8>);
  tuple17: func(x: tuple<u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8>);

  tuple-r0: func() -> tuple<>;
  tuple-r1: func() -> tuple<u8>;
  tuple-r2: func() -> tuple<u8, u8>;
}
(module
  (type (;0;) (func))
  (type (;1;) (func (param i32)))
  (type (;2;) (func (param i32 i32)))
  (type (;3;) (func (param i32 i32 i32 i32)))
  (type (;4;) (func (param i32 i32 i32 i32 i32 i32 i32 i32 i32 i32 i32 i32 i32 i32 i32 i32)))
  (type (;5;) (func (result i32)))
  (type (;6;) (func (param i32 i32 i32 i32) (result i32)))
  (import "examples:tests/tuples" "tuple0" (func (;0;) (type 0)))
  (import "examples:tests/tuples" "tuple1" (func (;1;) (type 1)))
  (import "examples:tests/tuples" "tuple2" (func (;2;) (type 2)))
  (import "examples:tests/tuples" "tuple4" (func (;3;) (type 3)))
  (import "examples:tests/tuples" "tuple16" (func (;4;) (type 4)))
  (import "examples:tests/tuples" "tuple17" (func (;5;) (type 1)))
  (import "examples:tests/tuples" "tuple-r0" (func (;6;) (type 0)))
  (import "examples:tests/tuples" "tuple-r1" (func (;7;) (type 5)))
  (import "examples:tests/tuples" "tuple-r2" (func (;8;) (type 1)))
)

@cpetig
Copy link

cpetig commented Aug 1, 2024

I just created a new MR #384 with the same changes but all conflicts resolved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants