search

WebAssembly / component-model

Repository for design and specification of the Component Model
Other
897 stars 75 forks source link

proposal: limit flags to 64 labels #370

Open ydnar opened 2 weeks ago

ydnar commented 2 weeks ago

The current specification for flags types allows an arbitrary (unlimited) number of labels.

For languages without a bit vector types or operator overloading (like Go), flags types with <= 64 labels can be represented as an unsigned integer type u8, u16, u32, u64. Flags can be composed with simple boolean operations, e.g.: f |= FlagFoo. For flags types with > 64 labels, the representation and user-facing API must change to a less ergonomic form. This can be seen in the Canonical ABI flattening rules for flags types, which require lowering into a sequence of i32.

If there exist limited or no current or practical applications for flags with > 64 labels, I propose we constrain flags types to at most 64 labels, and change the Canonical ABI form for types with > 32 and <= 64 labels to an i64.

Would changing the flattening rules be a breaking ABI change?

lukewagner commented 2 weeks ago

Adding an implementation limit of 32 flags makes sense to me, since we could relax it later and there's a good chance noone is actually using >32 flags. Changing the current flattening rules for the [32, 64]-flag case seems like maybe more of a recipe for bugs and accidental incompatibility in the rare case that anyone uses [32, 64] flags. Perhaps then later, when we relax the flag limit based on motivating use cases, we could relax it to a different ABI than is currently proposed. WDYT?

ydnar commented 2 weeks ago

Sure. Is there a mechanism to solicit feedback from community on a change like this?

lukewagner commented 2 weeks ago

Beyond discussing it in this repo, for a tweak like this, I suppose the best way to get additional feedback is to land the change and see if anything breaks (that's what nightlies and previews are for).

alexcrichton commented 2 weeks ago

Bindgen for Rust panics for >128 flags but that's only because u128 is convenient in Rust. Bindgen for C already panics if there's more than 64 flags, but again that's only because 64-bits is easy to support. That's to say I'd be in favor of limiting the maximal number of flags definitely to 64, possibly to 32 too. Languages generally don't have a great way of representing >=64 flags for sure. In the long term given that wasm has a primitive i64 datatype I think it's best to cap the flags at 64 and use an i64 as the lowering when there's >32 flags.

As to how to get to that world since it's naturally a breaking change from today I'm not so sure. I wouldn't say that we have a great nigthly-style channel to see the breakage before it happens, folks tend to just get broken and not be able to upgrade a dependency they're using. The best way forward might be to implement a warning in WIT pointing to this issue if folks have >32 flags and if no one comments for awhile we can assume it never comes up and then clamp to 32 with the option of increasing to 64 in the future.

ydnar commented 2 weeks ago

If there is currently no implementation in the wild with (32,64] labels, then changing the flatting rules to an i64 seems possible in the short term?

alexcrichton commented 2 weeks ago

Yeah I'd agree with that myself. I'd be ok either being conservative and limiting it at 32 for a bit or just going straight to 64 with an unconditional single-element flattening.

lukewagner commented 1 week ago

It's quite possible that literally noone is using >32 flags, so perhaps we could get away with going straight to the goal state without breaking anyone.

pchickey commented 1 week ago

I also suspect that noone is using >32 flags. Lets push out this restriction in wasm-tools's validator, and if nobody complains within a month or so, we have confirmed its true.

alexcrichton commented 1 week ago

I've prototyped this at https://github.com/bytecodealliance/wasm-tools/pull/1635. I'll reiterate that we don't have a great staging mechanism for things like this, so my current plan is:

If no one comments then we're then in a free spot to change things without breaking anyone since no one has 33+ flags. At that point we can change the canonical ABI to allow up to 64 flags and use a single i64 to represent them in flat lowering.

If someone comments then I'm hopeful we can find a workaround in the meantime, but that highly depends on specifics.