From 2c6f12721425b79266b16e7385fb6f24d2534eed Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 25 Jun 2024 13:19:32 -0500 Subject: [PATCH] Limit component model flags to 32 (#1635) * Limit component model flags to 32 This commit is an attempt to explore the design space of WebAssembly/component-model#370. This limits, by default, the number of flags in a component model type to 32 by default. The hope of this issue is to be able to ratchet the maximum number of flags to make it easier on bindings generators to not need to work with arbitrary numbers of flags. The secondary hope is that we can ratchet flags straight to 32 instead of 64 due to it being unlikely that more than 32 flags are in use. Once this percolates there can then be a separate feature for enabling 33-64 flags. * Drop a link --- crates/wasmparser/src/features.rs | 2 + crates/wasmparser/src/validator/component.rs | 22 +++++- .../wit-component/tests/interfaces/flags.wat | 56 +++++++-------- .../wit-component/tests/interfaces/flags.wit | 69 ------------------- .../tests/interfaces/flags.wit.print | 69 ------------------- tests/local/component-model/more-flags.wast | 37 ++++++++++ tests/local/component-model/types.wast | 40 +++++++++++ tests/roundtrip.rs | 6 +- .../component-model/more-flags.wast/0.print | 3 + 9 files changed, 130 insertions(+), 174 deletions(-) create mode 100644 tests/local/component-model/more-flags.wast create mode 100644 tests/snapshots/local/component-model/more-flags.wast/0.print diff --git a/crates/wasmparser/src/features.rs b/crates/wasmparser/src/features.rs index e73a9e9a2b..6478cf9c91 100644 --- a/crates/wasmparser/src/features.rs +++ b/crates/wasmparser/src/features.rs @@ -146,6 +146,8 @@ define_wasm_features! { pub component_model_values: COMPONENT_MODEL_VALUES(1 << 21) = false; /// Support for the nested namespaces and projects in component model names. pub component_model_nested_names: COMPONENT_MODEL_NESTED_NAMES(1 << 22) = false; + /// Support for more than 32 flags per-type in the component model. + pub component_model_more_flags: COMPONENT_MODEL_MORE_FLAGS(1 << 23) = false; } } diff --git a/crates/wasmparser/src/validator/component.rs b/crates/wasmparser/src/validator/component.rs index f39be44d4f..80dbd2e380 100644 --- a/crates/wasmparser/src/validator/component.rs +++ b/crates/wasmparser/src/validator/component.rs @@ -348,7 +348,7 @@ impl ComponentState { let id = match ty { crate::ComponentType::Defined(ty) => { - let ty = current(components).create_defined_type(ty, types, offset)?; + let ty = current(components).create_defined_type(ty, types, features, offset)?; types.push(ty).into() } crate::ComponentType::Func(ty) => { @@ -2512,6 +2512,7 @@ impl ComponentState { &self, ty: crate::ComponentDefinedType, types: &TypeList, + features: &WasmFeatures, offset: usize, ) -> Result { match ty { @@ -2529,7 +2530,7 @@ impl ComponentState { self.create_tuple_type(tys.as_ref(), types, offset) } crate::ComponentDefinedType::Flags(names) => { - self.create_flags_type(names.as_ref(), offset) + self.create_flags_type(names.as_ref(), features, offset) } crate::ComponentDefinedType::Enum(cases) => { self.create_enum_type(cases.as_ref(), offset) @@ -2681,7 +2682,12 @@ impl ComponentState { Ok(ComponentDefinedType::Tuple(TupleType { info, types })) } - fn create_flags_type(&self, names: &[&str], offset: usize) -> Result { + fn create_flags_type( + &self, + names: &[&str], + features: &WasmFeatures, + offset: usize, + ) -> Result { let mut names_set = IndexSet::default(); names_set.reserve(names.len()); @@ -2689,6 +2695,16 @@ impl ComponentState { bail!(offset, "flags must have at least one entry"); } + if names.len() > 32 && !features.component_model_more_flags() { + bail!( + offset, + "cannot have more than 32 flags; this was previously \ + accepted and if this is required for your project please \ + leave a comment on \ + https://github.com/WebAssembly/component-model/issues/370" + ); + } + for name in names { let name = to_kebab_str(name, "flag", offset)?; if !names_set.insert(name.to_owned()) { diff --git a/crates/wit-component/tests/interfaces/flags.wat b/crates/wit-component/tests/interfaces/flags.wat index 7c0aa39899..61a6ea97bc 100644 --- a/crates/wit-component/tests/interfaces/flags.wat +++ b/crates/wit-component/tests/interfaces/flags.wat @@ -15,22 +15,18 @@ (export (;9;) "flag16" (type (eq 8))) (type (;10;) (flags "b0" "b1" "b2" "b3" "b4" "b5" "b6" "b7" "b8" "b9" "b10" "b11" "b12" "b13" "b14" "b15" "b16" "b17" "b18" "b19" "b20" "b21" "b22" "b23" "b24" "b25" "b26" "b27" "b28" "b29" "b30" "b31")) (export (;11;) "flag32" (type (eq 10))) - (type (;12;) (flags "b0" "b1" "b2" "b3" "b4" "b5" "b6" "b7" "b8" "b9" "b10" "b11" "b12" "b13" "b14" "b15" "b16" "b17" "b18" "b19" "b20" "b21" "b22" "b23" "b24" "b25" "b26" "b27" "b28" "b29" "b30" "b31" "b32" "b33" "b34" "b35" "b36" "b37" "b38" "b39" "b40" "b41" "b42" "b43" "b44" "b45" "b46" "b47" "b48" "b49" "b50" "b51" "b52" "b53" "b54" "b55" "b56" "b57" "b58" "b59" "b60" "b61" "b62" "b63")) - (export (;13;) "flag64" (type (eq 12))) - (type (;14;) (func (param "x" 1) (result 1))) - (export (;0;) "roundtrip-flag1" (func (type 14))) - (type (;15;) (func (param "x" 3) (result 3))) - (export (;1;) "roundtrip-flag2" (func (type 15))) - (type (;16;) (func (param "x" 5) (result 5))) - (export (;2;) "roundtrip-flag4" (func (type 16))) - (type (;17;) (func (param "x" 7) (result 7))) - (export (;3;) "roundtrip-flag8" (func (type 17))) - (type (;18;) (func (param "x" 9) (result 9))) - (export (;4;) "roundtrip-flag16" (func (type 18))) - (type (;19;) (func (param "x" 11) (result 11))) - (export (;5;) "roundtrip-flag32" (func (type 19))) - (type (;20;) (func (param "x" 13) (result 13))) - (export (;6;) "roundtrip-flag64" (func (type 20))) + (type (;12;) (func (param "x" 1) (result 1))) + (export (;0;) "roundtrip-flag1" (func (type 12))) + (type (;13;) (func (param "x" 3) (result 3))) + (export (;1;) "roundtrip-flag2" (func (type 13))) + (type (;14;) (func (param "x" 5) (result 5))) + (export (;2;) "roundtrip-flag4" (func (type 14))) + (type (;15;) (func (param "x" 7) (result 7))) + (export (;3;) "roundtrip-flag8" (func (type 15))) + (type (;16;) (func (param "x" 9) (result 9))) + (export (;4;) "roundtrip-flag16" (func (type 16))) + (type (;17;) (func (param "x" 11) (result 11))) + (export (;5;) "roundtrip-flag32" (func (type 17))) ) ) (export (;0;) "foo:flags/imports" (instance (type 0))) @@ -55,22 +51,18 @@ (export (;9;) "flag16" (type (eq 8))) (type (;10;) (flags "b0" "b1" "b2" "b3" "b4" "b5" "b6" "b7" "b8" "b9" "b10" "b11" "b12" "b13" "b14" "b15" "b16" "b17" "b18" "b19" "b20" "b21" "b22" "b23" "b24" "b25" "b26" "b27" "b28" "b29" "b30" "b31")) (export (;11;) "flag32" (type (eq 10))) - (type (;12;) (flags "b0" "b1" "b2" "b3" "b4" "b5" "b6" "b7" "b8" "b9" "b10" "b11" "b12" "b13" "b14" "b15" "b16" "b17" "b18" "b19" "b20" "b21" "b22" "b23" "b24" "b25" "b26" "b27" "b28" "b29" "b30" "b31" "b32" "b33" "b34" "b35" "b36" "b37" "b38" "b39" "b40" "b41" "b42" "b43" "b44" "b45" "b46" "b47" "b48" "b49" "b50" "b51" "b52" "b53" "b54" "b55" "b56" "b57" "b58" "b59" "b60" "b61" "b62" "b63")) - (export (;13;) "flag64" (type (eq 12))) - (type (;14;) (func (param "x" 1) (result 1))) - (export (;0;) "roundtrip-flag1" (func (type 14))) - (type (;15;) (func (param "x" 3) (result 3))) - (export (;1;) "roundtrip-flag2" (func (type 15))) - (type (;16;) (func (param "x" 5) (result 5))) - (export (;2;) "roundtrip-flag4" (func (type 16))) - (type (;17;) (func (param "x" 7) (result 7))) - (export (;3;) "roundtrip-flag8" (func (type 17))) - (type (;18;) (func (param "x" 9) (result 9))) - (export (;4;) "roundtrip-flag16" (func (type 18))) - (type (;19;) (func (param "x" 11) (result 11))) - (export (;5;) "roundtrip-flag32" (func (type 19))) - (type (;20;) (func (param "x" 13) (result 13))) - (export (;6;) "roundtrip-flag64" (func (type 20))) + (type (;12;) (func (param "x" 1) (result 1))) + (export (;0;) "roundtrip-flag1" (func (type 12))) + (type (;13;) (func (param "x" 3) (result 3))) + (export (;1;) "roundtrip-flag2" (func (type 13))) + (type (;14;) (func (param "x" 5) (result 5))) + (export (;2;) "roundtrip-flag4" (func (type 14))) + (type (;15;) (func (param "x" 7) (result 7))) + (export (;3;) "roundtrip-flag8" (func (type 15))) + (type (;16;) (func (param "x" 9) (result 9))) + (export (;4;) "roundtrip-flag16" (func (type 16))) + (type (;17;) (func (param "x" 11) (result 11))) + (export (;5;) "roundtrip-flag32" (func (type 17))) ) ) (import "foo:flags/imports" (instance (;0;) (type 0))) diff --git a/crates/wit-component/tests/interfaces/flags.wit b/crates/wit-component/tests/interfaces/flags.wit index 9542f96e08..75f16cb355 100644 --- a/crates/wit-component/tests/interfaces/flags.wit +++ b/crates/wit-component/tests/interfaces/flags.wit @@ -82,73 +82,6 @@ interface imports { b31, } - flags flag64 { - b0, - b1, - b2, - b3, - b4, - b5, - b6, - b7, - b8, - b9, - b10, - b11, - b12, - b13, - b14, - b15, - b16, - b17, - b18, - b19, - b20, - b21, - b22, - b23, - b24, - b25, - b26, - b27, - b28, - b29, - b30, - b31, - b32, - b33, - b34, - b35, - b36, - b37, - b38, - b39, - b40, - b41, - b42, - b43, - b44, - b45, - b46, - b47, - b48, - b49, - b50, - b51, - b52, - b53, - b54, - b55, - b56, - b57, - b58, - b59, - b60, - b61, - b62, - b63, - } - roundtrip-flag1: func(x: flag1) -> flag1; roundtrip-flag2: func(x: flag2) -> flag2; @@ -160,8 +93,6 @@ interface imports { roundtrip-flag16: func(x: flag16) -> flag16; roundtrip-flag32: func(x: flag32) -> flag32; - - roundtrip-flag64: func(x: flag64) -> flag64; } world flags-world { diff --git a/crates/wit-component/tests/interfaces/flags.wit.print b/crates/wit-component/tests/interfaces/flags.wit.print index 9542f96e08..75f16cb355 100644 --- a/crates/wit-component/tests/interfaces/flags.wit.print +++ b/crates/wit-component/tests/interfaces/flags.wit.print @@ -82,73 +82,6 @@ interface imports { b31, } - flags flag64 { - b0, - b1, - b2, - b3, - b4, - b5, - b6, - b7, - b8, - b9, - b10, - b11, - b12, - b13, - b14, - b15, - b16, - b17, - b18, - b19, - b20, - b21, - b22, - b23, - b24, - b25, - b26, - b27, - b28, - b29, - b30, - b31, - b32, - b33, - b34, - b35, - b36, - b37, - b38, - b39, - b40, - b41, - b42, - b43, - b44, - b45, - b46, - b47, - b48, - b49, - b50, - b51, - b52, - b53, - b54, - b55, - b56, - b57, - b58, - b59, - b60, - b61, - b62, - b63, - } - roundtrip-flag1: func(x: flag1) -> flag1; roundtrip-flag2: func(x: flag2) -> flag2; @@ -160,8 +93,6 @@ interface imports { roundtrip-flag16: func(x: flag16) -> flag16; roundtrip-flag32: func(x: flag32) -> flag32; - - roundtrip-flag64: func(x: flag64) -> flag64; } world flags-world { diff --git a/tests/local/component-model/more-flags.wast b/tests/local/component-model/more-flags.wast new file mode 100644 index 0000000000..6d17e45095 --- /dev/null +++ b/tests/local/component-model/more-flags.wast @@ -0,0 +1,37 @@ +(component + (type (flags + "f1" + "f2" + "f3" + "f4" + "f5" + "f6" + "f7" + "f8" + "f9" + "f10" + "f11" + "f12" + "f13" + "f14" + "f15" + "f16" + "f17" + "f18" + "f19" + "f20" + "f21" + "f22" + "f23" + "f24" + "f25" + "f26" + "f27" + "f28" + "f29" + "f30" + "f31" + "f32" + "f33" + )) +) diff --git a/tests/local/component-model/types.wast b/tests/local/component-model/types.wast index 628ec825cf..88fbd2d376 100644 --- a/tests/local/component-model/types.wast +++ b/tests/local/component-model/types.wast @@ -323,3 +323,43 @@ )) ) ) + +(assert_invalid + (component + (type (flags + "f1" + "f2" + "f3" + "f4" + "f5" + "f6" + "f7" + "f8" + "f9" + "f10" + "f11" + "f12" + "f13" + "f14" + "f15" + "f16" + "f17" + "f18" + "f19" + "f20" + "f21" + "f22" + "f23" + "f24" + "f25" + "f26" + "f27" + "f28" + "f29" + "f30" + "f31" + "f32" + "f33" + )) + ) + "cannot have more than 32 flags") diff --git a/tests/roundtrip.rs b/tests/roundtrip.rs index 61620033e9..66fdc1e49b 100644 --- a/tests/roundtrip.rs +++ b/tests/roundtrip.rs @@ -578,7 +578,8 @@ impl TestState { let mut features = WasmFeatures::all() & !WasmFeatures::SHARED_EVERYTHING_THREADS & !WasmFeatures::COMPONENT_MODEL - & !WasmFeatures::COMPONENT_MODEL_NESTED_NAMES; + & !WasmFeatures::COMPONENT_MODEL_NESTED_NAMES + & !WasmFeatures::COMPONENT_MODEL_MORE_FLAGS; for part in test.iter().filter_map(|t| t.to_str()) { match part { "testsuite" => { @@ -623,6 +624,9 @@ impl TestState { "import-extended.wast" => { features.insert(WasmFeatures::COMPONENT_MODEL_NESTED_NAMES); } + "more-flags.wast" => { + features.insert(WasmFeatures::COMPONENT_MODEL_MORE_FLAGS); + } _ => {} } } diff --git a/tests/snapshots/local/component-model/more-flags.wast/0.print b/tests/snapshots/local/component-model/more-flags.wast/0.print new file mode 100644 index 0000000000..5e23480ae2 --- /dev/null +++ b/tests/snapshots/local/component-model/more-flags.wast/0.print @@ -0,0 +1,3 @@ +(component + (type (;0;) (flags "f1" "f2" "f3" "f4" "f5" "f6" "f7" "f8" "f9" "f10" "f11" "f12" "f13" "f14" "f15" "f16" "f17" "f18" "f19" "f20" "f21" "f22" "f23" "f24" "f25" "f26" "f27" "f28" "f29" "f30" "f31" "f32" "f33")) +)