fix(wasmtime):Config methods should be idempotent

RELEASES.md

@@ -8,6 +8,17 @@ Unreleased.
 
 ### Changed
 
+* The `Config::strategy`, `Config::cranelift_flag_enable`, `Config::cranelift_flag_set` 
+ and `Config::profiler` 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.
@@ -238,7 +238,7 @@ WASMTIME_CONFIG_PROP(void, cranelift_opt_level, wasmtime_opt_level_t)
  *
  * This setting in #WASMTIME_PROFILING_STRATEGY_NONE by default.
  */
-WASMTIME_CONFIG_PROP(wasmtime_error_t*, profiler, wasmtime_profiling_strategy_t)
+WASMTIME_CONFIG_PROP(void, profiler, wasmtime_profiling_strategy_t)
 
 /**
  * \brief Configures the maximum size for memory to be considered "static"

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]
@@ -145,13 +144,12 @@ pub extern "C" fn wasmtime_config_cranelift_opt_level_set(
 pub extern "C" fn wasmtime_config_profiler_set(
  c: &mut wasm_config_t,
  strategy: wasmtime_profiling_strategy_t,
-) -> Option<Box<wasmtime_error_t>> {
+) {
  use wasmtime_profiling_strategy_t::*;
- let result = c.config.profiler(match strategy {
+ c.config.profiler(match strategy {
  WASMTIME_PROFILING_STRATEGY_NONE => ProfilingStrategy::None,
  WASMTIME_PROFILING_STRATEGY_JITDUMP => ProfilingStrategy::JitDump,
  });
- handle_result(result, |_cfg| {})
 }
 
 #[no_mangle]

crates/cli-flags/src/lib.rs

@@ -260,20 +260,20 @@ impl CommonOptions {
  .cranelift_debug_verifier(self.enable_cranelift_debug_verifier)
  .debug_info(self.debug_info)
  .cranelift_opt_level(self.opt_level())
- .profiler(pick_profiling_strategy(self.jitdump, self.vtune)?)?
+ .profiler(pick_profiling_strategy(self.jitdump, self.vtune)?)
  .cranelift_nan_canonicalization(self.enable_cranelift_nan_canonicalization);
 
  self.enable_wasm_features(&mut config);
 
  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);
  }
  }
  }