From 9129cf76a6d277eb41feafa4a43b77a2815990dc Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 6 Apr 2023 11:07:40 -0700 Subject: [PATCH 1/2] wit-component: Implement merging `Resolve`s together This commit addresses the fundamental issue exposed by #967, namely that worlds were not managed well between adapters and the "main module". On the surface the issue is that wit-component maintains worlds separately for each adapter and for the main module, and then ends up tripping over itself when the worlds overlap in imports, for example. The first fix applied to handle this was to refactor wit-component to instead merge the worlds of the adapters into the main module's world. This merging operation is similar to the union-of-worlds proposed in WebAssembly/component-model#169. Prior to this commit, however, the merge operation between worlds was pretty naive and would bail out if there was any overlap at all. This meant that the panic from #967 was turned into a first-class error message, but the use case shouldn't return an error and instead should succeed as the underlying packages and definitions were all shared. This comes back to #962 which is an initial attempt at merging worlds which are structurally equivalent. The full implementation of this, however, was deeper than what #962 uncovered and further required updating merging. One of the issues with #962 (and initial implementations of this commit) is that a `Resolve` would still have multiple `Interface`s and `TypeDef`s and such floating around which are all the same where some were referred to by worlds and some weren't. What this commit chose to address this was to update the `Resolve::merge` operation. Previously this was an infallible simple "move the things from A to B" operation but now it's a much more involved "weaving things together" operation where overlap between A and B doesn't copy between them but instead uses items in-place. For example if a document in A isn't present in B, it's still added like before. If the document A is already present in B, though, then it's no longer added and instead it's recursed within to see if extra items are added to the new document. This new "fancy" merge operation is intended to be a more long-term implementation of this. At a high level this enables projects to separately depend on WASI, for example, without every project coordinating and using the exact same WIT-level dependency within one build invocation (which isn't possible with separate compilation). All WASI worlds will, in the end, get merged together into one picture of the WASI world when a component is produced. This was a fair bit of support which wasn't easily supported or tested before. I've modified the `components` test suite to be able to have shared WIT dependencies between the adapter and the main module to add tests for that. Additionally I've added a dedicated set of tests for just merging resolves which showcases some of the features at this time. With all that said, there are a few aspects and limitations of merging resolves that are worth mentioning: * Packages are merged as keyed by their name and URL. For now this basically means that only packages under `deps/` are candidates for merging (as only they have URLs) and they must be placed in same-named subfolders. * When merging packages are allowed to pick up more documents. This is intended to model where each resolve may have a subset of the whole picture file-wise. The use case in #967 specifically exercises this, for example. * When merging documents it's allowed for new interfaces and worlds not previously present in the document to get merged in. This is intended to model a bit of forward-compatible-ness where for example the preview1 adapter may have been built against a historical version of WASI whereas a project using that may use a bleeding edge version that has more interfaces. This merge should still be ok since they're both only using what they declared, so by adding new interfaces and worlds it shouldn't ever tamper with the others' view of the world. * Merging interfaces and worlds, however, currently require exact "structural equivalence". The thinking behind this is that if one component exports an `interface` but then a merge operation suddenly adds items to that `interface` then the component would no longer validly export the interface. This should be possible for the converse, though, where adding items to an interface that's only ever imported should be valid. This doesn't implement this sort of detection logic and takes a more conservative stance that interfaces and worlds must exactly match. * Currently merging does not actually test for structural equivalence beyond top-level names. That's deferred to later if necessary, for example by updating #962 to change where the tests happen. The result of all this is to fix the panic found in #967 and more generally enable adapters and main modules in `wit-component` to import/use the same WIT interfaces. This means it's now possible to have a project use new preview2 interfaces with a preview1 standard library and the preview1 adapter and everything should work. Closes #962 Closes #967 --- crates/wit-component/src/encoding.rs | 84 ++- crates/wit-component/src/encoding/types.rs | 1 + crates/wit-component/src/encoding/world.rs | 75 +-- crates/wit-component/src/metadata.rs | 6 +- crates/wit-component/src/validation.rs | 11 +- crates/wit-component/tests/components.rs | 42 +- .../adapt-old.wat | 23 + .../adapt-old.wit | 9 + .../component.wat | 207 +++++++ .../component.wit | 35 ++ .../deps/shared-dependency/doc.wit | 11 + .../deps/shared-dependency/types.wit | 3 + .../module.wat | 15 + .../module.wit | 9 + crates/wit-component/tests/merge.rs | 181 +++++++ .../tests/merge/bad-interface1/error.txt | 6 + .../tests/merge/bad-interface1/from/a.wit | 3 + .../merge/bad-interface1/from/deps/foo/a.wit | 4 + .../tests/merge/bad-interface1/into/a.wit | 3 + .../merge/bad-interface1/into/deps/foo/a.wit | 4 + .../tests/merge/bad-interface2/error.txt | 6 + .../tests/merge/bad-interface2/from/a.wit | 3 + .../merge/bad-interface2/from/deps/foo/a.wit | 6 + .../tests/merge/bad-interface2/into/a.wit | 3 + .../merge/bad-interface2/into/deps/foo/a.wit | 6 + .../tests/merge/bad-world1/error.txt | 6 + .../tests/merge/bad-world1/from/a.wit | 3 + .../merge/bad-world1/from/deps/foo/a.wit | 7 + .../tests/merge/bad-world1/into/a.wit | 3 + .../merge/bad-world1/into/deps/foo/a.wit | 7 + .../tests/merge/bad-world2/error.txt | 6 + .../tests/merge/bad-world2/from/a.wit | 3 + .../merge/bad-world2/from/deps/foo/a.wit | 6 + .../tests/merge/bad-world2/into/a.wit | 3 + .../merge/bad-world2/into/deps/foo/a.wit | 7 + .../tests/merge/bad-world3/error.txt | 6 + .../tests/merge/bad-world3/from/a.wit | 3 + .../merge/bad-world3/from/deps/foo/a.wit | 6 + .../tests/merge/bad-world3/into/a.wit | 3 + .../merge/bad-world3/into/deps/foo/a.wit | 7 + .../tests/merge/bad-world4/error.txt | 6 + .../tests/merge/bad-world4/from/a.wit | 3 + .../merge/bad-world4/from/deps/foo/a.wit | 7 + .../tests/merge/bad-world4/into/a.wit | 3 + .../merge/bad-world4/into/deps/foo/a.wit | 7 + .../tests/merge/bad-world5/error.txt | 7 + .../tests/merge/bad-world5/from/a.wit | 3 + .../merge/bad-world5/from/deps/foo/a.wit | 9 + .../tests/merge/bad-world5/into/a.wit | 3 + .../merge/bad-world5/into/deps/foo/a.wit | 7 + .../tests/merge/success/from/a.wit | 5 + .../merge/success/from/deps/foo/only-from.wit | 5 + .../merge/success/from/deps/foo/shared.wit | 23 + .../success/from/deps/only-from-dep/a.wit | 3 + .../tests/merge/success/into/b.wit | 5 + .../merge/success/into/deps/foo/only-into.wit | 5 + .../merge/success/into/deps/foo/shared.wit | 21 + .../merge/success/merge/foo/only-from.wit | 7 + .../merge/success/merge/foo/only-into.wit | 7 + .../tests/merge/success/merge/foo/shared.wit | 31 ++ .../tests/merge/success/merge/from/a.wit | 5 + .../tests/merge/success/merge/into/b.wit | 5 + .../merge/success/merge/only-from-dep/a.wit | 5 + crates/wit-parser/src/resolve.rs | 510 +++++++++++++++++- 64 files changed, 1428 insertions(+), 96 deletions(-) create mode 100644 crates/wit-component/tests/components/import-in-adapter-and-main-module/adapt-old.wat create mode 100644 crates/wit-component/tests/components/import-in-adapter-and-main-module/adapt-old.wit create mode 100644 crates/wit-component/tests/components/import-in-adapter-and-main-module/component.wat create mode 100644 crates/wit-component/tests/components/import-in-adapter-and-main-module/component.wit create mode 100644 crates/wit-component/tests/components/import-in-adapter-and-main-module/deps/shared-dependency/doc.wit create mode 100644 crates/wit-component/tests/components/import-in-adapter-and-main-module/deps/shared-dependency/types.wit create mode 100644 crates/wit-component/tests/components/import-in-adapter-and-main-module/module.wat create mode 100644 crates/wit-component/tests/components/import-in-adapter-and-main-module/module.wit create mode 100644 crates/wit-component/tests/merge.rs create mode 100644 crates/wit-component/tests/merge/bad-interface1/error.txt create mode 100644 crates/wit-component/tests/merge/bad-interface1/from/a.wit create mode 100644 crates/wit-component/tests/merge/bad-interface1/from/deps/foo/a.wit create mode 100644 crates/wit-component/tests/merge/bad-interface1/into/a.wit create mode 100644 crates/wit-component/tests/merge/bad-interface1/into/deps/foo/a.wit create mode 100644 crates/wit-component/tests/merge/bad-interface2/error.txt create mode 100644 crates/wit-component/tests/merge/bad-interface2/from/a.wit create mode 100644 crates/wit-component/tests/merge/bad-interface2/from/deps/foo/a.wit create mode 100644 crates/wit-component/tests/merge/bad-interface2/into/a.wit create mode 100644 crates/wit-component/tests/merge/bad-interface2/into/deps/foo/a.wit create mode 100644 crates/wit-component/tests/merge/bad-world1/error.txt create mode 100644 crates/wit-component/tests/merge/bad-world1/from/a.wit create mode 100644 crates/wit-component/tests/merge/bad-world1/from/deps/foo/a.wit create mode 100644 crates/wit-component/tests/merge/bad-world1/into/a.wit create mode 100644 crates/wit-component/tests/merge/bad-world1/into/deps/foo/a.wit create mode 100644 crates/wit-component/tests/merge/bad-world2/error.txt create mode 100644 crates/wit-component/tests/merge/bad-world2/from/a.wit create mode 100644 crates/wit-component/tests/merge/bad-world2/from/deps/foo/a.wit create mode 100644 crates/wit-component/tests/merge/bad-world2/into/a.wit create mode 100644 crates/wit-component/tests/merge/bad-world2/into/deps/foo/a.wit create mode 100644 crates/wit-component/tests/merge/bad-world3/error.txt create mode 100644 crates/wit-component/tests/merge/bad-world3/from/a.wit create mode 100644 crates/wit-component/tests/merge/bad-world3/from/deps/foo/a.wit create mode 100644 crates/wit-component/tests/merge/bad-world3/into/a.wit create mode 100644 crates/wit-component/tests/merge/bad-world3/into/deps/foo/a.wit create mode 100644 crates/wit-component/tests/merge/bad-world4/error.txt create mode 100644 crates/wit-component/tests/merge/bad-world4/from/a.wit create mode 100644 crates/wit-component/tests/merge/bad-world4/from/deps/foo/a.wit create mode 100644 crates/wit-component/tests/merge/bad-world4/into/a.wit create mode 100644 crates/wit-component/tests/merge/bad-world4/into/deps/foo/a.wit create mode 100644 crates/wit-component/tests/merge/bad-world5/error.txt create mode 100644 crates/wit-component/tests/merge/bad-world5/from/a.wit create mode 100644 crates/wit-component/tests/merge/bad-world5/from/deps/foo/a.wit create mode 100644 crates/wit-component/tests/merge/bad-world5/into/a.wit create mode 100644 crates/wit-component/tests/merge/bad-world5/into/deps/foo/a.wit create mode 100644 crates/wit-component/tests/merge/success/from/a.wit create mode 100644 crates/wit-component/tests/merge/success/from/deps/foo/only-from.wit create mode 100644 crates/wit-component/tests/merge/success/from/deps/foo/shared.wit create mode 100644 crates/wit-component/tests/merge/success/from/deps/only-from-dep/a.wit create mode 100644 crates/wit-component/tests/merge/success/into/b.wit create mode 100644 crates/wit-component/tests/merge/success/into/deps/foo/only-into.wit create mode 100644 crates/wit-component/tests/merge/success/into/deps/foo/shared.wit create mode 100644 crates/wit-component/tests/merge/success/merge/foo/only-from.wit create mode 100644 crates/wit-component/tests/merge/success/merge/foo/only-into.wit create mode 100644 crates/wit-component/tests/merge/success/merge/foo/shared.wit create mode 100644 crates/wit-component/tests/merge/success/merge/from/a.wit create mode 100644 crates/wit-component/tests/merge/success/merge/into/b.wit create mode 100644 crates/wit-component/tests/merge/success/merge/only-from-dep/a.wit diff --git a/crates/wit-component/src/encoding.rs b/crates/wit-component/src/encoding.rs index f0992100fd..9271015730 100644 --- a/crates/wit-component/src/encoding.rs +++ b/crates/wit-component/src/encoding.rs @@ -76,14 +76,14 @@ use crate::metadata::{self, Bindgen, ModuleMetadata}; use crate::validation::{ValidatedModule, BARE_FUNC_MODULE_NAME, MAIN_MODULE_IMPORT_NAME}; use crate::StringEncoding; use anyhow::{anyhow, bail, Context, Result}; -use indexmap::IndexMap; +use indexmap::{IndexMap, IndexSet}; use std::collections::HashMap; use std::hash::Hash; use wasm_encoder::*; use wasmparser::{Validator, WasmFeatures}; use wit_parser::{ abi::{AbiVariant, WasmSignature, WasmType}, - Function, InterfaceId, Resolve, Type, TypeDefKind, TypeId, TypeOwner, WorldId, WorldItem, + Function, InterfaceId, Resolve, Type, TypeDefKind, TypeId, TypeOwner, WorldItem, }; const INDIRECT_TABLE_NAME: &str = "$imports"; @@ -457,7 +457,10 @@ impl<'a> EncodingState<'a> { // will forward them through. if let Some(live) = encoder.state.info.live_types.get(&interface_id) { for ty in live { - log::trace!("encoding extra type {ty:?}"); + log::trace!( + "encoding extra type {ty:?} name={:?}", + resolve.types[*ty].name + ); encoder.encode_valtype(resolve, &Type::Id(*ty))?; } } @@ -593,11 +596,20 @@ impl<'a> EncodingState<'a> { Some(core_wasm_name) }; let import = &self.info.import_map[&interface]; + let required_imports = match for_module { + CustomModule::Main => &self.info.info.required_imports[core_wasm_name], + CustomModule::Adapter(name) => { + &self.info.adapters[name].0.required_imports[core_wasm_name] + } + }; let mut exports = Vec::with_capacity(import.direct.len() + import.indirect.len()); // Add an entry for all indirect lowerings which come as an export of // the shim module. for (i, lowering) in import.indirect.iter().enumerate() { + if !required_imports.contains(&lowering.name) { + continue; + } let encoding = metadata.import_encodings[&(core_wasm_name.to_string(), lowering.name.to_string())]; let index = self.component.alias_core_item( @@ -617,6 +629,9 @@ impl<'a> EncodingState<'a> { // All direct lowerings can be `canon lower`'d here immediately and // passed as arguments. for lowering in &import.direct { + if !required_imports.contains(&lowering.name) { + continue; + } let func_index = match &import.interface { Some((interface, _url)) => { let instance_index = self.imported_instances[interface]; @@ -633,13 +648,13 @@ impl<'a> EncodingState<'a> { fn encode_exports(&mut self, module: CustomModule) -> Result<()> { let resolve = &self.info.encoder.metadata.resolve; - let world = match module { - CustomModule::Main => self.info.encoder.metadata.world, - CustomModule::Adapter(name) => self.info.encoder.adapters[name].2, + let exports = match module { + CustomModule::Main => &self.info.encoder.main_module_exports, + CustomModule::Adapter(name) => &self.info.encoder.adapters[name].2, }; - let world = &resolve.worlds[world]; - for (export_name, export) in world.exports.iter() { - match export { + let world = &resolve.worlds[self.info.encoder.metadata.world]; + for export_name in exports { + match &world.exports[export_name] { WorldItem::Function(func) => { let mut enc = self.root_type_encoder(None); let ty = enc.encode_func_type(resolve, func)?; @@ -857,7 +872,7 @@ impl<'a> EncodingState<'a> { // For all interfaces imported into the main module record all of their // indirect lowerings into `Shims`. - for core_wasm_name in info.required_imports.keys() { + for (core_wasm_name, required) in info.required_imports.iter() { let import_name = if *core_wasm_name == BARE_FUNC_MODULE_NAME { None } else { @@ -868,6 +883,7 @@ impl<'a> EncodingState<'a> { core_wasm_name, CustomModule::Main, import, + required, info.metadata, &mut signatures, ); @@ -877,12 +893,13 @@ impl<'a> EncodingState<'a> { // function and additionally a set of shims are created for the // interface imported into the shim module itself. for (adapter, (info, _wasm)) in self.info.adapters.iter() { - for (name, _) in info.required_imports.iter() { + for (name, required) in info.required_imports.iter() { let import = &self.info.import_map[&Some(*name)]; ret.append_indirect( name, CustomModule::Adapter(adapter), import, + required, info.metadata, &mut signatures, ); @@ -916,7 +933,8 @@ impl<'a> EncodingState<'a> { } for shim in ret.list.iter() { - ret.shim_names.insert(shim.kind, shim.name.clone()); + let prev = ret.shim_names.insert(shim.kind, shim.name.clone()); + assert!(prev.is_none()); } assert!(self.shim_instance_index.is_none()); @@ -1304,6 +1322,7 @@ impl<'a> Shims<'a> { core_wasm_module: &'a str, for_module: CustomModule<'a>, import: &ImportedInterface<'a>, + required: &IndexSet<&str>, metadata: &ModuleMetadata, sigs: &mut Vec, ) { @@ -1313,6 +1332,9 @@ impl<'a> Shims<'a> { Some(core_wasm_module) }; for (indirect_index, lowering) in import.indirect.iter().enumerate() { + if !required.contains(&lowering.name) { + continue; + } let shim_name = self.list.len().to_string(); log::debug!( "shim {shim_name} is import `{core_wasm_module}` lowering {indirect_index} `{}`", @@ -1342,6 +1364,7 @@ pub struct ComponentEncoder { module: Vec, metadata: Bindgen, validate: bool, + main_module_exports: IndexSet, // This is a map from the name of the adapter to a pair of: // @@ -1349,8 +1372,9 @@ pub struct ComponentEncoder { // stripped. // * the metadata for the adapter, verified to have no exports and only // imports. - // * The world within `self.metadata.doc` which the adapter works with. - adapters: IndexMap, ModuleMetadata, WorldId)>, + // * The set of exports from the final world which are defined by this + // adapter. + adapters: IndexMap, ModuleMetadata, IndexSet)>, } impl ComponentEncoder { @@ -1361,7 +1385,15 @@ impl ComponentEncoder { /// core module. pub fn module(mut self, module: &[u8]) -> Result { let (wasm, metadata) = metadata::decode(module)?; - self.metadata.merge(metadata)?; + self.main_module_exports.extend( + metadata.resolve.worlds[metadata.world] + .exports + .keys() + .cloned(), + ); + self.metadata + .merge(metadata) + .context("failed merge WIT package sets together")?; self.module = if let Some(producers) = &self.metadata.producers { producers.add_to_wasm(&wasm)? } else { @@ -1398,9 +1430,27 @@ impl ComponentEncoder { // Merge the adapter's document into our own document to have one large // document, but the adapter's world isn't merged in to our world so // retain it separately. - let world = self.metadata.resolve.merge(metadata.resolve).worlds[metadata.world.index()]; + let world = self + .metadata + .resolve + .merge(metadata.resolve) + .with_context(|| { + format!("failed to merge WIT packages of adapter `{name}` into main packages") + })? + .worlds[metadata.world.index()]; + self.metadata + .resolve + .merge_worlds(world, self.metadata.world) + .with_context(|| { + format!("failed to merge WIT world of adapter `{name}` into main package") + })?; + let exports = self.metadata.resolve.worlds[world] + .exports + .keys() + .cloned() + .collect(); self.adapters - .insert(name.to_string(), (wasm, metadata.metadata, world)); + .insert(name.to_string(), (wasm, metadata.metadata, exports)); Ok(self) } diff --git a/crates/wit-component/src/encoding/types.rs b/crates/wit-component/src/encoding/types.rs index 5a08e8c8bc..7fadcc66af 100644 --- a/crates/wit-component/src/encoding/types.rs +++ b/crates/wit-component/src/encoding/types.rs @@ -127,6 +127,7 @@ pub trait ValtypeEncoder<'a> { // If this type is imported from another interface then return // it as it was bound here with an alias. let ty = &resolve.types[id]; + log::trace!("encode type name={:?}", ty.name); if let Some(index) = self.maybe_import_type(resolve, id) { self.type_map().insert(id, index); return Ok(ComponentValType::Type(index)); diff --git a/crates/wit-component/src/encoding/world.rs b/crates/wit-component/src/encoding/world.rs index 7771482f6e..8b89b2ac0c 100644 --- a/crates/wit-component/src/encoding/world.rs +++ b/crates/wit-component/src/encoding/world.rs @@ -62,7 +62,12 @@ impl<'a> ComponentWorld<'a> { .keys() .map(|s| s.as_str()) .collect::>(); - let info = validate_module(&encoder.module, &encoder.metadata, &adapters)?; + let info = validate_module( + &encoder.module, + &encoder.metadata, + &encoder.main_module_exports, + &adapters, + )?; let mut ret = ComponentWorld { encoder, @@ -85,15 +90,17 @@ impl<'a> ComponentWorld<'a> { /// main module or they're part of the adapter's exports. fn process_adapters(&mut self) -> Result<()> { let resolve = &self.encoder.metadata.resolve; - for (name, (wasm, metadata, world)) in self.encoder.adapters.iter() { + let world = self.encoder.metadata.world; + for (name, (wasm, metadata, required_exports)) in self.encoder.adapters.iter() { let required_by_import = self.info.adapters_required.get(name.as_str()); - let required = self.required_adapter_exports(resolve, *world, required_by_import); + let required = + self.required_adapter_exports(resolve, world, required_exports, required_by_import); if required.is_empty() { continue; } let wasm = crate::gc::run(wasm, &required, self.info.realloc) .context("failed to reduce input adapter module to its minimal size")?; - let info = validate_adapter_module(&wasm, resolve, *world, metadata, &required) + let info = validate_adapter_module(&wasm, resolve, world, metadata, &required) .context("failed to validate the imports of the minimized adapter module")?; self.adapters.insert(name, (info, wasm)); } @@ -107,6 +114,7 @@ impl<'a> ComponentWorld<'a> { &self, resolve: &Resolve, world: WorldId, + required_exports: &IndexSet, required_by_import: Option<&IndexMap<&str, FuncType>>, ) -> IndexMap { use wasmparser::ValType; @@ -129,8 +137,8 @@ impl<'a> ComponentWorld<'a> { ); assert!(prev.is_none()); }; - for (name, item) in resolve.worlds[world].exports.iter() { - match item { + for name in required_exports { + match &resolve.worlds[world].exports[name] { WorldItem::Function(func) => add_func(func, None), WorldItem::Interface(id) => { for (_, func) in resolve.interfaces[*id].functions.iter() { @@ -158,27 +166,29 @@ impl<'a> ComponentWorld<'a> { fn process_imports(&mut self) -> Result<()> { let resolve = &self.encoder.metadata.resolve; let world = self.encoder.metadata.world; + let mut all_required_imports = IndexMap::new(); + for map in self + .adapters + .values() + .map(|(i, _)| &i.required_imports) + .chain([&self.info.required_imports]) + { + for (k, v) in map { + all_required_imports + .entry(*k) + .or_insert_with(IndexSet::new) + .extend(v); + } + } for (name, item) in resolve.worlds[world].imports.iter() { add_item( &mut self.import_map, resolve, name, item, - &self.info.required_imports, + &all_required_imports, )?; } - for (adapter_name, (info, _wasm)) in self.adapters.iter() { - let (_, _, world) = self.encoder.adapters[*adapter_name]; - for (name, item) in resolve.worlds[world].imports.iter() { - add_item( - &mut self.import_map, - resolve, - name, - item, - &info.required_imports, - )?; - } - } return Ok(()); fn add_item<'a>( @@ -188,6 +198,7 @@ impl<'a> ComponentWorld<'a> { item: &'a WorldItem, required: &IndexMap<&str, IndexSet<&str>>, ) -> Result<()> { + log::trace!("register import `{name}`"); let empty = IndexSet::new(); match item { WorldItem::Function(func) => { @@ -202,6 +213,7 @@ impl<'a> ComponentWorld<'a> { indirect: Default::default(), required: Default::default(), }); + assert!(interface.interface.is_none()); add_import(interface, resolve, func) } WorldItem::Interface(id) => { @@ -216,6 +228,7 @@ impl<'a> ComponentWorld<'a> { indirect: Default::default(), required: Default::default(), }); + assert_eq!(interface.interface.as_ref().unwrap().0, *id); for (_name, func) in resolve.interfaces[*id].functions.iter() { // If this function isn't actually required then skip it if required.contains(func.name.as_str()) { @@ -236,6 +249,7 @@ impl<'a> ComponentWorld<'a> { if !interface.required.insert(func.name.as_str()) { return Ok(()); } + log::trace!("add func {}", func.name); let options = RequiredOptions::for_import(resolve, func); if options.is_empty() { interface.direct.push(DirectLowering { name: &func.name }); @@ -258,21 +272,15 @@ impl<'a> ComponentWorld<'a> { fn process_live_types(&mut self) { let mut live = LiveTypes::default(); let resolve = &self.encoder.metadata.resolve; - let world = &resolve.worlds[self.encoder.metadata.world]; - self.add_live_imports( - self.encoder.metadata.world, - &self.info.required_imports, - &mut live, - ); - for (_, item) in world.exports.iter() { - live.add_world_item(resolve, item); - } + let world = self.encoder.metadata.world; + self.add_live_imports(world, &self.info.required_imports, &mut live); for (adapter_name, (info, _wasm)) in self.adapters.iter() { - let (_, _, world) = self.encoder.adapters[*adapter_name]; + log::trace!("processing adapter `{adapter_name}`"); self.add_live_imports(world, &info.required_imports, &mut live); - for (_, item) in resolve.worlds[world].exports.iter() { - live.add_world_item(resolve, item); - } + } + for (name, item) in resolve.worlds[world].exports.iter() { + log::trace!("add live world export `{name}`"); + live.add_world_item(resolve, item); } for live in live.iter() { @@ -304,6 +312,7 @@ impl<'a> ComponentWorld<'a> { if !required.contains(name.as_str()) { continue; } + log::trace!("add live function import `{name}`"); live.add_func(resolve, func); } WorldItem::Interface(id) => { @@ -311,8 +320,10 @@ impl<'a> ComponentWorld<'a> { Some(set) => set, None => continue, }; + log::trace!("add live interface import `{name}`"); for (name, func) in resolve.interfaces[*id].functions.iter() { if required.contains(name.as_str()) { + log::trace!("add live func `{name}`"); live.add_func(resolve, func); } } diff --git a/crates/wit-component/src/metadata.rs b/crates/wit-component/src/metadata.rs index 005e518f34..f2ae09285b 100644 --- a/crates/wit-component/src/metadata.rs +++ b/crates/wit-component/src/metadata.rs @@ -240,7 +240,11 @@ impl Bindgen { producers, } = other; - let world = self.resolve.merge(resolve).worlds[world.index()]; + let world = self + .resolve + .merge(resolve) + .context("failed to merge WIT package sets together")? + .worlds[world.index()]; self.resolve .merge_worlds(world, self.world) .context("failed to merge worlds from two documents")?; diff --git a/crates/wit-component/src/validation.rs b/crates/wit-component/src/validation.rs index 89b04f7dd1..1ae6a869de 100644 --- a/crates/wit-component/src/validation.rs +++ b/crates/wit-component/src/validation.rs @@ -89,6 +89,7 @@ pub struct ValidatedModule<'a> { pub fn validate_module<'a>( bytes: &'a [u8], metadata: &'a Bindgen, + exports: &IndexSet, adapters: &IndexSet<&str>, ) -> Result> { let mut validator = Validator::new(); @@ -197,8 +198,14 @@ pub fn validate_module<'a>( } } - for (name, item) in world.exports.iter() { - validate_exported_item(&metadata.resolve, item, name, &export_funcs, &types)?; + for name in exports { + validate_exported_item( + &metadata.resolve, + &world.exports[name], + name, + &export_funcs, + &types, + )?; } Ok(ret) diff --git a/crates/wit-component/tests/components.rs b/crates/wit-component/tests/components.rs index c27988c28b..a1189b8f63 100644 --- a/crates/wit-component/tests/components.rs +++ b/crates/wit-component/tests/components.rs @@ -1,9 +1,9 @@ -use anyhow::Result; +use anyhow::{anyhow, Result}; use pretty_assertions::assert_eq; use std::{fs, path::Path}; use wasm_encoder::{Encode, Section}; use wit_component::{ComponentEncoder, DecodedWasm, DocumentPrinter, StringEncoding}; -use wit_parser::{Resolve, UnresolvedPackage}; +use wit_parser::{PackageId, Resolve}; /// Tests the encoding of components. /// @@ -48,10 +48,13 @@ fn component_encoding_via_flags() -> Result<()> { let test_case = path.file_stem().unwrap().to_str().unwrap(); println!("testing {test_case}"); + let mut resolve = Resolve::default(); + let (pkg, _) = resolve.push_dir(&path)?; + let module_path = path.join("module.wat"); - let module = read_core_module(&module_path)?; + let module = read_core_module(&module_path, &resolve, pkg)?; let mut encoder = ComponentEncoder::default().module(&module)?.validate(true); - encoder = add_adapters(encoder, &path)?; + encoder = add_adapters(encoder, &path, &resolve, pkg)?; let component_path = path.join("component.wat"); let component_wit_path = path.join("component.wit"); let error_path = path.join("error.txt"); @@ -105,10 +108,15 @@ fn component_encoding_via_flags() -> Result<()> { Ok(()) } -fn add_adapters(mut encoder: ComponentEncoder, path: &Path) -> Result { +fn add_adapters( + mut encoder: ComponentEncoder, + path: &Path, + resolve: &Resolve, + pkg: PackageId, +) -> Result { for adapter in glob::glob(path.join("adapt-*.wat").to_str().unwrap())? { let adapter = adapter?; - let wasm = read_core_module(&adapter)?; + let wasm = read_core_module(&adapter, resolve, pkg)?; let stem = adapter.file_stem().unwrap().to_str().unwrap(); let name = stem.trim_start_matches("adapt-"); encoder = encoder.adapter(&name, &wasm)?; @@ -118,17 +126,19 @@ fn add_adapters(mut encoder: ComponentEncoder, path: &Path) -> Result Result> { +/// The `resolve` and `pkg` are the parsed WIT package from this test's +/// directory and the `path`'s filename is used to find a WIT document of the +/// corresponding name which should have a world that `path` ascribes to. +fn read_core_module(path: &Path, resolve: &Resolve, pkg: PackageId) -> Result> { let mut wasm = wat::parse_file(path)?; - let interface = path.with_extension("wit"); - let mut resolve = Resolve::default(); - let pkg = resolve.push( - UnresolvedPackage::parse_file(&interface)?, - &Default::default(), - )?; - let world = resolve.select_world(pkg, None)?; + let name = path.file_stem().and_then(|s| s.to_str()).unwrap(); + let doc = *resolve.packages[pkg] + .documents + .get(name) + .ok_or_else(|| anyhow!("failed to find document named `{name}`"))?; + let world = resolve.documents[doc] + .default_world + .ok_or_else(|| anyhow!("failed to find default world in document `{name}`"))?; // Add this producer data to the wit-component metadata so we can make sure it gets through the // translation: diff --git a/crates/wit-component/tests/components/import-in-adapter-and-main-module/adapt-old.wat b/crates/wit-component/tests/components/import-in-adapter-and-main-module/adapt-old.wat new file mode 100644 index 0000000000..65a6863807 --- /dev/null +++ b/crates/wit-component/tests/components/import-in-adapter-and-main-module/adapt-old.wat @@ -0,0 +1,23 @@ +(module + (import "shared-dependency" "f1" (func $f1)) + (import "shared-dependency" "f3" (func $f3)) + + (import "shared-dependency" "g1" (func $g1 (param i32))) + (import "shared-dependency" "g3" (func $g3 (param i32))) + + (import "shared-dependency" "unused-in-adapter" (func)) + (import "env" "memory" (memory 0)) + + (import "adapter-dep" "foo" (func $foo (result i32))) + + (func (export "adapter-f1") + call $f1 + call $f3 + (call $g1 (i32.const 0)) + (call $g3 (i32.const 0)) + (drop (call $foo)) + ) + + (func (export "cabi_import_realloc") (param i32 i32 i32 i32) (result i32) + unreachable) +) diff --git a/crates/wit-component/tests/components/import-in-adapter-and-main-module/adapt-old.wit b/crates/wit-component/tests/components/import-in-adapter-and-main-module/adapt-old.wit new file mode 100644 index 0000000000..077605bf5e --- /dev/null +++ b/crates/wit-component/tests/components/import-in-adapter-and-main-module/adapt-old.wit @@ -0,0 +1,9 @@ +default world the-adapter { + import shared-dependency: shared-dependency.doc + + import adapter-dep: interface { + use shared-dependency.types.{a-typedef} + + foo: func() -> a-typedef + } +} diff --git a/crates/wit-component/tests/components/import-in-adapter-and-main-module/component.wat b/crates/wit-component/tests/components/import-in-adapter-and-main-module/component.wat new file mode 100644 index 0000000000..552b97b702 --- /dev/null +++ b/crates/wit-component/tests/components/import-in-adapter-and-main-module/component.wat @@ -0,0 +1,207 @@ +(component + (type (;0;) + (instance + (type (;0;) (func)) + (export (;0;) "f1" (func (type 0))) + (export (;1;) "f2" (func (type 0))) + (export (;2;) "f3" (func (type 0))) + (type (;1;) (func (result string))) + (export (;3;) "g1" (func (type 1))) + (export (;4;) "g2" (func (type 1))) + (export (;5;) "g3" (func (type 1))) + ) + ) + (import "shared-dependency" "path:/shared-dependency/doc/doc" (instance (;0;) (type 0))) + (type (;1;) + (instance + (type (;0;) u32) + (export (;1;) "a-typedef" (type (eq 0))) + ) + ) + (import "types" "path:/shared-dependency/types/types" (instance (;1;) (type 1))) + (alias export 1 "a-typedef" (type (;2;))) + (type (;3;) + (instance + (alias outer 1 2 (type (;0;))) + (export (;1;) "a-typedef" (type (eq 0))) + (type (;2;) (func (result 1))) + (export (;0;) "foo" (func (type 2))) + ) + ) + (import "main-dep" (instance (;2;) (type 3))) + (alias export 1 "a-typedef" (type (;4;))) + (type (;5;) + (instance + (alias outer 1 4 (type (;0;))) + (export (;1;) "a-typedef" (type (eq 0))) + (type (;2;) (func (result 1))) + (export (;0;) "foo" (func (type 2))) + ) + ) + (import "adapter-dep" (instance (;3;) (type 5))) + (core module (;0;) + (type (;0;) (func)) + (type (;1;) (func (param i32))) + (type (;2;) (func (result i32))) + (type (;3;) (func (param i32 i32 i32 i32) (result i32))) + (import "shared-dependency" "f1" (func (;0;) (type 0))) + (import "shared-dependency" "f2" (func (;1;) (type 0))) + (import "shared-dependency" "g1" (func (;2;) (type 1))) + (import "shared-dependency" "g2" (func (;3;) (type 1))) + (import "old" "adapter-f1" (func (;4;) (type 0))) + (import "main-dep" "foo" (func (;5;) (type 2))) + (func (;6;) (type 3) (param i32 i32 i32 i32) (result i32) + unreachable + ) + (memory (;0;) 1) + (export "memory" (memory 0)) + (export "cabi_realloc" (func 6)) + ) + (core module (;1;) + (type (;0;) (func)) + (type (;1;) (func (param i32))) + (type (;2;) (func (result i32))) + (type (;3;) (func (param i32 i32 i32 i32) (result i32))) + (import "shared-dependency" "f1" (func $f1 (;0;) (type 0))) + (import "shared-dependency" "f3" (func $f3 (;1;) (type 0))) + (import "shared-dependency" "g1" (func $g1 (;2;) (type 1))) + (import "shared-dependency" "g3" (func $g3 (;3;) (type 1))) + (import "adapter-dep" "foo" (func $foo (;4;) (type 2))) + (func (;5;) (type 0) + call $f1 + call $f3 + i32.const 0 + call $g1 + i32.const 0 + call $g3 + call $foo + drop + ) + (func (;6;) (type 3) (param i32 i32 i32 i32) (result i32) + unreachable + ) + (export "adapter-f1" (func 5)) + (export "cabi_import_realloc" (func 6)) + ) + (core module (;2;) + (type (;0;) (func (param i32))) + (type (;1;) (func)) + (func $indirect-shared-dependency-g1 (;0;) (type 0) (param i32) + local.get 0 + i32.const 0 + call_indirect (type 0) + ) + (func $indirect-shared-dependency-g2 (;1;) (type 0) (param i32) + local.get 0 + i32.const 1 + call_indirect (type 0) + ) + (func $#func2 (@name "indirect-shared-dependency-g1") (;2;) (type 0) (param i32) + local.get 0 + i32.const 2 + call_indirect (type 0) + ) + (func $indirect-shared-dependency-g3 (;3;) (type 0) (param i32) + local.get 0 + i32.const 3 + call_indirect (type 0) + ) + (func $adapt-old-adapter-f1 (;4;) (type 1) + i32.const 4 + call_indirect (type 1) + ) + (table (;0;) 5 5 funcref) + (export "0" (func $indirect-shared-dependency-g1)) + (export "1" (func $indirect-shared-dependency-g2)) + (export "2" (func $#func2)) + (export "3" (func $indirect-shared-dependency-g3)) + (export "4" (func $adapt-old-adapter-f1)) + (export "$imports" (table 0)) + ) + (core module (;3;) + (type (;0;) (func (param i32))) + (type (;1;) (func)) + (import "" "0" (func (;0;) (type 0))) + (import "" "1" (func (;1;) (type 0))) + (import "" "2" (func (;2;) (type 0))) + (import "" "3" (func (;3;) (type 0))) + (import "" "4" (func (;4;) (type 1))) + (import "" "$imports" (table (;0;) 5 5 funcref)) + (elem (;0;) (i32.const 0) func 0 1 2 3 4) + ) + (core instance (;0;) (instantiate 2)) + (alias core export 0 "0" (core func (;0;))) + (alias core export 0 "1" (core func (;1;))) + (alias export 0 "f1" (func (;0;))) + (core func (;2;) (canon lower (func 0))) + (alias export 0 "f2" (func (;1;))) + (core func (;3;) (canon lower (func 1))) + (core instance (;1;) + (export "g1" (func 0)) + (export "g2" (func 1)) + (export "f1" (func 2)) + (export "f2" (func 3)) + ) + (alias export 2 "foo" (func (;2;))) + (core func (;4;) (canon lower (func 2))) + (core instance (;2;) + (export "foo" (func 4)) + ) + (alias core export 0 "4" (core func (;5;))) + (core instance (;3;) + (export "adapter-f1" (func 5)) + ) + (core instance (;4;) (instantiate 0 + (with "shared-dependency" (instance 1)) + (with "main-dep" (instance 2)) + (with "old" (instance 3)) + ) + ) + (alias core export 4 "memory" (core memory (;0;))) + (alias core export 4 "cabi_realloc" (core func (;6;))) + (alias core export 0 "2" (core func (;7;))) + (alias core export 0 "3" (core func (;8;))) + (alias export 0 "f1" (func (;3;))) + (core func (;9;) (canon lower (func 3))) + (alias export 0 "f3" (func (;4;))) + (core func (;10;) (canon lower (func 4))) + (core instance (;5;) + (export "g1" (func 7)) + (export "g3" (func 8)) + (export "f1" (func 9)) + (export "f3" (func 10)) + ) + (alias export 3 "foo" (func (;5;))) + (core func (;11;) (canon lower (func 5))) + (core instance (;6;) + (export "foo" (func 11)) + ) + (core instance (;7;) (instantiate 1 + (with "shared-dependency" (instance 5)) + (with "adapter-dep" (instance 6)) + ) + ) + (alias core export 7 "cabi_import_realloc" (core func (;12;))) + (alias core export 0 "$imports" (core table (;0;))) + (alias export 0 "g1" (func (;6;))) + (core func (;13;) (canon lower (func 6) (memory 0) (realloc 6) string-encoding=utf8)) + (alias export 0 "g2" (func (;7;))) + (core func (;14;) (canon lower (func 7) (memory 0) (realloc 6) string-encoding=utf8)) + (alias export 0 "g1" (func (;8;))) + (core func (;15;) (canon lower (func 8) (memory 0) (realloc 12) string-encoding=utf8)) + (alias export 0 "g3" (func (;9;))) + (core func (;16;) (canon lower (func 9) (memory 0) (realloc 12) string-encoding=utf8)) + (alias core export 7 "adapter-f1" (core func (;17;))) + (core instance (;8;) + (export "$imports" (table 0)) + (export "0" (func 13)) + (export "1" (func 14)) + (export "2" (func 15)) + (export "3" (func 16)) + (export "4" (func 17)) + ) + (core instance (;9;) (instantiate 3 + (with "" (instance 8)) + ) + ) +) \ No newline at end of file diff --git a/crates/wit-component/tests/components/import-in-adapter-and-main-module/component.wit b/crates/wit-component/tests/components/import-in-adapter-and-main-module/component.wit new file mode 100644 index 0000000000..82dbbcdfde --- /dev/null +++ b/crates/wit-component/tests/components/import-in-adapter-and-main-module/component.wit @@ -0,0 +1,35 @@ +interface shared-dependency { + f1: func() + + f2: func() + + f3: func() + + g1: func() -> string + + g2: func() -> string + + g3: func() -> string +} + +interface types { + type a-typedef = u32 + +} + +interface main-dep { + use self.types.{a-typedef} + foo: func() -> a-typedef +} + +interface adapter-dep { + use self.types.{a-typedef} + foo: func() -> a-typedef +} + +default world component { + import shared-dependency: self.shared-dependency + import types: self.types + import main-dep: self.main-dep + import adapter-dep: self.adapter-dep +} diff --git a/crates/wit-component/tests/components/import-in-adapter-and-main-module/deps/shared-dependency/doc.wit b/crates/wit-component/tests/components/import-in-adapter-and-main-module/deps/shared-dependency/doc.wit new file mode 100644 index 0000000000..b02b572f0e --- /dev/null +++ b/crates/wit-component/tests/components/import-in-adapter-and-main-module/deps/shared-dependency/doc.wit @@ -0,0 +1,11 @@ +default interface doc { + f1: func() + f2: func() + f3: func() + + g1: func() -> string + g2: func() -> string + g3: func() -> string + + unused-in-adapter: func() +} diff --git a/crates/wit-component/tests/components/import-in-adapter-and-main-module/deps/shared-dependency/types.wit b/crates/wit-component/tests/components/import-in-adapter-and-main-module/deps/shared-dependency/types.wit new file mode 100644 index 0000000000..04673c8ce1 --- /dev/null +++ b/crates/wit-component/tests/components/import-in-adapter-and-main-module/deps/shared-dependency/types.wit @@ -0,0 +1,3 @@ +default interface types { + type a-typedef = u32 +} diff --git a/crates/wit-component/tests/components/import-in-adapter-and-main-module/module.wat b/crates/wit-component/tests/components/import-in-adapter-and-main-module/module.wat new file mode 100644 index 0000000000..1a0abd1df0 --- /dev/null +++ b/crates/wit-component/tests/components/import-in-adapter-and-main-module/module.wat @@ -0,0 +1,15 @@ +(module + (import "shared-dependency" "f1" (func)) + (import "shared-dependency" "f2" (func)) + + (import "shared-dependency" "g1" (func (param i32))) + (import "shared-dependency" "g2" (func (param i32))) + + (import "old" "adapter-f1" (func)) + + (import "main-dep" "foo" (func (result i32))) + + (memory (export "memory") 1) + (func (export "cabi_realloc") (param i32 i32 i32 i32) (result i32) + unreachable) +) diff --git a/crates/wit-component/tests/components/import-in-adapter-and-main-module/module.wit b/crates/wit-component/tests/components/import-in-adapter-and-main-module/module.wit new file mode 100644 index 0000000000..6a28b56da6 --- /dev/null +++ b/crates/wit-component/tests/components/import-in-adapter-and-main-module/module.wit @@ -0,0 +1,9 @@ +default world the-module { + import shared-dependency: shared-dependency.doc + + import main-dep: interface { + use shared-dependency.types.{a-typedef} + + foo: func() -> a-typedef + } +} diff --git a/crates/wit-component/tests/merge.rs b/crates/wit-component/tests/merge.rs new file mode 100644 index 0000000000..94b1b46c91 --- /dev/null +++ b/crates/wit-component/tests/merge.rs @@ -0,0 +1,181 @@ +use anyhow::{Context, Result}; +use pretty_assertions::assert_eq; +use std::collections::HashSet; +use std::{fs, path::Path}; +use wit_component::DocumentPrinter; +use wit_parser::{Resolve, TypeOwner, WorldItem}; + +/// This is a test which iterates over the `tests/merge` directory and treats +/// each subdirectory as its own test. Each subdirectory has an `into` and a +/// `from` folder which represent two different `Resolve` sets, each with their +/// own set of dependencies as well. +/// +/// Each test will merge the `from` into the `into` and assert that everything +/// is valid along the way. On successful merge the resulting documents +/// are printed and asserted against expectations. Failures assert the +/// correct error message. +#[test] +fn merging() -> Result<()> { + drop(env_logger::try_init()); + + for entry in fs::read_dir("tests/merge")? { + let path = entry?.path(); + if !path.is_dir() { + continue; + } + + let test_case = path.file_stem().unwrap().to_str().unwrap(); + println!("testing {test_case}"); + + let mut from = Resolve::default(); + from.push_dir(&path.join("from"))?; + let mut into = Resolve::default(); + into.push_dir(&path.join("into"))?; + + assert_valid_resolve(&from); + assert_valid_resolve(&into); + + match into.merge(from) { + Ok(_) => { + assert!( + !test_case.starts_with("bad-"), + "should have failed to merge" + ); + assert_valid_resolve(&into); + for (_id, pkg) in into.packages.iter() { + for (name, doc) in pkg.documents.iter() { + let expected = path + .join("merge") + .join(&pkg.name) + .join(name) + .with_extension("wit"); + let output = DocumentPrinter::default().print(&into, *doc)?; + assert_output(&expected, &output)?; + } + } + } + Err(e) => { + assert!(test_case.starts_with("bad-"), "failed to merge with {e:?}"); + assert_output(&path.join("error.txt"), &format!("{e:?}"))?; + } + } + } + + Ok(()) +} + +fn assert_output(expected: &Path, actual: &str) -> Result<()> { + if std::env::var_os("BLESS").is_some() { + fs::create_dir_all(expected.parent().unwrap())?; + fs::write(&expected, actual).with_context(|| format!("failed to write {expected:?}"))?; + } else { + assert_eq!( + fs::read_to_string(&expected) + .with_context(|| format!("failed to read {expected:?}"))? + .replace("\r\n", "\n"), + actual, + "expectation `{}` did not match actual", + expected.display(), + ); + } + Ok(()) +} + +fn assert_valid_resolve(resolve: &Resolve) { + let mut package_documents = Vec::new(); + for (id, package) in resolve.packages.iter() { + for (name, doc) in package.documents.iter() { + let doc = &resolve.documents[*doc]; + assert_eq!(*name, doc.name); + assert_eq!(doc.package, Some(id)); + } + package_documents.push(package.documents.values().copied().collect::>()); + } + + let mut document_interfaces = Vec::new(); + let mut document_worlds = Vec::new(); + for (id, doc) in resolve.documents.iter() { + if let Some(pkgid) = doc.package { + assert!(resolve.packages.get(pkgid).is_some()); + assert!(package_documents[pkgid.index()].contains(&id)); + } + let mut interfaces = HashSet::new(); + for (name, iface) in doc.interfaces.iter() { + assert!(interfaces.insert(*iface)); + let iface = &resolve.interfaces[*iface]; + assert_eq!(name, iface.name.as_ref().unwrap()); + assert_eq!(iface.document, id); + } + document_interfaces.push(doc.interfaces.values().copied().collect::>()); + let mut worlds = HashSet::new(); + for (name, world) in doc.worlds.iter() { + assert!(worlds.insert(*world)); + let world = &resolve.worlds[*world]; + assert_eq!(*name, world.name); + assert_eq!(world.document, id); + } + document_worlds.push(doc.worlds.values().copied().collect::>()); + + if let Some(id) = doc.default_interface { + assert!(interfaces.contains(&id)); + } + if let Some(id) = doc.default_world { + assert!(worlds.contains(&id)); + } + } + + let mut interface_types = Vec::new(); + for (id, iface) in resolve.interfaces.iter() { + assert!(resolve.documents.get(iface.document).is_some()); + if iface.name.is_some() { + assert!(document_interfaces[iface.document.index()].contains(&id)); + } + + for (name, ty) in iface.types.iter() { + let ty = &resolve.types[*ty]; + assert_eq!(ty.name.as_ref(), Some(name)); + assert_eq!(ty.owner, TypeOwner::Interface(id)); + } + interface_types.push(iface.types.values().copied().collect::>()); + for (name, f) in iface.functions.iter() { + assert_eq!(*name, f.name); + } + } + + let mut world_types = Vec::new(); + for (id, world) in resolve.worlds.iter() { + assert!(resolve.documents.get(world.document).is_some()); + assert!(document_worlds[world.document.index()].contains(&id)); + + let mut types = HashSet::new(); + for (name, item) in world.imports.iter().chain(world.exports.iter()) { + match item { + WorldItem::Interface(_) => {} + WorldItem::Function(f) => { + assert_eq!(f.name, *name); + } + WorldItem::Type(ty) => { + assert!(types.insert(*ty)); + let ty = &resolve.types[*ty]; + assert_eq!(ty.name.as_ref(), Some(name)); + assert_eq!(ty.owner, TypeOwner::World(id)); + } + } + } + world_types.push(types); + } + + for (ty_id, ty) in resolve.types.iter() { + match ty.owner { + TypeOwner::Interface(id) => { + assert!(resolve.interfaces.get(id).is_some()); + assert!(interface_types[id.index()].contains(&ty_id)); + } + TypeOwner::World(id) => { + assert!(resolve.worlds.get(id).is_some()); + assert!(world_types[id.index()].contains(&ty_id)); + } + TypeOwner::None => {} + } + } +} diff --git a/crates/wit-component/tests/merge/bad-interface1/error.txt b/crates/wit-component/tests/merge/bad-interface1/error.txt new file mode 100644 index 0000000000..0fbe06bf28 --- /dev/null +++ b/crates/wit-component/tests/merge/bad-interface1/error.txt @@ -0,0 +1,6 @@ +failed to merge package `foo` into existing copy + +Caused by: + 0: failed to merge document `a` into existing copy + 1: failed to merge interface `a` + 2: expected type `a` to be present \ No newline at end of file diff --git a/crates/wit-component/tests/merge/bad-interface1/from/a.wit b/crates/wit-component/tests/merge/bad-interface1/from/a.wit new file mode 100644 index 0000000000..e4ad26d3d7 --- /dev/null +++ b/crates/wit-component/tests/merge/bad-interface1/from/a.wit @@ -0,0 +1,3 @@ +interface a { + use foo.a.{a} +} diff --git a/crates/wit-component/tests/merge/bad-interface1/from/deps/foo/a.wit b/crates/wit-component/tests/merge/bad-interface1/from/deps/foo/a.wit new file mode 100644 index 0000000000..27e826261a --- /dev/null +++ b/crates/wit-component/tests/merge/bad-interface1/from/deps/foo/a.wit @@ -0,0 +1,4 @@ +default interface a { + type a = u32 +} + diff --git a/crates/wit-component/tests/merge/bad-interface1/into/a.wit b/crates/wit-component/tests/merge/bad-interface1/into/a.wit new file mode 100644 index 0000000000..a26bc50b92 --- /dev/null +++ b/crates/wit-component/tests/merge/bad-interface1/into/a.wit @@ -0,0 +1,3 @@ +interface a { + use foo.a.{b} +} diff --git a/crates/wit-component/tests/merge/bad-interface1/into/deps/foo/a.wit b/crates/wit-component/tests/merge/bad-interface1/into/deps/foo/a.wit new file mode 100644 index 0000000000..dd381ab466 --- /dev/null +++ b/crates/wit-component/tests/merge/bad-interface1/into/deps/foo/a.wit @@ -0,0 +1,4 @@ +default interface a { + type b = s32 +} + diff --git a/crates/wit-component/tests/merge/bad-interface2/error.txt b/crates/wit-component/tests/merge/bad-interface2/error.txt new file mode 100644 index 0000000000..5f9d3ffa85 --- /dev/null +++ b/crates/wit-component/tests/merge/bad-interface2/error.txt @@ -0,0 +1,6 @@ +failed to merge package `foo` into existing copy + +Caused by: + 0: failed to merge document `a` into existing copy + 1: failed to merge interface `a` + 2: expected function `a` to be present \ No newline at end of file diff --git a/crates/wit-component/tests/merge/bad-interface2/from/a.wit b/crates/wit-component/tests/merge/bad-interface2/from/a.wit new file mode 100644 index 0000000000..199015ee5e --- /dev/null +++ b/crates/wit-component/tests/merge/bad-interface2/from/a.wit @@ -0,0 +1,3 @@ +interface a { + use foo.a.{t} +} diff --git a/crates/wit-component/tests/merge/bad-interface2/from/deps/foo/a.wit b/crates/wit-component/tests/merge/bad-interface2/from/deps/foo/a.wit new file mode 100644 index 0000000000..6e227228b2 --- /dev/null +++ b/crates/wit-component/tests/merge/bad-interface2/from/deps/foo/a.wit @@ -0,0 +1,6 @@ +default interface a { + type t = u32 + + a: func() +} + diff --git a/crates/wit-component/tests/merge/bad-interface2/into/a.wit b/crates/wit-component/tests/merge/bad-interface2/into/a.wit new file mode 100644 index 0000000000..199015ee5e --- /dev/null +++ b/crates/wit-component/tests/merge/bad-interface2/into/a.wit @@ -0,0 +1,3 @@ +interface a { + use foo.a.{t} +} diff --git a/crates/wit-component/tests/merge/bad-interface2/into/deps/foo/a.wit b/crates/wit-component/tests/merge/bad-interface2/into/deps/foo/a.wit new file mode 100644 index 0000000000..ffacff0cb2 --- /dev/null +++ b/crates/wit-component/tests/merge/bad-interface2/into/deps/foo/a.wit @@ -0,0 +1,6 @@ +default interface a { + type t = u32 + + b: func() +} + diff --git a/crates/wit-component/tests/merge/bad-world1/error.txt b/crates/wit-component/tests/merge/bad-world1/error.txt new file mode 100644 index 0000000000..3f557f31e1 --- /dev/null +++ b/crates/wit-component/tests/merge/bad-world1/error.txt @@ -0,0 +1,6 @@ +failed to merge package `foo` into existing copy + +Caused by: + 0: failed to merge document `a` into existing copy + 1: failed to merge world `foo` + 2: import `b` not found in target world \ No newline at end of file diff --git a/crates/wit-component/tests/merge/bad-world1/from/a.wit b/crates/wit-component/tests/merge/bad-world1/from/a.wit new file mode 100644 index 0000000000..e4ad26d3d7 --- /dev/null +++ b/crates/wit-component/tests/merge/bad-world1/from/a.wit @@ -0,0 +1,3 @@ +interface a { + use foo.a.{a} +} diff --git a/crates/wit-component/tests/merge/bad-world1/from/deps/foo/a.wit b/crates/wit-component/tests/merge/bad-world1/from/deps/foo/a.wit new file mode 100644 index 0000000000..806f2dff23 --- /dev/null +++ b/crates/wit-component/tests/merge/bad-world1/from/deps/foo/a.wit @@ -0,0 +1,7 @@ +default interface a { + type a = u32 +} + +world foo { + import b: self.a +} diff --git a/crates/wit-component/tests/merge/bad-world1/into/a.wit b/crates/wit-component/tests/merge/bad-world1/into/a.wit new file mode 100644 index 0000000000..e4ad26d3d7 --- /dev/null +++ b/crates/wit-component/tests/merge/bad-world1/into/a.wit @@ -0,0 +1,3 @@ +interface a { + use foo.a.{a} +} diff --git a/crates/wit-component/tests/merge/bad-world1/into/deps/foo/a.wit b/crates/wit-component/tests/merge/bad-world1/into/deps/foo/a.wit new file mode 100644 index 0000000000..a74effb045 --- /dev/null +++ b/crates/wit-component/tests/merge/bad-world1/into/deps/foo/a.wit @@ -0,0 +1,7 @@ +default interface a { + type a = u32 +} + +world foo { + import a: self.a +} diff --git a/crates/wit-component/tests/merge/bad-world2/error.txt b/crates/wit-component/tests/merge/bad-world2/error.txt new file mode 100644 index 0000000000..f85100543c --- /dev/null +++ b/crates/wit-component/tests/merge/bad-world2/error.txt @@ -0,0 +1,6 @@ +failed to merge package `foo` into existing copy + +Caused by: + 0: failed to merge document `a` into existing copy + 1: failed to merge world `foo` + 2: world contains different number of imports than expected \ No newline at end of file diff --git a/crates/wit-component/tests/merge/bad-world2/from/a.wit b/crates/wit-component/tests/merge/bad-world2/from/a.wit new file mode 100644 index 0000000000..e4ad26d3d7 --- /dev/null +++ b/crates/wit-component/tests/merge/bad-world2/from/a.wit @@ -0,0 +1,3 @@ +interface a { + use foo.a.{a} +} diff --git a/crates/wit-component/tests/merge/bad-world2/from/deps/foo/a.wit b/crates/wit-component/tests/merge/bad-world2/from/deps/foo/a.wit new file mode 100644 index 0000000000..48460f0c39 --- /dev/null +++ b/crates/wit-component/tests/merge/bad-world2/from/deps/foo/a.wit @@ -0,0 +1,6 @@ +default interface a { + type a = u32 +} + +world foo { +} diff --git a/crates/wit-component/tests/merge/bad-world2/into/a.wit b/crates/wit-component/tests/merge/bad-world2/into/a.wit new file mode 100644 index 0000000000..e4ad26d3d7 --- /dev/null +++ b/crates/wit-component/tests/merge/bad-world2/into/a.wit @@ -0,0 +1,3 @@ +interface a { + use foo.a.{a} +} diff --git a/crates/wit-component/tests/merge/bad-world2/into/deps/foo/a.wit b/crates/wit-component/tests/merge/bad-world2/into/deps/foo/a.wit new file mode 100644 index 0000000000..a74effb045 --- /dev/null +++ b/crates/wit-component/tests/merge/bad-world2/into/deps/foo/a.wit @@ -0,0 +1,7 @@ +default interface a { + type a = u32 +} + +world foo { + import a: self.a +} diff --git a/crates/wit-component/tests/merge/bad-world3/error.txt b/crates/wit-component/tests/merge/bad-world3/error.txt new file mode 100644 index 0000000000..a427a9e472 --- /dev/null +++ b/crates/wit-component/tests/merge/bad-world3/error.txt @@ -0,0 +1,6 @@ +failed to merge package `foo` into existing copy + +Caused by: + 0: failed to merge document `a` into existing copy + 1: failed to merge world `foo` + 2: world contains different number of exports than expected \ No newline at end of file diff --git a/crates/wit-component/tests/merge/bad-world3/from/a.wit b/crates/wit-component/tests/merge/bad-world3/from/a.wit new file mode 100644 index 0000000000..e4ad26d3d7 --- /dev/null +++ b/crates/wit-component/tests/merge/bad-world3/from/a.wit @@ -0,0 +1,3 @@ +interface a { + use foo.a.{a} +} diff --git a/crates/wit-component/tests/merge/bad-world3/from/deps/foo/a.wit b/crates/wit-component/tests/merge/bad-world3/from/deps/foo/a.wit new file mode 100644 index 0000000000..48460f0c39 --- /dev/null +++ b/crates/wit-component/tests/merge/bad-world3/from/deps/foo/a.wit @@ -0,0 +1,6 @@ +default interface a { + type a = u32 +} + +world foo { +} diff --git a/crates/wit-component/tests/merge/bad-world3/into/a.wit b/crates/wit-component/tests/merge/bad-world3/into/a.wit new file mode 100644 index 0000000000..e4ad26d3d7 --- /dev/null +++ b/crates/wit-component/tests/merge/bad-world3/into/a.wit @@ -0,0 +1,3 @@ +interface a { + use foo.a.{a} +} diff --git a/crates/wit-component/tests/merge/bad-world3/into/deps/foo/a.wit b/crates/wit-component/tests/merge/bad-world3/into/deps/foo/a.wit new file mode 100644 index 0000000000..2e891eac5f --- /dev/null +++ b/crates/wit-component/tests/merge/bad-world3/into/deps/foo/a.wit @@ -0,0 +1,7 @@ +default interface a { + type a = u32 +} + +world foo { + export a: self.a +} diff --git a/crates/wit-component/tests/merge/bad-world4/error.txt b/crates/wit-component/tests/merge/bad-world4/error.txt new file mode 100644 index 0000000000..e113a02ea1 --- /dev/null +++ b/crates/wit-component/tests/merge/bad-world4/error.txt @@ -0,0 +1,6 @@ +failed to merge package `foo` into existing copy + +Caused by: + 0: failed to merge document `a` into existing copy + 1: failed to merge world `foo` + 2: export `b` not found in target world \ No newline at end of file diff --git a/crates/wit-component/tests/merge/bad-world4/from/a.wit b/crates/wit-component/tests/merge/bad-world4/from/a.wit new file mode 100644 index 0000000000..e4ad26d3d7 --- /dev/null +++ b/crates/wit-component/tests/merge/bad-world4/from/a.wit @@ -0,0 +1,3 @@ +interface a { + use foo.a.{a} +} diff --git a/crates/wit-component/tests/merge/bad-world4/from/deps/foo/a.wit b/crates/wit-component/tests/merge/bad-world4/from/deps/foo/a.wit new file mode 100644 index 0000000000..6a593a7e1c --- /dev/null +++ b/crates/wit-component/tests/merge/bad-world4/from/deps/foo/a.wit @@ -0,0 +1,7 @@ +default interface a { + type a = u32 +} + +world foo { + export b: self.a +} diff --git a/crates/wit-component/tests/merge/bad-world4/into/a.wit b/crates/wit-component/tests/merge/bad-world4/into/a.wit new file mode 100644 index 0000000000..e4ad26d3d7 --- /dev/null +++ b/crates/wit-component/tests/merge/bad-world4/into/a.wit @@ -0,0 +1,3 @@ +interface a { + use foo.a.{a} +} diff --git a/crates/wit-component/tests/merge/bad-world4/into/deps/foo/a.wit b/crates/wit-component/tests/merge/bad-world4/into/deps/foo/a.wit new file mode 100644 index 0000000000..2e891eac5f --- /dev/null +++ b/crates/wit-component/tests/merge/bad-world4/into/deps/foo/a.wit @@ -0,0 +1,7 @@ +default interface a { + type a = u32 +} + +world foo { + export a: self.a +} diff --git a/crates/wit-component/tests/merge/bad-world5/error.txt b/crates/wit-component/tests/merge/bad-world5/error.txt new file mode 100644 index 0000000000..91b49b9396 --- /dev/null +++ b/crates/wit-component/tests/merge/bad-world5/error.txt @@ -0,0 +1,7 @@ +failed to merge package `foo` into existing copy + +Caused by: + 0: failed to merge document `a` into existing copy + 1: failed to merge world `foo` + 2: import `a` didn't match target world + 3: interfaces are not the same \ No newline at end of file diff --git a/crates/wit-component/tests/merge/bad-world5/from/a.wit b/crates/wit-component/tests/merge/bad-world5/from/a.wit new file mode 100644 index 0000000000..e4ad26d3d7 --- /dev/null +++ b/crates/wit-component/tests/merge/bad-world5/from/a.wit @@ -0,0 +1,3 @@ +interface a { + use foo.a.{a} +} diff --git a/crates/wit-component/tests/merge/bad-world5/from/deps/foo/a.wit b/crates/wit-component/tests/merge/bad-world5/from/deps/foo/a.wit new file mode 100644 index 0000000000..88ee308faf --- /dev/null +++ b/crates/wit-component/tests/merge/bad-world5/from/deps/foo/a.wit @@ -0,0 +1,9 @@ +default interface a { + type a = u32 +} + +world foo { + import a: interface { + type a = u32 + } +} diff --git a/crates/wit-component/tests/merge/bad-world5/into/a.wit b/crates/wit-component/tests/merge/bad-world5/into/a.wit new file mode 100644 index 0000000000..e4ad26d3d7 --- /dev/null +++ b/crates/wit-component/tests/merge/bad-world5/into/a.wit @@ -0,0 +1,3 @@ +interface a { + use foo.a.{a} +} diff --git a/crates/wit-component/tests/merge/bad-world5/into/deps/foo/a.wit b/crates/wit-component/tests/merge/bad-world5/into/deps/foo/a.wit new file mode 100644 index 0000000000..a74effb045 --- /dev/null +++ b/crates/wit-component/tests/merge/bad-world5/into/deps/foo/a.wit @@ -0,0 +1,7 @@ +default interface a { + type a = u32 +} + +world foo { + import a: self.a +} diff --git a/crates/wit-component/tests/merge/success/from/a.wit b/crates/wit-component/tests/merge/success/from/a.wit new file mode 100644 index 0000000000..67cbcfc9e3 --- /dev/null +++ b/crates/wit-component/tests/merge/success/from/a.wit @@ -0,0 +1,5 @@ +interface a { + use foo.only-from.only-from.{r} + + foo: func() +} diff --git a/crates/wit-component/tests/merge/success/from/deps/foo/only-from.wit b/crates/wit-component/tests/merge/success/from/deps/foo/only-from.wit new file mode 100644 index 0000000000..5c4c62d7ad --- /dev/null +++ b/crates/wit-component/tests/merge/success/from/deps/foo/only-from.wit @@ -0,0 +1,5 @@ +interface only-from { + record r {} + + foo: func() -> r +} diff --git a/crates/wit-component/tests/merge/success/from/deps/foo/shared.wit b/crates/wit-component/tests/merge/success/from/deps/foo/shared.wit new file mode 100644 index 0000000000..c0edd7054c --- /dev/null +++ b/crates/wit-component/tests/merge/success/from/deps/foo/shared.wit @@ -0,0 +1,23 @@ +interface only-from { + variant v { + c1, + } + + bar: func(x: v) + + use only-from-dep.a.{a} +} + +interface shared { + type a = u32 +} + +world shared-world { + import a: self.shared + + export b: self.shared + + type c = u32 + + import d: interface {} +} diff --git a/crates/wit-component/tests/merge/success/from/deps/only-from-dep/a.wit b/crates/wit-component/tests/merge/success/from/deps/only-from-dep/a.wit new file mode 100644 index 0000000000..b2c77a17c4 --- /dev/null +++ b/crates/wit-component/tests/merge/success/from/deps/only-from-dep/a.wit @@ -0,0 +1,3 @@ +default interface foo { + type a = u32 +} diff --git a/crates/wit-component/tests/merge/success/into/b.wit b/crates/wit-component/tests/merge/success/into/b.wit new file mode 100644 index 0000000000..44ee7aea90 --- /dev/null +++ b/crates/wit-component/tests/merge/success/into/b.wit @@ -0,0 +1,5 @@ +interface b { + use foo.only-into.only-into.{r} + + foo: func() +} diff --git a/crates/wit-component/tests/merge/success/into/deps/foo/only-into.wit b/crates/wit-component/tests/merge/success/into/deps/foo/only-into.wit new file mode 100644 index 0000000000..012f2b9616 --- /dev/null +++ b/crates/wit-component/tests/merge/success/into/deps/foo/only-into.wit @@ -0,0 +1,5 @@ +interface only-into { + record r {} + + foo: func() -> r +} diff --git a/crates/wit-component/tests/merge/success/into/deps/foo/shared.wit b/crates/wit-component/tests/merge/success/into/deps/foo/shared.wit new file mode 100644 index 0000000000..cfa3eb14e4 --- /dev/null +++ b/crates/wit-component/tests/merge/success/into/deps/foo/shared.wit @@ -0,0 +1,21 @@ +interface only-into { + variant v { + c1, + } + + bar: func(x: v) +} + +interface shared { + type a = u32 +} + +world shared-world { + import a: self.shared + + export b: self.shared + + type c = u32 + + import d: interface {} +} diff --git a/crates/wit-component/tests/merge/success/merge/foo/only-from.wit b/crates/wit-component/tests/merge/success/merge/foo/only-from.wit new file mode 100644 index 0000000000..f8258177a0 --- /dev/null +++ b/crates/wit-component/tests/merge/success/merge/foo/only-from.wit @@ -0,0 +1,7 @@ +interface only-from { + record r { + } + + foo: func() -> r +} + diff --git a/crates/wit-component/tests/merge/success/merge/foo/only-into.wit b/crates/wit-component/tests/merge/success/merge/foo/only-into.wit new file mode 100644 index 0000000000..ae159a367c --- /dev/null +++ b/crates/wit-component/tests/merge/success/merge/foo/only-into.wit @@ -0,0 +1,7 @@ +interface only-into { + record r { + } + + foo: func() -> r +} + diff --git a/crates/wit-component/tests/merge/success/merge/foo/shared.wit b/crates/wit-component/tests/merge/success/merge/foo/shared.wit new file mode 100644 index 0000000000..68fb5714e1 --- /dev/null +++ b/crates/wit-component/tests/merge/success/merge/foo/shared.wit @@ -0,0 +1,31 @@ +interface shared { + type a = u32 + +} + +interface only-into { + variant v { + c1, + } + + bar: func(x: v) +} + +interface only-from { + use only-from-dep.a.foo.{a} + + variant v { + c1, + } + + bar: func(x: v) +} + +world shared-world { + import a: self.shared + import d: interface { + } + type c = u32 + + export b: self.shared +} diff --git a/crates/wit-component/tests/merge/success/merge/from/a.wit b/crates/wit-component/tests/merge/success/merge/from/a.wit new file mode 100644 index 0000000000..09f2ae02d6 --- /dev/null +++ b/crates/wit-component/tests/merge/success/merge/from/a.wit @@ -0,0 +1,5 @@ +interface a { + use foo.only-from.only-from.{r} + foo: func() +} + diff --git a/crates/wit-component/tests/merge/success/merge/into/b.wit b/crates/wit-component/tests/merge/success/merge/into/b.wit new file mode 100644 index 0000000000..fe57f4668b --- /dev/null +++ b/crates/wit-component/tests/merge/success/merge/into/b.wit @@ -0,0 +1,5 @@ +interface b { + use foo.only-into.only-into.{r} + foo: func() +} + diff --git a/crates/wit-component/tests/merge/success/merge/only-from-dep/a.wit b/crates/wit-component/tests/merge/success/merge/only-from-dep/a.wit new file mode 100644 index 0000000000..bac7278001 --- /dev/null +++ b/crates/wit-component/tests/merge/success/merge/only-from-dep/a.wit @@ -0,0 +1,5 @@ +default interface foo { + type a = u32 + +} + diff --git a/crates/wit-parser/src/resolve.rs b/crates/wit-parser/src/resolve.rs index 4bdca9899f..e20f63f3fa 100644 --- a/crates/wit-parser/src/resolve.rs +++ b/crates/wit-parser/src/resolve.rs @@ -234,7 +234,53 @@ impl Resolve { /// Merges all the contents of a different `Resolve` into this one. The /// `Remap` structure returned provides a mapping from all old indices to /// new indices - pub fn merge(&mut self, resolve: Resolve) -> Remap { + /// + /// This operation can fail if `resolve` disagrees with `self` about the + /// packages being inserted. Otherwise though this will additionally attempt + /// to "union" packages found in `resolve` with those found in `self`. + /// Unioning packages is keyed on the name/url of packages for those with + /// URLs present. If found then it's assumed that both `Resolve` instances + /// were originally created from the same contents and are two views + /// of the same package. + pub fn merge(&mut self, resolve: Resolve) -> Result { + log::trace!( + "merging {} packages into {} packages", + resolve.packages.len(), + self.packages.len() + ); + + let mut map = MergeMap::new(&resolve, &self)?; + map.build()?; + let MergeMap { + package_map, + interface_map, + type_map, + doc_map, + world_map, + documents_to_add, + interfaces_to_add, + worlds_to_add, + .. + } = map; + + // With a set of maps from ids in `resolve` to ids in `self` the next + // operation is to start moving over items and building a `Remap` to + // update ids. + // + // Each component field of `resolve` is moved into `self` so long as + // its ID is not within one of the maps above. If it's present in a map + // above then that means the item is already present in `self` so a new + // one need not be added. If it's not present in a map that means it's + // not present in `self` so it must be added to an arena. + // + // When adding an item to an arena one of the `remap.update_*` methods + // is additionally called to update all identifiers from pointers within + // `resolve` to becoming pointers within `self`. + // + // Altogether this should weave all the missing items in `self` from + // `resolve` into one structure while updating all identifiers to + // be local within `self`. + let mut remap = Remap::default(); let Resolve { types, @@ -244,36 +290,52 @@ impl Resolve { packages, } = resolve; + let mut moved_types = Vec::new(); for (id, mut ty) in types { - remap.update_typedef(&mut ty); - let new_id = self.types.alloc(ty); + let new_id = type_map.get(&id).copied().unwrap_or_else(|| { + moved_types.push(id); + remap.update_typedef(&mut ty); + self.types.alloc(ty) + }); assert_eq!(remap.types.len(), id.index()); remap.types.push(new_id); } + let mut moved_interfaces = Vec::new(); for (id, mut iface) in interfaces { - remap.update_interface(&mut iface); - let new_id = self.interfaces.alloc(iface); + let new_id = interface_map.get(&id).copied().unwrap_or_else(|| { + moved_interfaces.push(id); + remap.update_interface(&mut iface); + self.interfaces.alloc(iface) + }); assert_eq!(remap.interfaces.len(), id.index()); remap.interfaces.push(new_id); } + let mut moved_worlds = Vec::new(); for (id, mut world) in worlds { - for (_, item) in world.imports.iter_mut().chain(&mut world.exports) { - match item { - WorldItem::Function(f) => remap.update_function(f), - WorldItem::Interface(i) => *i = remap.interfaces[i.index()], - WorldItem::Type(i) => *i = remap.types[i.index()], + let new_id = world_map.get(&id).copied().unwrap_or_else(|| { + moved_worlds.push(id); + for (_, item) in world.imports.iter_mut().chain(&mut world.exports) { + match item { + WorldItem::Function(f) => remap.update_function(f), + WorldItem::Interface(i) => *i = remap.interfaces[i.index()], + WorldItem::Type(i) => *i = remap.types[i.index()], + } } - } - let new_id = self.worlds.alloc(world); + self.worlds.alloc(world) + }); assert_eq!(remap.worlds.len(), id.index()); remap.worlds.push(new_id); } + let mut moved_documents = Vec::new(); for (id, mut doc) in documents { - remap.update_document(&mut doc); - let new_id = self.documents.alloc(doc); + let new_id = doc_map.get(&id).copied().unwrap_or_else(|| { + moved_documents.push(id); + remap.update_document(&mut doc); + self.documents.alloc(doc) + }); assert_eq!(remap.documents.len(), id.index()); remap.documents.push(new_id); } @@ -282,26 +344,39 @@ impl Resolve { for (_, doc) in pkg.documents.iter_mut() { *doc = remap.documents[doc.index()]; } - let new_id = self.packages.alloc(pkg); + let new_id = package_map + .get(&id) + .copied() + .unwrap_or_else(|| self.packages.alloc(pkg)); assert_eq!(remap.packages.len(), id.index()); remap.packages.push(new_id); } - // Fixup all "parent" links now - for id in remap.documents.iter().copied() { + // Fixup all "parent" links now. + // + // Note that this is only done for items that are actually moved from + // `resolve` into `self`, which is tracked by the various `moved_*` + // lists built incrementally above. The ids in the `moved_*` lists + // are ids within `resolve`, so they're translated through `remap` to + // ids within `self`. + for id in moved_documents { + let id = remap.documents[id.index()]; if let Some(pkg) = &mut self.documents[id].package { *pkg = remap.packages[pkg.index()]; } } - for id in remap.worlds.iter().copied() { + for id in moved_worlds { + let id = remap.worlds[id.index()]; let doc = &mut self.worlds[id].document; *doc = remap.documents[doc.index()]; } - for id in remap.interfaces.iter().copied() { + for id in moved_interfaces { + let id = remap.interfaces[id.index()]; let doc = &mut self.interfaces[id].document; *doc = remap.documents[doc.index()]; } - for id in remap.types.iter().copied() { + for id in moved_types { + let id = remap.types[id.index()]; match &mut self.types[id].owner { TypeOwner::Interface(id) => *id = remap.interfaces[id.index()], TypeOwner::World(id) => *id = remap.worlds[id.index()], @@ -309,7 +384,31 @@ impl Resolve { } } - remap + // And finally process documents that were present in `resolve` but were + // not present in `self`. This is only done for merged packages as + // documents may be added to `self.documents` but wouldn't otherwise be + // present in the `documents` field of the corresponding package. + for (name, pkg, doc) in documents_to_add { + let prev = self.packages[pkg] + .documents + .insert(name, remap.documents[doc.index()]); + assert!(prev.is_none()); + } + for (name, doc, iface) in interfaces_to_add { + let prev = self.documents[doc] + .interfaces + .insert(name, remap.interfaces[iface.index()]); + assert!(prev.is_none()); + } + for (name, doc, world) in worlds_to_add { + let prev = self.documents[doc] + .worlds + .insert(name, remap.worlds[world.index()]); + assert!(prev.is_none()); + } + + log::trace!("now have {} packages", self.packages.len()); + Ok(remap) } /// Merges the world `from` into the world `into`. @@ -322,21 +421,76 @@ impl Resolve { /// interface. /// /// This operation can fail if the imports/exports overlap. - // - // TODO: overlap shouldn't be a hard error here, there should be some form - // of comparing names/urls/deep merging or such to get this working. pub fn merge_worlds(&mut self, from: WorldId, into: WorldId) -> Result<()> { let mut new_imports = Vec::new(); let mut new_exports = Vec::new(); let from_world = &self.worlds[from]; let into_world = &self.worlds[into]; + + // Build a map of the imports/exports in `into` going the reverse + // direction from what's listed. This is then consulted below to ensure + // that the same item isn't exported or imported under two different + // names which isn't allowed in the component model. + let mut into_imports_by_id = HashMap::new(); + let mut into_exports_by_id = HashMap::new(); + for (name, import) in into_world.imports.iter() { + if let WorldItem::Interface(id) = *import { + let prev = into_imports_by_id.insert(id, name); + assert!(prev.is_none()); + } + } + for (name, export) in into_world.exports.iter() { + if let WorldItem::Interface(id) = *export { + let prev = into_exports_by_id.insert(id, name); + assert!(prev.is_none()); + } + } for (name, import) in from_world.imports.iter() { + // If the "from" world imports an interface which is already + // imported by the "into" world then this is allowed if the names + // are the same. Importing the same interface under different names + // isn't allowed, but otherwise merging imports of + // same-named-interfaces is allowed to merge them together. + if let WorldItem::Interface(id) = import { + if let Some(prev) = into_imports_by_id.get(id) { + if *prev != name { + bail!("import `{name}` conflicts with previous name of `{prev}`"); + } + } + } + } + for (name, export) in from_world.exports.iter() { + // Note that unlike imports same-named exports are not handled here + // since if something is exported twice there's no way to "unify" it + // so it's left as an error. + if let WorldItem::Interface(id) = export { + if let Some(prev) = into_exports_by_id.get(id) { + bail!("export `{name}` conflicts with previous name of `{prev}`"); + } + } + } + + // Next walk over the interfaces imported into `from_world` and queue up + // imports to get inserted into `into_world`. + for (name, from_import) in from_world.imports.iter() { match into_world.imports.get(name) { - Some(_) => bail!("duplicate import found for interface `{name}`"), - None => new_imports.push((name.clone(), import.clone())), + Some(into_import) => match (from_import, into_import) { + // If these imports, which have the same name, are of the + // same interface then union them together at this point. + (WorldItem::Interface(from), WorldItem::Interface(into)) if from == into => { + continue + } + _ => bail!("duplicate import found for interface `{name}`"), + }, + None => new_imports.push((name.clone(), from_import.clone())), } } + + // All exports at this time must be unique. For example the same + // interface exported from two locations can't really be resolved to one + // canonical definition, so make sure that merging worlds only succeeds + // if the worlds have disjoint sets of exports. for (name, export) in from_world.exports.iter() { match into_world.exports.get(name) { Some(_) => bail!("duplicate export found for interface `{name}`"), @@ -344,13 +498,14 @@ impl Resolve { } } + // Insert any new imports and new exports found first. let into = &mut self.worlds[into]; for (name, import) in new_imports { let prev = into.imports.insert(name, import); assert!(prev.is_none()); } - for (name, import) in new_exports { - let prev = into.exports.insert(name, import); + for (name, export) in new_exports { + let prev = into.exports.insert(name, export); assert!(prev.is_none()); } @@ -1022,3 +1177,302 @@ impl<'a> WorldElaborator<'a, '_> { .unwrap_or_else(|| self.resolve.interfaces[id].name.as_ref().unwrap()) } } + +struct MergeMap<'a> { + /// A map of package ids in `from` to those in `into` for those that are + /// found to be equivalent. + package_map: HashMap, + + /// A map of interface ids in `from` to those in `into` for those that are + /// found to be equivalent. + interface_map: HashMap, + + /// A map of type ids in `from` to those in `into` for those that are + /// found to be equivalent. + type_map: HashMap, + + /// A map of document ids in `from` to those in `into` for those that are + /// found to be equivalent. + doc_map: HashMap, + + /// A map of world ids in `from` to those in `into` for those that are + /// found to be equivalent. + world_map: HashMap, + + /// A list of documents that need to be added to packages in `into`. + /// + /// The elements here are: + /// + /// * The name of the document + /// * The ID within `into` of the package being added to + /// * The ID within `from` of the document being added. + documents_to_add: Vec<(String, PackageId, DocumentId)>, + interfaces_to_add: Vec<(String, DocumentId, InterfaceId)>, + worlds_to_add: Vec<(String, DocumentId, WorldId)>, + + /// Which `Resolve` is being merged from. + from: &'a Resolve, + + /// Which `Resolve` is being merged into. + into: &'a Resolve, + + /// A cache of packages, keyed by name/url, within `into`. + packages_in_into: HashMap<(&'a String, &'a Option), PackageId>, +} + +impl<'a> MergeMap<'a> { + fn new(from: &'a Resolve, into: &'a Resolve) -> Result> { + let mut packages_in_into = HashMap::new(); + for (id, package) in into.packages.iter() { + log::trace!("previous package {}/{:?}", package.name, package.url); + if package.url.is_none() { + continue; + } + let prev = packages_in_into.insert((&package.name, &package.url), id); + if prev.is_some() { + bail!( + "found duplicate name/url combination in current resolve: {}/{:?}", + package.name, + package.url + ); + } + } + Ok(MergeMap { + package_map: Default::default(), + interface_map: Default::default(), + type_map: Default::default(), + doc_map: Default::default(), + world_map: Default::default(), + documents_to_add: Default::default(), + interfaces_to_add: Default::default(), + worlds_to_add: Default::default(), + from, + into, + packages_in_into, + }) + } + + fn build(&mut self) -> Result<()> { + for (from_id, from) in self.from.packages.iter() { + let into_id = match self.packages_in_into.get(&(&from.name, &from.url)) { + Some(id) => *id, + + // This package, according to its name and url, is not present + // in `self` so it needs to get added below. + None => { + log::trace!("adding unique package {} / {:?}", from.name, from.url); + continue; + } + }; + log::trace!("merging duplicate package {} / {:?}", from.name, from.url); + + self.build_package(from_id, into_id).with_context(|| { + format!("failed to merge package `{}` into existing copy", from.name) + })?; + } + + Ok(()) + } + + fn build_package(&mut self, from_id: PackageId, into_id: PackageId) -> Result<()> { + let prev = self.package_map.insert(from_id, into_id); + assert!(prev.is_none()); + + let from = &self.from.packages[from_id]; + let into = &self.into.packages[into_id]; + + // All documents in `from` should already be present in `into` to get + // merged, or it's assumed `self.from` contains a view of the package + // which happens to contain more files. In this situation the job of + // merging will be to add a new document to the package within + // `self.into` which is queued up with `self.documents_to_add`. + for (name, from_id) in from.documents.iter() { + let into_id = match into.documents.get(name) { + Some(id) => *id, + None => { + self.documents_to_add + .push((name.clone(), into_id, *from_id)); + continue; + } + }; + + self.build_document(*from_id, into_id) + .with_context(|| format!("failed to merge document `{name}` into existing copy"))?; + } + + Ok(()) + } + + fn build_document(&mut self, from_id: DocumentId, into_id: DocumentId) -> Result<()> { + let prev = self.doc_map.insert(from_id, into_id); + assert!(prev.is_none()); + + let from_doc = &self.from.documents[from_id]; + let into_doc = &self.into.documents[into_id]; + + // Like documents above if an interface is present in `from_id` but not + // present in `into_id` then it can be copied over wholesale. That + // copy is scheduled to happen within the `self.interfaces_to_add` list. + for (name, from_interface_id) in from_doc.interfaces.iter() { + let into_interface_id = match into_doc.interfaces.get(name) { + Some(id) => *id, + None => { + self.interfaces_to_add + .push((name.clone(), into_id, *from_interface_id)); + continue; + } + }; + + self.build_interface(*from_interface_id, into_interface_id) + .with_context(|| format!("failed to merge interface `{name}`"))?; + } + + for (name, from_world_id) in from_doc.worlds.iter() { + let into_world_id = match into_doc.worlds.get(name) { + Some(id) => *id, + None => { + self.worlds_to_add + .push((name.clone(), into_id, *from_world_id)); + continue; + } + }; + + self.build_world(*from_world_id, into_world_id) + .with_context(|| format!("failed to merge world `{name}`"))?; + } + Ok(()) + } + + fn build_interface(&mut self, from_id: InterfaceId, into_id: InterfaceId) -> Result<()> { + let prev = self.interface_map.insert(from_id, into_id); + assert!(prev.is_none()); + + let from_interface = &self.from.interfaces[from_id]; + let into_interface = &self.into.interfaces[into_id]; + + // Unlike documents/interfaces above if an interface in `from` + // differs from the interface in `into` then that's considered an + // error. Changing interfaces can reflect changes in imports/exports + // which may not be expected so it's currently required that all + // interfaces, when merged, exactly match. + // + // One case to consider here, for example, is that if a world in + // `into` exports the interface `into_id` then if `from_id` were to + // add more items into `into` then it would unexpectedly require more + // items to be exported which may not work. In an import context this + // might work since it's "just more items available for import", but + // for now a conservative route of "interfaces must match" is taken. + + for (name, from_type_id) in from_interface.types.iter() { + let into_type_id = *into_interface + .types + .get(name) + .ok_or_else(|| anyhow!("expected type `{name}` to be present"))?; + let prev = self.type_map.insert(*from_type_id, into_type_id); + assert!(prev.is_none()); + + // FIXME: ideally the types should be "structurally + // equal" but that's not trivial to do in the face of + // resources. + } + + for (name, _) in from_interface.functions.iter() { + if !into_interface.functions.contains_key(name) { + bail!("expected function `{name}` to be present"); + } + + // FIXME: ideally the functions should be "structurally + // equal" but that's not trivial to do in the face of + // resources. + } + + Ok(()) + } + + fn build_world(&mut self, from_id: WorldId, into_id: WorldId) -> Result<()> { + let prev = self.world_map.insert(from_id, into_id); + assert!(prev.is_none()); + + let from_world = &self.from.worlds[from_id]; + let into_world = &self.into.worlds[into_id]; + + // Same as interfaces worlds are expected to exactly match to avoid + // unexpectedly changing a particular component's view of imports and + // exports. + // + // FIXME: this should probably share functionality with + // `Resolve::merge_worlds` to support adding imports but not changing + // exports. + + if from_world.imports.len() != into_world.imports.len() { + bail!("world contains different number of imports than expected"); + } + if from_world.exports.len() != into_world.exports.len() { + bail!("world contains different number of exports than expected"); + } + + for (name, from) in from_world.imports.iter() { + let into = into_world + .imports + .get(name) + .ok_or_else(|| anyhow!("import `{name}` not found in target world"))?; + self.match_world_item(from, into) + .with_context(|| format!("import `{name}` didn't match target world"))?; + } + + for (name, from) in from_world.exports.iter() { + let into = into_world + .exports + .get(name) + .ok_or_else(|| anyhow!("export `{name}` not found in target world"))?; + self.match_world_item(from, into) + .with_context(|| format!("export `{name}` didn't match target world"))?; + } + + Ok(()) + } + + fn match_world_item(&mut self, from: &WorldItem, into: &WorldItem) -> Result<()> { + match (from, into) { + (WorldItem::Interface(from), WorldItem::Interface(into)) => { + match ( + &self.from.interfaces[*from].name, + &self.into.interfaces[*into].name, + ) { + // If one interface is unnamed then they must both be + // unnamed and they must both have the same structure for + // now. + (None, None) => self.build_interface(*from, *into)?, + + // Otherwise both interfaces must be named and they must + // have been previously found to be equivalent. Note that + // if either is unnamed it won't be present in + // `interface_map` so this'll return an error. + _ => { + if self.interface_map.get(&from) != Some(&into) { + bail!("interfaces are not the same"); + } + } + } + } + (WorldItem::Function(from), WorldItem::Function(into)) => { + drop((from, into)); + // FIXME: should assert an check that `from` structurally + // matches `into` + } + (WorldItem::Type(from), WorldItem::Type(into)) => { + // FIXME: should assert an check that `from` structurally + // matches `into` + let prev = self.type_map.insert(*from, *into); + assert!(prev.is_none()); + } + + (WorldItem::Interface(_), _) + | (WorldItem::Function(_), _) + | (WorldItem::Type(_), _) => { + bail!("world items do not have the same type") + } + } + Ok(()) + } +} From 372add8f9238efb1b6ad9a7502452dc763d8fe91 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 11 Apr 2023 07:27:44 -0700 Subject: [PATCH 2/2] Fix an outdated comment --- crates/wit-component/src/encoding.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/crates/wit-component/src/encoding.rs b/crates/wit-component/src/encoding.rs index 9271015730..466d941f1d 100644 --- a/crates/wit-component/src/encoding.rs +++ b/crates/wit-component/src/encoding.rs @@ -1428,8 +1428,14 @@ impl ComponentEncoder { pub fn adapter(mut self, name: &str, bytes: &[u8]) -> Result { let (wasm, metadata) = metadata::decode(bytes)?; // Merge the adapter's document into our own document to have one large - // document, but the adapter's world isn't merged in to our world so - // retain it separately. + // document, and then afterwards merge worlds as well. + // + // The first `merge` operation will interleave equivalent packages from + // each adapter into packages that are stored within our own resolve. + // The second `merge_worlds` operation will then ensure that both the + // adapter and the main module have compatible worlds, meaning that they + // either import the same items or they import disjoint items, for + // example. let world = self .metadata .resolve