From 83a8ca77cd291acfa9beeea81930a365899b13d8 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 10 Apr 2023 11:27:13 -0500 Subject: [PATCH] Mirror default-owned change for guests in `wit-bindgen` (#6189) This commit is a mirror of bytecodealliance/wit-bindgen#547 into the `bindgen!` macro for Wasmtime. The new default is to generate only one Rust type per WIT type input, regardless of if the representation can be slightly more optimal in niche cases with more borrows. This should make the macro easier to work with in the limit ideally. Closes #6124 --- crates/component-macro/src/bindgen.rs | 9 ++++ crates/component-macro/tests/codegen.rs | 1 + crates/wit-bindgen/src/lib.rs | 9 ++++ crates/wit-bindgen/src/rust.rs | 63 +++++++++++++++++++++---- 4 files changed, 73 insertions(+), 9 deletions(-) diff --git a/crates/component-macro/src/bindgen.rs b/crates/component-macro/src/bindgen.rs index bb82b1750a..76e06c80d7 100644 --- a/crates/component-macro/src/bindgen.rs +++ b/crates/component-macro/src/bindgen.rs @@ -76,6 +76,7 @@ impl Parse for Config { Opt::Tracing(val) => opts.tracing = val, Opt::Async(val) => opts.async_ = val, Opt::TrappableErrorType(val) => opts.trappable_error_type = val, + Opt::DuplicateIfNecessary(val) => opts.duplicate_if_necessary = val, } } } else { @@ -131,6 +132,7 @@ mod kw { syn::custom_keyword!(tracing); syn::custom_keyword!(trappable_error_type); syn::custom_keyword!(world); + syn::custom_keyword!(duplicate_if_necessary); } enum Opt { @@ -140,6 +142,7 @@ enum Opt { Tracing(bool), Async(bool), TrappableErrorType(Vec), + DuplicateIfNecessary(bool), } impl Parse for Opt { @@ -165,6 +168,12 @@ impl Parse for Opt { input.parse::()?; input.parse::()?; Ok(Opt::Async(input.parse::()?.value)) + } else if l.peek(kw::duplicate_if_necessary) { + input.parse::()?; + input.parse::()?; + Ok(Opt::DuplicateIfNecessary( + input.parse::()?.value, + )) } else if l.peek(kw::trappable_error_type) { input.parse::()?; input.parse::()?; diff --git a/crates/component-macro/tests/codegen.rs b/crates/component-macro/tests/codegen.rs index 695f432fef..0384ab4c14 100644 --- a/crates/component-macro/tests/codegen.rs +++ b/crates/component-macro/tests/codegen.rs @@ -18,6 +18,7 @@ macro_rules! gentest { path: $path, world: $name, tracing: true, + duplicate_if_necessary: true, }); } } diff --git a/crates/wit-bindgen/src/lib.rs b/crates/wit-bindgen/src/lib.rs index 426c0a58ff..3d516d7d18 100644 --- a/crates/wit-bindgen/src/lib.rs +++ b/crates/wit-bindgen/src/lib.rs @@ -61,6 +61,11 @@ pub struct Opts { /// A list of "trappable errors" which are used to replace the `E` in /// `result` found in WIT. pub trappable_error_type: Vec, + + /// Whether or not to generate "duplicate" type definitions for a single + /// WIT type if necessary, for example if it's used as both an import and an + /// export. + pub duplicate_if_necessary: bool, } #[derive(Debug, Clone)] @@ -1385,6 +1390,10 @@ impl<'a> RustGenerator<'a> for InterfaceGenerator<'a> { self.resolve } + fn duplicate_if_necessary(&self) -> bool { + self.gen.opts.duplicate_if_necessary + } + fn path_to_interface(&self, interface: InterfaceId) -> Option { match self.current_interface { Some(id) if id == interface => None, diff --git a/crates/wit-bindgen/src/rust.rs b/crates/wit-bindgen/src/rust.rs index bba7df58b7..e1f602ac35 100644 --- a/crates/wit-bindgen/src/rust.rs +++ b/crates/wit-bindgen/src/rust.rs @@ -17,6 +17,16 @@ pub trait RustGenerator<'a> { fn info(&self, ty: TypeId) -> TypeInfo; fn path_to_interface(&self, interface: InterfaceId) -> Option; + /// This, if enabled, will possibly cause types to get duplicate copies to + /// get generated of each other. For example a record containing a string + /// used both in the import and export context would get one variant + /// generated for both. + /// + /// If this is disabled then the import context would require the same type + /// used for the export context, which has an owned string that might not + /// otherwise be necessary. + fn duplicate_if_necessary(&self) -> bool; + fn print_ty(&mut self, ty: &Type, mode: TypeMode) { match ty { Type::Id(t) => self.print_tyid(*t, mode), @@ -58,6 +68,20 @@ pub trait RustGenerator<'a> { let lt = self.lifetime_for(&info, mode); let ty = &self.resolve().types[id]; if ty.name.is_some() { + // If this type has a list internally, no lifetime is being printed, + // but we're in a borrowed mode, then that means we're in a borrowed + // context and don't want ownership of the type but we're using an + // owned type definition. Inject a `&` in front to indicate that, at + // the API level, ownership isn't required. + if info.has_list && lt.is_none() { + if let TypeMode::AllBorrowed(lt) = mode { + self.push_str("&"); + if lt != "'_" { + self.push_str(lt); + self.push_str(" "); + } + } + } let name = if lt.is_some() { self.param_name(id) } else { @@ -197,12 +221,19 @@ pub trait RustGenerator<'a> { fn modes_of(&self, ty: TypeId) -> Vec<(String, TypeMode)> { let info = self.info(ty); - let mut result = Vec::new(); - if info.borrowed { - result.push((self.param_name(ty), TypeMode::AllBorrowed("'a"))); + if !info.owned && !info.borrowed { + return Vec::new(); } - if info.owned && (!info.borrowed || self.uses_two_names(&info)) { - result.push((self.result_name(ty), TypeMode::Owned)); + let mut result = Vec::new(); + let first_mode = if info.owned || !info.borrowed { + TypeMode::Owned + } else { + assert!(!self.uses_two_names(&info)); + TypeMode::AllBorrowed("'a") + }; + result.push((self.result_name(ty), first_mode)); + if self.uses_two_names(&info) { + result.push((self.param_name(ty), TypeMode::AllBorrowed("'a"))); } return result; } @@ -347,13 +378,27 @@ pub trait RustGenerator<'a> { } fn uses_two_names(&self, info: &TypeInfo) -> bool { - info.has_list && info.borrowed && info.owned + info.has_list && info.borrowed && info.owned && self.duplicate_if_necessary() } fn lifetime_for(&self, info: &TypeInfo, mode: TypeMode) -> Option<&'static str> { - match mode { - TypeMode::AllBorrowed(s) if info.has_list => Some(s), - _ => None, + let lt = match mode { + TypeMode::AllBorrowed(s) => s, + _ => return None, + }; + // No lifetimes needed unless this has a list. + if !info.has_list { + return None; + } + // If two names are used then this type will have an owned and a + // borrowed copy and the borrowed copy is being used, so it needs a + // lifetime. Otherwise if it's only borrowed and not owned then this can + // also use a lifetime since it's not needed in two contexts and only + // the borrowed version of the structure was generated. + if self.uses_two_names(info) || (info.borrowed && !info.owned) { + Some(lt) + } else { + None } } }