From fe55b3b0869b4f1cf248f0d271728b8ec836b2fa Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 24 Jun 2024 11:37:09 -0700 Subject: [PATCH] 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. --- crates/wasmparser/src/features.rs | 2 + crates/wasmparser/src/validator/component.rs | 16 ++++- .../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, 124 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..05fddfd65d 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,10 @@ 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"); + } + 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")) +)