Repository: bytecodealliance/wasm-tools
Pull request #1760 (2024-09-05 19:33:55)

Add a shorthand feature constructor for Wasm 3.0
This is still in-development but is likely close to the final set of proposals. Additionally move the previous function constructors to associated `const` values to be a bit more idiomatic with other feature names. Finally refactor the CLI to more easily support these named sets in suggestions and refactor some feature selection in the roundtrip tests.


Patch:
diff --git a/crates/wasmparser/src/features.rs b/crates/wasmparser/src/features.rs
index 4290ff42..30c8a2f1 100644
--- a/crates/wasmparser/src/features.rs
+++ b/crates/wasmparser/src/features.rs
@@ -229,26 +229,39 @@ define_wasm_features! {
 }
 
 impl WasmFeatures {
-    /// Returns the feature set associated with the 1.0 version of the
+    /// The feature set associated with the 1.0 version of the
     /// WebAssembly specification or the "MVP" feature set.
     #[cfg(feature = "features")]
-    pub fn wasm1() -> WasmFeatures {
-        WasmFeatures::FLOATS | WasmFeatures::GC_TYPES
-    }
+    pub const WASM1: WasmFeatures = WasmFeatures::FLOATS.union(WasmFeatures::GC_TYPES);
 
-    /// Returns the feature set associated with the 2.0 version of the
+    /// The feature set associated with the 2.0 version of the
     /// WebAssembly specification.
     #[cfg(feature = "features")]
-    pub fn wasm2() -> WasmFeatures {
-        WasmFeatures::wasm1()
-            | WasmFeatures::BULK_MEMORY
-            | WasmFeatures::REFERENCE_TYPES
-            | WasmFeatures::SIGN_EXTENSION
-            | WasmFeatures::MUTABLE_GLOBAL
-            | WasmFeatures::SATURATING_FLOAT_TO_INT
-            | WasmFeatures::MULTI_VALUE
-            | WasmFeatures::SIMD
-    }
+    pub const WASM2: WasmFeatures = WasmFeatures::WASM1
+        .union(WasmFeatures::BULK_MEMORY)
+        .union(WasmFeatures::REFERENCE_TYPES)
+        .union(WasmFeatures::SIGN_EXTENSION)
+        .union(WasmFeatures::MUTABLE_GLOBAL)
+        .union(WasmFeatures::SATURATING_FLOAT_TO_INT)
+        .union(WasmFeatures::MULTI_VALUE)
+        .union(WasmFeatures::SIMD);
+
+    /// The feature set associated with the 3.0 version of the
+    /// WebAssembly specification.
+    ///
+    /// Note that as of the time of this writing the 3.0 version of the
+    /// specification is not yet published. The precise set of features set
+    /// here may change as that continues to evolve.
+    #[cfg(feature = "features")]
+    pub const WASM3: WasmFeatures = WasmFeatures::WASM2
+        .union(WasmFeatures::GC)
+        .union(WasmFeatures::TAIL_CALL)
+        .union(WasmFeatures::EXTENDED_CONST)
+        .union(WasmFeatures::FUNCTION_REFERENCES)
+        .union(WasmFeatures::MULTI_MEMORY)
+        .union(WasmFeatures::RELAXED_SIMD)
+        .union(WasmFeatures::THREADS)
+        .union(WasmFeatures::EXCEPTIONS);
 }
 
 #[cfg(feature = "features")]
diff --git a/src/bin/wasm-tools/validate.rs b/src/bin/wasm-tools/validate.rs
index 7283c4da..a065665d 100644
--- a/src/bin/wasm-tools/validate.rs
+++ b/src/bin/wasm-tools/validate.rs
@@ -1,5 +1,5 @@
 use addr2line::LookupResult;
-use anyhow::{anyhow, bail, Context, Result};
+use anyhow::{bail, Context, Result};
 use bitflags::Flags;
 use rayon::prelude::*;
 use std::fmt::Write;
@@ -183,53 +183,76 @@ impl Opts {
 fn parse_features(arg: &str) -> Result<WasmFeatures> {
     let mut ret = WasmFeatures::default();
 
-    fn flag_name(flag: &bitflags::Flag<WasmFeatures>) -> String {
-        flag.name().to_lowercase().replace('_', "-")
+    const GROUPS: &[(&str, WasmFeatures)] = &[
+        ("mvp", WasmFeatures::WASM1),
+        ("wasm1", WasmFeatures::WASM1),
+        ("wasm2", WasmFeatures::WASM2),
+        ("wasm3", WasmFeatures::WASM3),
+    ];
+
+    enum Action {
+        ChangeAll,
+        Group(WasmFeatures),
+        Modify(WasmFeatures),
+    }
+
+    fn actions() -> impl Iterator<Item = (&'static str, Action)> {
+        WasmFeatures::FLAGS
+            .iter()
+            .map(|f| (f.name(), Action::Modify(*f.value())))
+            .chain(
+                GROUPS
+                    .iter()
+                    .map(|(name, features)| (*name, Action::Group(*features))),
+            )
+            .chain([("all", Action::ChangeAll)])
+    }
+
+    fn flag_name(name: &str) -> String {
+        name.to_lowercase().replace('_', "-")
     }
 
-    for part in arg.split(',').map(|s| s.trim()).filter(|s| !s.is_empty()) {
+    'outer: for part in arg.split(',').map(|s| s.trim()).filter(|s| !s.is_empty()) {
         let (enable, part) = if let Some(part) = part.strip_prefix("-") {
             (false, part)
         } else {
             (true, part)
         };
-        match part {
-            "all" => {
-                for flag in WasmFeatures::FLAGS.iter() {
-                    ret.set(*flag.value(), enable);
-                }
+        for (name, action) in actions() {
+            if part != flag_name(name) {
+                continue;
             }
-            "wasm1" | "mvp" => {
-                if !enable {
-                    bail!("cannot disable `{part}`, it can only be enabled");
+            match action {
+                Action::ChangeAll => {
+                    for flag in WasmFeatures::FLAGS.iter() {
+                        ret.set(*flag.value(), enable);
+                    }
                 }
-                ret = WasmFeatures::wasm1();
-            }
-            "wasm2" => {
-                if !enable {
-                    bail!("cannot disable `{part}`, it can only be enabled");
+                Action::Modify(feature) => {
+                    ret.set(feature, enable);
+                }
+                Action::Group(features) => {
+                    if !enable {
+                        bail!("cannot disable `{part}`, it can only be enabled");
+                    }
+                    ret = features;
                 }
-                ret = WasmFeatures::wasm2();
             }
+            continue 'outer;
+        }
 
-            name => {
-                let flag = WasmFeatures::FLAGS
-                    .iter()
-                    .find(|f| flag_name(f) == name)
-                    .ok_or_else(|| {
-                        anyhow!(
-                            "unknown feature `{}`\nValid features: {}",
-                            name,
-                            WasmFeatures::FLAGS
-                                .iter()
-                                .map(flag_name)
-                                .collect::<Vec<_>>()
-                                .join(", "),
-                        )
-                    })?;
-                ret.set(*flag.value(), enable);
+        let mut error = format!("unknown feature `{part}`\n");
+        error.push_str("Valid features: ");
+        let mut first = true;
+        for (name, _) in actions() {
+            if first {
+                first = false;
+            } else {
+                error.push_str(", ");
             }
+            error.push_str(&flag_name(name));
         }
+        bail!("{error}")
     }
 
     Ok(ret)


Test patch:
diff --git a/crates/wasm-smith/tests/common/mod.rs b/crates/wasm-smith/tests/common/mod.rs
index 332fd0ca..688bb87f 100644
--- a/crates/wasm-smith/tests/common/mod.rs
+++ b/crates/wasm-smith/tests/common/mod.rs
@@ -2,7 +2,7 @@ use wasm_smith::Config;
 use wasmparser::{types::Types, Validator, WasmFeatures};
 
 pub fn parser_features_from_config(config: &Config) -> WasmFeatures {
-    let mut features = WasmFeatures::MUTABLE_GLOBAL | WasmFeatures::wasm1();
+    let mut features = WasmFeatures::MUTABLE_GLOBAL | WasmFeatures::WASM1;
     features.set(
         WasmFeatures::SATURATING_FLOAT_TO_INT,
         config.saturating_float_to_int_enabled,
diff --git a/tests/cli/validate-unknown-features.wat.stderr b/tests/cli/validate-unknown-features.wat.stderr
index 3e0ccb84..95e79d76 100644
--- a/tests/cli/validate-unknown-features.wat.stderr
+++ b/tests/cli/validate-unknown-features.wat.stderr
@@ -1,4 +1,4 @@
 error: invalid value 'unknown' for '--features <FEATURES>': unknown feature `unknown`
-Valid features: mutable-global, saturating-float-to-int, sign-extension, reference-types, multi-value, bulk-memory, simd, relaxed-simd, threads, shared-everything-threads, tail-call, floats, multi-memory, exceptions, memory64, extended-const, component-model, function-references, memory-control, gc, custom-page-sizes, component-model-values, component-model-nested-names, component-model-more-flags, component-model-multiple-returns, legacy-exceptions, gc-types
+Valid features: mutable-global, saturating-float-to-int, sign-extension, reference-types, multi-value, bulk-memory, simd, relaxed-simd, threads, shared-everything-threads, tail-call, floats, multi-memory, exceptions, memory64, extended-const, component-model, function-references, memory-control, gc, custom-page-sizes, component-model-values, component-model-nested-names, component-model-more-flags, component-model-multiple-returns, legacy-exceptions, gc-types, mvp, wasm1, wasm2, wasm3, all
 
 For more information, try '--help'.
diff --git a/tests/roundtrip.rs b/tests/roundtrip.rs
index a5b15c4c..bd40b897 100644
--- a/tests/roundtrip.rs
+++ b/tests/roundtrip.rs
@@ -598,16 +598,9 @@ impl TestState {
         for part in test.iter().filter_map(|t| t.to_str()) {
             match part {
                 "testsuite" => {
-                    features = WasmFeatures::wasm2();
-                    features |= WasmFeatures::TAIL_CALL;
-                    features |= WasmFeatures::EXTENDED_CONST;
-
-                    // NB: when these proposals are merged upstream in the spec
-                    // repo then this should be removed. Currently this hasn't
-                    // happened so this is required to get tests passing for
-                    // when these proposals are enabled by default.
-                    features.remove(WasmFeatures::MULTI_MEMORY);
-                    features.remove(WasmFeatures::THREADS);
+                    features = WasmFeatures::WASM2
+                        | WasmFeatures::TAIL_CALL
+                        | WasmFeatures::EXTENDED_CONST;
                 }
                 "missing-features" => {
                     features =
@@ -624,14 +617,7 @@ impl TestState {
                 "exception-handling" => features.insert(WasmFeatures::EXCEPTIONS),
                 "legacy-exceptions" => features.insert(WasmFeatures::LEGACY_EXCEPTIONS),
                 "tail-call" => features.insert(WasmFeatures::TAIL_CALL),
-                "memory64" => features.insert(
-                    WasmFeatures::MEMORY64
-                        | WasmFeatures::GC
-                        | WasmFeatures::REFERENCE_TYPES
-                        | WasmFeatures::MULTI_MEMORY
-                        | WasmFeatures::FUNCTION_REFERENCES
-                        | WasmFeatures::EXCEPTIONS,
-                ),
+                "memory64" => features.insert(WasmFeatures::MEMORY64 | WasmFeatures::WASM3),
                 "component-model" => features.insert(WasmFeatures::COMPONENT_MODEL),
                 "shared-everything-threads" => {
                     features.insert(WasmFeatures::COMPONENT_MODEL);