Refactor the #[wasmtime_test] macro (#9627)

* Refactor the `#[wasmtime_test]` macro

* Start tests with a blank slate of features instead of with the default
  set of features enables (ensures each test explicitly specifies
  required features)

* Reuse test features from `wasmtime_wast_util` to avoid duplicating
  listings of features. Also shares logic for "should this compiler fail
  this test because of unsupported features".

* Move logic in `tests/wast.rs` to apply test configuration to a
  `Config` to a new location that can be shared across suites.

* Add a new feature for `simd` and flag tests that need it with the feature.

This is done in preparation for adding a new compiler strategy of Pulley
to be able to flag tests as passing for pulley or not.

* Review feedback

Author: alexcrichton

Cargo.lock

@@ -122,33 +122,50 @@ impl Config {
     /// This will additionally update limits in the pooling allocator to be able
     /// to execute all tests.
     pub fn make_wast_test_compliant(&mut self, test: &WastTest) -> WastConfig {
+        let wasmtime_wast_util::TestConfig {
+            memory64,
+            custom_page_sizes,
+            multi_memory,
+            threads,
+            gc,
+            function_references,
+            relaxed_simd,
+            reference_types,
+            tail_call,
+            extended_const,
+            wide_arithmetic,
+            component_model_more_flags,
+            simd,
+
+            hogs_memory: _,
+            nan_canonicalization: _,
+            gc_types: _,
+        } = test.config;
+
         // Enable/disable some proposals that aren't configurable in wasm-smith
         // but are configurable in Wasmtime.
-        self.module_config.function_references_enabled = test
-            .config
-            .function_references
-            .or(test.config.gc)
-            .unwrap_or(false);
-        self.module_config.component_model_more_flags =
-            test.config.component_model_more_flags.unwrap_or(false);
+        self.module_config.function_references_enabled =
+            function_references.or(gc).unwrap_or(false);
+        self.module_config.component_model_more_flags = component_model_more_flags.unwrap_or(false);
 
         // Enable/disable proposals that wasm-smith has knobs for which will be
         // read when creating `wasmtime::Config`.
         let config = &mut self.module_config.config;
         config.bulk_memory_enabled = true;
         config.multi_value_enabled = true;
-        config.simd_enabled = true;
-        config.wide_arithmetic_enabled = test.config.wide_arithmetic.unwrap_or(false);
-        config.memory64_enabled = test.config.memory64.unwrap_or(false);
-        config.tail_call_enabled = test.config.tail_call.unwrap_or(false);
-        config.custom_page_sizes_enabled = test.config.custom_page_sizes.unwrap_or(false);
-        config.threads_enabled = test.config.threads.unwrap_or(false);
-        config.gc_enabled = test.config.gc.unwrap_or(false);
+        config.wide_arithmetic_enabled = wide_arithmetic.unwrap_or(false);
+        config.memory64_enabled = memory64.unwrap_or(false);
+        config.relaxed_simd_enabled = relaxed_simd.unwrap_or(false);
+        config.simd_enabled = config.relaxed_simd_enabled || simd.unwrap_or(false);
+        config.tail_call_enabled = tail_call.unwrap_or(false);
+        config.custom_page_sizes_enabled = custom_page_sizes.unwrap_or(false);
+        config.threads_enabled = threads.unwrap_or(false);
+        config.gc_enabled = gc.unwrap_or(false);
         config.reference_types_enabled = config.gc_enabled
             || self.module_config.function_references_enabled
-            || test.config.reference_types.unwrap_or(false);
-        config.extended_const_enabled = test.config.extended_const.unwrap_or(false);
-        if test.config.multi_memory.unwrap_or(false) {
+            || reference_types.unwrap_or(false);
+        config.extended_const_enabled = extended_const.unwrap_or(false);
+        if multi_memory.unwrap_or(false) {
             config.max_memories = limits::MEMORIES_PER_MODULE as usize;
         } else {
             config.max_memories = 1;

crates/fuzzing/src/generators/config.rs

@@ -12,3 +12,4 @@ env_logger = { workspace = true }
 anyhow = { workspace = true }
 arbitrary = { workspace = true, features = ["derive"] }
 wasmtime = { workspace = true, features = ["component-model", "async"] }
+wasmtime-wast-util = { path = '../../wast-util' }

crates/misc/component-test-util/Cargo.toml

@@ -129,3 +129,80 @@ forward_impls! {
     Float32 => f32,
     Float64 => f64,
 }
+
+/// Helper method to apply `wast_config` to `config`.
+pub fn apply_wast_config(config: &mut Config, wast_config: &wasmtime_wast_util::WastConfig) {
+    config.strategy(match wast_config.compiler {
+        wasmtime_wast_util::Compiler::Cranelift => wasmtime::Strategy::Cranelift,
+        wasmtime_wast_util::Compiler::Winch => wasmtime::Strategy::Winch,
+    });
+    config.collector(match wast_config.collector {
+        wasmtime_wast_util::Collector::Auto => wasmtime::Collector::Auto,
+        wasmtime_wast_util::Collector::Null => wasmtime::Collector::Null,
+        wasmtime_wast_util::Collector::DeferredReferenceCounting => {
+            wasmtime::Collector::DeferredReferenceCounting
+        }
+    });
+}
+
+/// Helper method to apply `test_config` to `config`.
+pub fn apply_test_config(config: &mut Config, test_config: &wasmtime_wast_util::TestConfig) {
+    let wasmtime_wast_util::TestConfig {
+        memory64,
+        custom_page_sizes,
+        multi_memory,
+        threads,
+        gc,
+        function_references,
+        relaxed_simd,
+        reference_types,
+        tail_call,
+        extended_const,
+        wide_arithmetic,
+        component_model_more_flags,
+        nan_canonicalization,
+        simd,
+
+        hogs_memory: _,
+        gc_types: _,
+    } = *test_config;
+    // Note that all of these proposals/features are currently default-off to
+    // ensure that we annotate all tests accurately with what features they
+    // need, even in the future when features are stabilized.
+    let memory64 = memory64.unwrap_or(false);
+    let custom_page_sizes = custom_page_sizes.unwrap_or(false);
+    let multi_memory = multi_memory.unwrap_or(false);
+    let threads = threads.unwrap_or(false);
+    let gc = gc.unwrap_or(false);
+    let tail_call = tail_call.unwrap_or(false);
+    let extended_const = extended_const.unwrap_or(false);
+    let wide_arithmetic = wide_arithmetic.unwrap_or(false);
+    let component_model_more_flags = component_model_more_flags.unwrap_or(false);
+    let nan_canonicalization = nan_canonicalization.unwrap_or(false);
+    let relaxed_simd = relaxed_simd.unwrap_or(false);
+
+    // Some proposals in wasm depend on previous proposals. For example the gc
+    // proposal depends on function-references which depends on reference-types.
+    // To avoid needing to enable all of them at once implicitly enable
+    // downstream proposals once the end proposal is enabled (e.g. when enabling
+    // gc that also enables function-references and reference-types).
+    let function_references = gc || function_references.unwrap_or(false);
+    let reference_types = function_references || reference_types.unwrap_or(false);
+    let simd = relaxed_simd || simd.unwrap_or(false);
+
+    config
+        .wasm_multi_memory(multi_memory)
+        .wasm_threads(threads)
+        .wasm_memory64(memory64)
+        .wasm_function_references(function_references)
+        .wasm_gc(gc)
+        .wasm_reference_types(reference_types)
+        .wasm_relaxed_simd(relaxed_simd)
+        .wasm_simd(simd)
+        .wasm_tail_call(tail_call)
+        .wasm_custom_page_sizes(custom_page_sizes)
+        .wasm_extended_const(extended_const)
+        .wasm_wide_arithmetic(wide_arithmetic)
+        .wasm_component_model_more_flags(component_model_more_flags)
+        .cranelift_nan_canonicalization(nan_canonicalization);
+}

crates/misc/component-test-util/src/lib.rs

@@ -21,3 +21,4 @@ quote = "1.0"
 proc-macro2 = "1.0"
 syn = { workspace = true, features = ["full"] }
 anyhow = { workspace = true }
+wasmtime-wast-util = { path = '../wast-util' }

crates/test-macros/Cargo.toml

@@ -29,26 +29,19 @@
 //! If the wasm feature is not supported by any of the compiler strategies, no
 //! tests will be generated for such strategy.
 use proc_macro::TokenStream;
-use proc_macro2::Span;
 use quote::{quote, ToTokens, TokenStreamExt};
 use syn::{
     braced,
     meta::ParseNestedMeta,
     parse::{Parse, ParseStream},
     parse_macro_input, token, Attribute, Ident, Result, ReturnType, Signature, Visibility,
 };
+use wasmtime_wast_util::Compiler;
 
 /// Test configuration.
 struct TestConfig {
-    /// Supported compiler strategies.
-    strategies: Vec<Ident>,
-    /// Known WebAssembly features that will be turned on by default in the
-    /// resulting Config.
-    /// The identifiers in this list are features that are off by default in
-    /// Wasmtime's Config, which will be explicitly turned on for a given test.
-    wasm_features: Vec<Ident>,
-    /// Flag to track if there are Wasm features not supported by Winch.
-    wasm_features_unsupported_by_winch: bool,
+    strategies: Vec<Compiler>,
+    flags: wasmtime_wast_util::TestConfig,
     /// The test attribute to use. Defaults to `#[test]`.
     test_attribute: Option<proc_macro2::TokenStream>,
 }
@@ -58,9 +51,11 @@ impl TestConfig {
         meta.parse_nested_meta(|meta| {
             if meta.path.is_ident("not") {
                 meta.parse_nested_meta(|meta| {
-                    if meta.path.is_ident("Winch") || meta.path.is_ident("Cranelift") {
-                        let id = meta.path.require_ident()?.clone();
-                        self.strategies.retain(|s| *s != id);
+                    if meta.path.is_ident("Winch") {
+                        self.strategies.retain(|s| *s != Compiler::Winch);
+                        Ok(())
+                    } else if meta.path.is_ident("Cranelift") {
+                        self.strategies.retain(|s| *s != Compiler::Cranelift);
                         Ok(())
                     } else {
                         Err(meta.error("Unknown strategy"))
@@ -80,32 +75,15 @@ impl TestConfig {
 
     fn wasm_features_from(&mut self, meta: &ParseNestedMeta) -> Result<()> {
         meta.parse_nested_meta(|meta| {
-            if meta.path.is_ident("gc") || meta.path.is_ident("function_references") {
-                let feature = meta.path.require_ident()?.clone();
-                self.wasm_features.push(feature.clone());
-                self.wasm_features_unsupported_by_winch = true;
-                Ok(())
-            } else if meta.path.is_ident("simd")
-                || meta.path.is_ident("relaxed_simd")
-                || meta.path.is_ident("reference_types")
-                || meta.path.is_ident("tail_call")
-                || meta.path.is_ident("threads")
-            {
-                self.wasm_features_unsupported_by_winch = true;
-                Ok(())
-            } else {
-                Err(meta.error("Unsupported wasm feature"))
+            for (feature, enabled) in self.flags.options_mut() {
+                if meta.path.is_ident(feature) {
+                    *enabled = Some(true);
+                    return Ok(());
+                }
             }
+            Err(meta.error("Unsupported test feature"))
         })?;
 
-        if self.wasm_features.len() > 2 {
-            return Err(meta.error("Expected at most 2 off-by-default wasm features"));
-        }
-
-        if self.wasm_features_unsupported_by_winch {
-            self.strategies.retain(|s| s.to_string() != "Winch");
-        }
-
         Ok(())
     }
 
@@ -119,12 +97,8 @@ impl TestConfig {
 impl Default for TestConfig {
     fn default() -> Self {
         Self {
-            strategies: vec![
-                Ident::new("Cranelift", Span::call_site()),
-                Ident::new("Winch", Span::call_site()),
-            ],
-            wasm_features: vec![],
-            wasm_features_unsupported_by_winch: false,
+            strategies: vec![Compiler::Cranelift, Compiler::Winch],
+            flags: Default::default(),
             test_attribute: None,
         }
     }
@@ -216,9 +190,7 @@ pub fn wasmtime_test(attrs: TokenStream, item: TokenStream) -> TokenStream {
 }
 
 fn expand(test_config: &TestConfig, func: Fn) -> Result<TokenStream> {
-    let mut tests = if test_config.strategies.len() == 1
-        && test_config.strategies.get(0).map(|s| s.to_string()) == Some("Winch".to_string())
-    {
+    let mut tests = if test_config.strategies == [Compiler::Winch] {
         vec![quote! {
             // This prevents dead code warning when the macro is invoked as:
             //     #[wasmtime_test(strategies(not(Cranelift))]
@@ -236,10 +208,10 @@ fn expand(test_config: &TestConfig, func: Fn) -> Result<TokenStream> {
         .clone()
         .unwrap_or_else(|| quote! { #[test] });
 
-    for ident in &test_config.strategies {
-        let strategy_name = ident.to_string();
+    for strategy in &test_config.strategies {
+        let strategy_name = format!("{strategy:?}");
         // Winch currently only offers support for x64.
-        let target = if strategy_name == "Winch" {
+        let target = if *strategy == Compiler::Winch {
             quote! { #[cfg(target_arch = "x86_64")] }
         } else {
             quote! {}
@@ -250,31 +222,46 @@ fn expand(test_config: &TestConfig, func: Fn) -> Result<TokenStream> {
             (quote! {}, quote! {})
         };
         let func_name = &func.sig.ident;
-        let ret = match &func.sig.output {
+        let expect = match &func.sig.output {
             ReturnType::Default => quote! {},
-            ReturnType::Type(_, ty) => quote! { -> #ty },
+            ReturnType::Type(..) => quote! { .expect("test is expected to pass") },
         };
         let test_name = Ident::new(
             &format!("{}_{}", strategy_name.to_lowercase(), func_name),
             func_name.span(),
         );
 
-        let config_setup = test_config.wasm_features.iter().map(|f| {
-            let method_name = Ident::new(&format!("wasm_{f}"), f.span());
-            quote! {
-                config.#method_name(true);
-            }
-        });
+        let should_panic = if strategy.should_fail(&test_config.flags) {
+            quote!(#[should_panic])
+        } else {
+            quote!()
+        };
+
+        let test_config = format!("wasmtime_wast_util::{:?}", test_config.flags)
+            .parse::<proc_macro2::TokenStream>()
+            .unwrap();
+        let strategy_ident = quote::format_ident!("{strategy_name}");
 
         let tok = quote! {
             #test_attr
             #target
+            #should_panic
             #(#attrs)*
-            #asyncness fn #test_name() #ret {
+            #asyncness fn #test_name() {
                 let mut config = Config::new();
-                config.strategy(Strategy::#ident);
-                #(#config_setup)*
-                #func_name(&mut config) #await_
+                component_test_util::apply_test_config(
+                    &mut config,
+                    &#test_config,
+                );
+                component_test_util::apply_wast_config(
+                    &mut config,
+                    &wasmtime_wast_util::WastConfig {
+                        compiler: wasmtime_wast_util::Compiler::#strategy_ident,
+                        pooling: false,
+                        collector: wasmtime_wast_util::Collector::Auto,
+                    },
+                );
+                #func_name(&mut config) #await_ #expect
             }
         };