component-model icon indicating copy to clipboard operation
component-model copied to clipboard

Limit tuple sizes to N (~16?) elements

Open rvolosatovs opened this issue 1 year ago • 2 comments

Currently tuples, as specified, may contain up to 4294967295 (u32::MAX) elements, however that presents challenges for integration, especially in languages that do not have a native tuple type (like Go).

An example Go implementation requires implementing custom tuple types per each length, e.g.:

  • https://github.com/bytecodealliance/wrpc/blob/51e857ec76378d91cf00abe4e99895df49a1005f/go/tuple.go#L227-L244

Even for languages supporting tuples, like Rust, unbounded tuples may present challenges, for example, Default is only implemented on tuples of up to 12 elements: https://doc.rust-lang.org/std/primitive.tuple.html#impl-Default-for-(T,)

Generally, it seems that using tuples containing excessive amount of elements is an anti-pattern, where such data types would be better represented using records. The suggestion is, therefore, to limit the tuple sizes to a reasonable default, 16 seems to be a reasonable ceiling here. 12 could be another option, which would nicely align with e.g. Rust. (but do note that e.g. wit-bindgen test suite utilizes tuples containing over 12 elements)

Related to #370

cc @alexcrichton

rvolosatovs avatar Jun 26 '24 16:06 rvolosatovs

Personally I'd be in favor of this if only to start out conservatively. Fully-general tuples can be convenient in situations with lots of generated code where it's less reasonable to expect a human-readable limit like 16 to be more easily applicable. That typically comes up internally within a language though rather than at the interface level, so I think it's reasonable to implement this limit.

This I suspect would sail through even easier than the changes for #370 since it's less likely someone's using 16-tuples rather than 33+ flags. (although that's still unlikely)

If you'd like @rvolosatovs I think it'd be reasonable to do something like https://github.com/bytecodealliance/wasm-tools/pull/1635 which puts the existing support behind a feature gate to keep the support there as a workaround but positions it for removal in the future.

alexcrichton avatar Jun 26 '24 19:06 alexcrichton

Sounds fine to me too.

lukewagner avatar Jun 28 '24 17:06 lukewagner