Revert the ISA builder to consumer self on "finish", but allow more components to clone themselves

Author: KevinRizzoTO

cranelift/codegen/meta/src/gen_settings.rs

@@ -21,7 +21,7 @@ pub(crate) enum ParentGroup {
 fn gen_constructor(group: &SettingGroup, parent: ParentGroup, fmt: &mut Formatter) {
     let args = match parent {
         ParentGroup::None => "builder: Builder",
-        ParentGroup::Shared => "shared: &settings::Flags, builder: &Builder",
+        ParentGroup::Shared => "shared: &settings::Flags, builder: Builder",
     };
     fmtln!(fmt, "impl Flags {");
     fmt.indent(|fmt| {

cranelift/codegen/src/isa/mod.rs

@@ -152,20 +152,20 @@ pub type Builder = IsaBuilder<CodegenResult<OwnedTargetIsa>>;
 /// Builder for a `TargetIsa`.
 /// Modify the ISA-specific settings before creating the `TargetIsa` trait object with `finish`.
 #[derive(Clone)]
-pub struct IsaBuilder<T> {
+pub struct IsaBuilder<T: Clone> {
     triple: Triple,
     setup: settings::Builder,
-    constructor: fn(Triple, settings::Flags, &settings::Builder) -> T,
+    constructor: fn(Triple, settings::Flags, settings::Builder) -> T,
 }
 
-impl<T> IsaBuilder<T> {
+impl<T> IsaBuilder<T> where T: Clone {
     /// Creates a new ISA-builder from its components, namely the `triple` for
     /// the ISA, the ISA-specific settings builder, and a final constructor
     /// function to generate the ISA from its components.
     pub fn new(
         triple: Triple,
         setup: settings::Builder,
-        constructor: fn(Triple, settings::Flags, &settings::Builder) -> T,
+        constructor: fn(Triple, settings::Flags, settings::Builder) -> T,
     ) -> Self {
         IsaBuilder {
             triple,
@@ -190,12 +190,12 @@ impl<T> IsaBuilder<T> {
     /// flags are inconsistent or incompatible: for example, some
     /// platform-independent features, like general SIMD support, may
     /// need certain ISA extensions to be enabled.
-    pub fn finish(&self, shared_flags: settings::Flags) -> T {
-        (self.constructor)(self.triple.clone(), shared_flags, &self.setup)
+    pub fn finish(self, shared_flags: settings::Flags) -> T {
+        (self.constructor)(self.triple.clone(), shared_flags, self.setup)
     }
 }
 
-impl<T> settings::Configurable for IsaBuilder<T> {
+impl<T> settings::Configurable for IsaBuilder<T> where T: Clone {
     fn set(&mut self, name: &str, value: &str) -> SetResult<()> {
         self.setup.set(name, value)
     }

cranelift/codegen/src/isa/riscv64/inst/emit_tests.rs

@@ -2095,7 +2095,7 @@ fn make_test_flags() -> (settings::Flags, super::super::riscv_settings::Flags) {
     let b = settings::builder();
     let flags = settings::Flags::new(b.clone());
     let b2 = super::super::riscv_settings::builder();
-    let isa_flags = super::super::riscv_settings::Flags::new(&flags, &b2);
+    let isa_flags = super::super::riscv_settings::Flags::new(&flags, b2);
     (flags, isa_flags)
 }
 

cranelift/codegen/src/isa/s390x/inst/emit_tests.rs

@@ -13362,7 +13362,7 @@ fn test_s390x_binemit() {
     use crate::settings::Configurable;
     let mut isa_flag_builder = s390x_settings::builder();
     isa_flag_builder.enable("arch13").unwrap();
-    let isa_flags = s390x_settings::Flags::new(&flags, &isa_flag_builder);
+    let isa_flags = s390x_settings::Flags::new(&flags, isa_flag_builder);
 
     let emit_info = EmitInfo::new(isa_flags);
     for (insn, expected_encoding, expected_printing) in insns {

cranelift/codegen/src/isa/unwind/systemv.rs

@@ -14,7 +14,7 @@ type Register = u16;
 
 /// Enumerate the errors possible in mapping Cranelift registers to their DWARF equivalent.
 #[allow(missing_docs)]
-#[derive(Debug, PartialEq, Eq)]
+#[derive(Debug, PartialEq, Eq, Clone)]
 pub enum RegisterMappingError {
     MissingBank,
     UnsupportedArchitecture,

cranelift/codegen/src/isa/x64/inst/emit_tests.rs

@@ -5155,7 +5155,7 @@ fn test_x64_emit() {
     isa_flag_builder.enable("has_avx512f").unwrap();
     isa_flag_builder.enable("has_avx512vbmi").unwrap();
     isa_flag_builder.enable("has_avx512vl").unwrap();
-    let isa_flags = x64::settings::Flags::new(&flags, &isa_flag_builder);
+    let isa_flags = x64::settings::Flags::new(&flags, isa_flag_builder);
 
     let emit_info = EmitInfo::new(flags, isa_flags);
     for (insn, expected_encoding, expected_printing) in insns {

cranelift/codegen/src/result.rs

@@ -8,7 +8,7 @@ use std::string::String;
 /// A compilation error.
 ///
 /// When Cranelift fails to compile a function, it will return one of these error codes.
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub enum CodegenError {
     /// A list of IR verifier errors.
     ///

crates/cranelift-shared/src/isa_builder.rs

@@ -9,7 +9,7 @@ use wasmtime_environ::{Setting, SettingKind};
 ///
 /// Most methods are mirrored from the `wasmtime_environ::CompilerBuilder` trait, so look there for more
 /// information.
-pub struct IsaBuilder<T> {
+pub struct IsaBuilder<T: Clone> {
     /// The shared flags that all targets share.
     shared_flags: settings::Builder,
     /// The internal ISA builder for the current target.
@@ -18,7 +18,7 @@ pub struct IsaBuilder<T> {
     pub lookup: fn(Triple) -> Result<Builder<T>>,
 }
 
-impl<T> IsaBuilder<T> {
+impl<T> IsaBuilder<T> where T: Clone {
     /// Create a new ISA builder with the given lookup function.
     pub fn new(lookup: fn(Triple) -> Result<Builder<T>>) -> Self {
         let mut flags = settings::builder();
@@ -93,7 +93,7 @@ impl<T> IsaBuilder<T> {
     }
 
     pub fn build(&self) -> T {
-        self.inner
+        self.inner.clone()
             .finish(settings::Flags::new(self.shared_flags.clone()))
     }