From f481dd98a1b5a9682a94eafe4d1dedfad24a82ce Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 6 Oct 2023 18:00:12 -0500 Subject: [PATCH] Support multiple versions in `bindgen!` (#7172) This commit fixes a bug in the `bindgen!` macro where when faced with multiple packages that differ only in version number invalid bindings were generated. The fix here is to add version number information to package module names if necessary in situations such as this. This required some refactoring internally to have a single source of truth for what the name of a module should be and avoid having it implicitly calculated in two locations. --- .../component-macro/test-helpers/src/lib.rs | 2 +- .../codegen/multiversion/deps/v1/root.wit | 5 + .../codegen/multiversion/deps/v2/root.wit | 5 + .../tests/codegen/multiversion/root.wit | 6 + crates/wit-bindgen/src/lib.rs | 235 ++++++++++-------- 5 files changed, 149 insertions(+), 104 deletions(-) create mode 100644 crates/component-macro/tests/codegen/multiversion/deps/v1/root.wit create mode 100644 crates/component-macro/tests/codegen/multiversion/deps/v2/root.wit create mode 100644 crates/component-macro/tests/codegen/multiversion/root.wit diff --git a/crates/component-macro/test-helpers/src/lib.rs b/crates/component-macro/test-helpers/src/lib.rs index 28ea3ca4652..9029b90bc68 100644 --- a/crates/component-macro/test-helpers/src/lib.rs +++ b/crates/component-macro/test-helpers/src/lib.rs @@ -10,7 +10,7 @@ pub fn foreach(input: TokenStream) -> TokenStream { let mut result = Vec::new(); for f in cwd.read_dir().unwrap() { let f = f.unwrap().path(); - if f.extension().and_then(|s| s.to_str()) == Some("wit") { + if f.extension().and_then(|s| s.to_str()) == Some("wit") || f.is_dir() { let name = f.file_stem().unwrap().to_str().unwrap(); let ident = Ident::new(&name.replace("-", "_"), Span::call_site()); let path = f.to_str().unwrap(); diff --git a/crates/component-macro/tests/codegen/multiversion/deps/v1/root.wit b/crates/component-macro/tests/codegen/multiversion/deps/v1/root.wit new file mode 100644 index 00000000000..cdda0a38b4b --- /dev/null +++ b/crates/component-macro/tests/codegen/multiversion/deps/v1/root.wit @@ -0,0 +1,5 @@ +package my:dep@0.1.0; + +interface a { + x: func(); +} diff --git a/crates/component-macro/tests/codegen/multiversion/deps/v2/root.wit b/crates/component-macro/tests/codegen/multiversion/deps/v2/root.wit new file mode 100644 index 00000000000..fa8fbeb2df6 --- /dev/null +++ b/crates/component-macro/tests/codegen/multiversion/deps/v2/root.wit @@ -0,0 +1,5 @@ +package my:dep@0.2.0; + +interface a { + x: func(); +} diff --git a/crates/component-macro/tests/codegen/multiversion/root.wit b/crates/component-macro/tests/codegen/multiversion/root.wit new file mode 100644 index 00000000000..ff23acf81cc --- /dev/null +++ b/crates/component-macro/tests/codegen/multiversion/root.wit @@ -0,0 +1,6 @@ +package foo:bar; + +world foo { + import my:dep/a@0.1.0; + import my:dep/a@0.2.0; +} diff --git a/crates/wit-bindgen/src/lib.rs b/crates/wit-bindgen/src/lib.rs index 7183e0bf232..0b1e286523a 100644 --- a/crates/wit-bindgen/src/lib.rs +++ b/crates/wit-bindgen/src/lib.rs @@ -27,20 +27,22 @@ mod source; mod types; use source::Source; -struct InterfaceName { - /// True when this interface name has been remapped through the use of `with` in the `bindgen!` - /// macro invocation. - remapped: bool, - - /// The string name for this interface. - path: String, +#[derive(Clone)] +enum InterfaceName { + /// This interface was remapped using `with` to some other Rust code. + Remapped { + name_at_root: String, + local_path: Vec, + }, + /// This interface is generated in the module hierarchy specified. + Path(Vec), } #[derive(Default)] struct Wasmtime { src: Source, opts: Opts, - import_interfaces: BTreeMap, Vec>, + import_interfaces: Vec<(String, InterfaceName)>, import_functions: Vec, exports: Exports, types: Types, @@ -51,10 +53,6 @@ struct Wasmtime { trappable_errors: IndexMap, } -struct ImportInterface { - snake: String, - module: String, -} struct ImportFunction { add_to_linker: String, sig: String, @@ -63,7 +61,7 @@ struct ImportFunction { #[derive(Default)] struct Exports { fields: BTreeMap, - modules: BTreeMap, Vec>, + modules: Vec<(String, InterfaceName)>, funcs: Vec, } @@ -178,45 +176,85 @@ impl Wasmtime { is_export: bool, ) -> bool { let with_name = resolve.name_world_key(name); + + let mut path = Vec::new(); + if is_export { + path.push("exports".to_string()); + } + match name { + WorldKey::Name(name) => { + path.push(name.to_snake_case()); + } + WorldKey::Interface(_) => { + let iface = &resolve.interfaces[id]; + let pkgname = &resolve.packages[iface.package.unwrap()].name; + path.push(pkgname.namespace.to_snake_case()); + path.push(self.name_package_module(resolve, iface.package.unwrap())); + path.push(iface.name.as_ref().unwrap().to_snake_case()); + } + } let entry = if let Some(remapped_path) = self.opts.with.get(&with_name) { let name = format!("__with_name{}", self.with_name_counter); self.with_name_counter += 1; uwriteln!(self.src, "use {remapped_path} as {name};"); - InterfaceName { - remapped: true, - path: name, + InterfaceName::Remapped { + name_at_root: name, + local_path: path, } } else { - let path = match name { - WorldKey::Name(name) => name.to_snake_case(), - WorldKey::Interface(_) => { - let iface = &resolve.interfaces[id]; - let pkgname = &resolve.packages[iface.package.unwrap()].name; - format!( - "{}::{}::{}", - pkgname.namespace.to_snake_case(), - pkgname.name.to_snake_case(), - iface.name.as_ref().unwrap().to_snake_case() - ) - } - }; - let path = if is_export { - format!("exports::{path}") - } else { - path - }; - InterfaceName { - remapped: false, - path, - } + InterfaceName::Path(path) }; - let remapped = entry.remapped; + let remapped = matches!(entry, InterfaceName::Remapped { .. }); self.interface_names.insert(id, entry); - remapped } + /// If the package `id` is the only package with its namespace/name combo + /// then pass through the name unmodified. If, however, there are multiple + /// versions of this package then the package module is going to get version + /// information. + fn name_package_module(&self, resolve: &Resolve, id: PackageId) -> String { + let pkg = &resolve.packages[id]; + let versions_with_same_name = resolve + .packages + .iter() + .filter_map(|(_, p)| { + if p.name.namespace == pkg.name.namespace && p.name.name == pkg.name.name { + Some(&p.name.version) + } else { + None + } + }) + .collect::>(); + let base = pkg.name.name.to_snake_case(); + if versions_with_same_name.len() == 1 { + return base; + } + + let version = match &pkg.name.version { + Some(version) => version, + // If this package didn't have a version then don't mangle its name + // and other packages with the same name but with versions present + // will have their names mangled. + None => return base, + }; + + // Here there's multiple packages with the same name that differ only in + // version, so the version needs to be mangled into the Rust module name + // that we're generating. This in theory could look at all of + // `versions_with_same_name` and produce a minimal diff, e.g. for 0.1.0 + // and 0.2.0 this could generate "foo1" and "foo2", but for now + // a simpler path is chosen to generate "foo0_1_0" and "foo0_2_0". + let version = version + .to_string() + .replace('.', "_") + .replace('-', "_") + .replace('+', "_") + .to_snake_case(); + format!("{base}{version}") + } + fn generate(&mut self, resolve: &Resolve, id: WorldId) -> String { self.types.analyze(resolve, id); for (i, te) in self.opts.trappable_error_type.iter().enumerate() { @@ -235,6 +273,7 @@ impl Wasmtime { self.import(resolve, id, name, import); } } + for (name, export) in world.exports.iter() { if !self.opts.only_interfaces || matches!(export, WorldItem::Interface(_)) { self.export(resolve, name, export); @@ -291,15 +330,8 @@ impl Wasmtime { }} " ); - let pkg = resolve.interfaces[*id].package.unwrap(); - let pkgname = match name { - WorldKey::Name(_) => None, - WorldKey::Interface(_) => Some(resolve.packages[pkg].name.clone()), - }; self.import_interfaces - .entry(pkgname) - .or_insert(Vec::new()) - .push(ImportInterface { snake, module }); + .push((module, self.interface_names[id].clone())); } WorldItem::Type(ty) => { let name = match name { @@ -433,9 +465,7 @@ impl Wasmtime { }; self.exports .modules - .entry(pkgname.clone()) - .or_insert(Vec::new()) - .push(module); + .push((module, self.interface_names[id].clone())); let name = resolve.name_world_key(name); let (path, method_name) = match pkgname { @@ -565,18 +595,10 @@ impl Wasmtime { } let imports = mem::take(&mut self.import_interfaces); - self.emit_modules( - &imports - .into_iter() - .map(|(k, v)| (k, v.into_iter().map(|m| m.module).collect())) - .collect(), - ); - if !self.exports.modules.is_empty() { - uwriteln!(self.src, "pub mod exports {{"); - let exports = mem::take(&mut self.exports.modules); - self.emit_modules(&exports); - uwriteln!(self.src, "}}"); - } + self.emit_modules(imports); + + let exports = mem::take(&mut self.exports.modules); + self.emit_modules(exports); let mut src = mem::take(&mut self.src); if self.opts.rustfmt { @@ -606,34 +628,39 @@ impl Wasmtime { src.into() } - fn emit_modules(&mut self, modules: &BTreeMap, Vec>) { - let mut map = BTreeMap::new(); - for (pkg, modules) in modules { - match pkg { - Some(pkg) => { - let prev = map - .entry(&pkg.namespace) - .or_insert(BTreeMap::new()) - .insert(&pkg.name, modules); - assert!(prev.is_none()); - } - None => { - for module in modules { - uwriteln!(self.src, "{module}"); - } - } + fn emit_modules(&mut self, modules: Vec<(String, InterfaceName)>) { + #[derive(Default)] + struct Module { + submodules: BTreeMap, + contents: Vec, + } + let mut map = Module::default(); + for (module, name) in modules { + let path = match name { + InterfaceName::Remapped { local_path, .. } => local_path, + InterfaceName::Path(path) => path, + }; + let mut cur = &mut map; + for name in path[..path.len() - 1].iter() { + cur = cur + .submodules + .entry(name.clone()) + .or_insert(Module::default()); } + cur.contents.push(module); } - for (ns, pkgs) in map { - uwriteln!(self.src, "pub mod {} {{", ns.to_snake_case()); - for (pkg, modules) in pkgs { - uwriteln!(self.src, "pub mod {} {{", pkg.to_snake_case()); - for module in modules { - uwriteln!(self.src, "{module}"); - } - uwriteln!(self.src, "}}"); + + emit(&mut self.src, map); + + fn emit(me: &mut Source, module: Module) { + for (name, submodule) in module.submodules { + uwriteln!(me, "pub mod {name} {{"); + emit(me, submodule); + uwriteln!(me, "}}"); + } + for submodule in module.contents { + uwriteln!(me, "{submodule}"); } - uwriteln!(self.src, "}}"); } } } @@ -675,19 +702,12 @@ impl Wasmtime { return; } let mut interfaces = Vec::new(); - for (pkg, imports) in self.import_interfaces.iter() { - for import in imports { - let mut path = String::new(); - if let Some(pkg) = pkg { - path.push_str(&pkg.namespace.to_snake_case()); - path.push_str("::"); - path.push_str(&pkg.name.to_snake_case()); - path.push_str("::"); - } - - path.push_str(&import.snake); - interfaces.push(path); - } + for (_, name) in self.import_interfaces.iter() { + let path = match name { + InterfaceName::Remapped { .. } => unreachable!("imported a remapped module"), + InterfaceName::Path(path) => path, + }; + interfaces.push(path.join("::")); } uwrite!( @@ -1951,8 +1971,17 @@ impl<'a> RustGenerator<'a> for InterfaceGenerator<'a> { } } let mut path_to_root = self.path_to_root(); - let InterfaceName { path, .. } = &self.gen.interface_names[&interface]; - path_to_root.push_str(path); + match &self.gen.interface_names[&interface] { + InterfaceName::Remapped { name_at_root, .. } => path_to_root.push_str(name_at_root), + InterfaceName::Path(path) => { + for (i, name) in path.iter().enumerate() { + if i > 0 { + path_to_root.push_str("::"); + } + path_to_root.push_str(name); + } + } + } Some(path_to_root) }