fix(wasmtime):Config methods should be idempotent

This commit refactored `Config` to use a seperate `CompilerConfig` field instead
of operating on `CompilerBuilder` directly to make all its methods idempotent.

Fixes #4189

RELEASES.md

@@ -8,6 +8,16 @@ Unreleased.
 
 ### Changed
 
+* The `Config::strategy`, `Config::cranelift_flag_enable` and `Config::cranelift_flag_set` APIs now return `&mut Self`
+  instead of `Result<&mut Self>` since the validation is deferred until `Engine::new`.
+  [#4252](https://github.com/bytecodealliance/wasmtime/pull/4252)
+
+### Fixed
+
+* A refactor of `Config` was made to fix an issue that the order of calls to `Config`
+  matters now, which may lead to unexpected behavior.
+  [#4252](https://github.com/bytecodealliance/wasmtime/pull/4252)
+
 --------------------------------------------------------------------------------
 
 ## 0.38.0

crates/c-api/include/wasmtime/config.h

@@ -201,7 +201,7 @@ WASMTIME_CONFIG_PROP(void, wasm_memory64, bool)
  * If the compilation strategy selected could not be enabled then an error is
  * returned.
  */
-WASMTIME_CONFIG_PROP(wasmtime_error_t*, strategy, wasmtime_strategy_t)
+WASMTIME_CONFIG_PROP(void, strategy, wasmtime_strategy_t)
 
 /**
  * \brief Configures whether Cranelift's debug verifier is enabled.

crates/c-api/src/config.rs

@@ -103,13 +103,12 @@ pub extern "C" fn wasmtime_config_wasm_memory64_set(c: &mut wasm_config_t, enabl
 pub extern "C" fn wasmtime_config_strategy_set(
     c: &mut wasm_config_t,
     strategy: wasmtime_strategy_t,
-) -> Option<Box<wasmtime_error_t>> {
+) {
     use wasmtime_strategy_t::*;
-    let result = c.config.strategy(match strategy {
+    c.config.strategy(match strategy {
         WASMTIME_STRATEGY_AUTO => Strategy::Auto,
         WASMTIME_STRATEGY_CRANELIFT => Strategy::Cranelift,
     });
-    handle_result(result, |_cfg| {})
 }
 
 #[no_mangle]

crates/cli-flags/src/lib.rs

@@ -267,13 +267,13 @@ impl CommonOptions {
 
         for name in &self.cranelift_enable {
             unsafe {
-                config.cranelift_flag_enable(name)?;
+                config.cranelift_flag_enable(name);
             }
         }
 
         for (name, value) in &self.cranelift_set {
             unsafe {
-                config.cranelift_flag_set(name, value)?;
+                config.cranelift_flag_set(name, value);
             }
         }
 

crates/cranelift/src/builder.rs

@@ -9,7 +9,6 @@ use cranelift_codegen::settings::{self, Configurable, SetError};
 use std::fmt;
 use wasmtime_environ::{CompilerBuilder, Setting, SettingKind};
 
-#[derive(Clone)]
 struct Builder {
     flags: settings::Builder,
     isa_flags: isa::Builder,
@@ -55,10 +54,6 @@ impl CompilerBuilder for Builder {
         self.isa_flags.triple()
     }
 
-    fn clone(&self) -> Box<dyn CompilerBuilder> {
-        Box::new(Clone::clone(self))
-    }
-
     fn target(&mut self, target: target_lexicon::Triple) -> Result<()> {
         self.isa_flags = isa::lookup(target)?;
         Ok(())

crates/environ/src/compilation.rs

@@ -76,9 +76,6 @@ pub enum CompileError {
 /// This is used in Wasmtime to separate compiler implementations, currently
 /// mostly used to separate Cranelift from Wasmtime itself.
 pub trait CompilerBuilder: Send + Sync + fmt::Debug {
-    /// Like the `Clone` trait, but for the boxed trait object.
-    fn clone(&self) -> Box<dyn CompilerBuilder>;
-
     /// Sets the target of compilation to the target specified.
     fn target(&mut self, target: target_lexicon::Triple) -> Result<()>;
 

crates/fuzzing/src/generators.rs

@@ -421,8 +421,7 @@ impl Config {
 
         if self.wasmtime.force_jump_veneers {
             unsafe {
-                cfg.cranelift_flag_set("wasmtime_linkopt_force_jump_veneer", "true")
-                    .unwrap();
+                cfg.cranelift_flag_set("wasmtime_linkopt_force_jump_veneer", "true");
             }
         }
 
@@ -431,8 +430,7 @@ impl Config {
                 cfg.cranelift_flag_set(
                     "wasmtime_linkopt_padding_between_functions",
                     &pad.to_string(),
-                )
-                .unwrap();
+                );
             }
         }
 
@@ -658,7 +656,7 @@ impl CodegenSettings {
                 config.target(target).unwrap();
                 for (key, value) in flags {
                     unsafe {
-                        config.cranelift_flag_set(key, value).unwrap();
+                        config.cranelift_flag_set(key, value);
                     }
                 }
             }