From 1a3d3cc4473b8787c66d3acfaec627f684b6e871 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Wed, 3 Jul 2024 17:33:08 +0200 Subject: [PATCH 1/6] Resolve implicit imports before doing target check. --- crates/wac-graph/src/graph.rs | 173 ++++++++++++++++------------ crates/wac-parser/src/resolution.rs | 17 +-- 2 files changed, 107 insertions(+), 83 deletions(-) diff --git a/crates/wac-graph/src/graph.rs b/crates/wac-graph/src/graph.rs index 7d9b34e9..fcd4c9a3 100644 --- a/crates/wac-graph/src/graph.rs +++ b/crates/wac-graph/src/graph.rs @@ -1259,6 +1259,89 @@ impl CompositionGraph { generation: entry.generation, } } + + /// Yields an iterator over the resolved imports of the graph. + pub fn imports(&self) -> Result, EncodeError> { + let import_nodes = self + .node_ids() + .filter_map(|n| match &self[n].kind() { + NodeKind::Import(_) => Some(n), + _ => None, + }) + .collect(); + let mut implicit_imports = Default::default(); + let mut explicit_imports = Default::default(); + let _ = self.resolve_imports(import_nodes, &mut implicit_imports, &mut explicit_imports)?; + + Ok(implicit_imports.into_iter().chain(explicit_imports)) + } + + /// Resolves the imports (both implicit and explicit) of the given nodes. + /// + /// Populates hashmaps that map the implicit and explicit import nodes to their import names. + /// Returns a type aggregator that contains the resolved types of the imports. + pub fn resolve_imports<'a>( + &'a self, + import_nodes: Vec, + implicit_imports: &mut HashMap<&'a str, NodeId>, + explicit_imports: &mut HashMap<&'a str, NodeId>, + ) -> Result { + let mut instantiations = HashMap::new(); + let mut aggregator = TypeAggregator::default(); + let mut cache = Default::default(); + let mut checker = SubtypeChecker::new(&mut cache); + log::debug!("populating implicit imports"); + for index in self.graph.node_indices() { + let node = &self.graph[index]; + if !matches!(node.kind, NodeKind::Instantiation(_)) { + continue; + } + + let package = &self[node.package.unwrap()]; + let world = &self.types[package.ty()]; + + let unsatisfied_args = world + .imports + .iter() + .enumerate() + .filter(|(i, _)| !node.is_arg_satisfied(*i)); + + // Go through the unsatisfied arguments and import them + for (_, (name, kind)) in unsatisfied_args { + if let Some(import) = self.imports.get(name).copied() { + return Err(EncodeError::ImplicitImportConflict { + import: NodeId(import), + instantiation: NodeId(index), + package: PackageKey::new(package), + name: name.to_string(), + }); + } + + instantiations.entry(name).or_insert(index); + + aggregator = aggregator + .aggregate(name, &self.types, *kind, &mut checker) + .map_err(|e| EncodeError::ImportTypeMergeConflict { + import: name.clone(), + first: NodeId(instantiations[&name]), + second: NodeId(index), + source: e, + })?; + implicit_imports.insert(name, NodeId(index)); + } + } + log::debug!("populating explicit imports"); + for n in import_nodes { + let node = &self.graph[n.0]; + if let NodeKind::Import(name) = &node.kind { + explicit_imports.insert(name.as_str(), n); + aggregator = aggregator + .aggregate(name, self.types(), node.item_kind, &mut checker) + .unwrap(); + } + } + Ok(aggregator) + } } impl Index for CompositionGraph { @@ -1382,8 +1465,9 @@ impl<'a> CompositionGraphEncoder<'a> { .toposort() .map_err(|n| EncodeError::GraphContainsCycle { node: NodeId(n) })? .into_iter() + .map(NodeId) .partition::, _>(|index| { - let node = &self.0.graph[*index]; + let node = &self.0.graph[index.0]; matches!(node.kind, NodeKind::Import(_)) }); @@ -1392,16 +1476,16 @@ impl<'a> CompositionGraphEncoder<'a> { // Encode non-import nodes in the graph in topographical order for n in other_nodes { - let node = &self.0.graph[n]; + let node = &self.0.graph[n.0]; let index = match &node.kind { NodeKind::Definition => self.definition(&mut state, node), - NodeKind::Instantiation(_) => self.instantiation(&mut state, n, node, options), - NodeKind::Alias => self.alias(&mut state, n), + NodeKind::Instantiation(_) => self.instantiation(&mut state, n.0, node, options), + NodeKind::Alias => self.alias(&mut state, n.0), // `other_nodes` does not contain any import nodes NodeKind::Import(_) => unreachable!(), }; - let prev = state.node_indexes.insert(n, index); + let prev = state.node_indexes.insert(n.0, index); assert!(prev.is_none()); } @@ -1489,73 +1573,20 @@ impl<'a> CompositionGraphEncoder<'a> { } /// Encode both implicit and explicit imports. + /// + /// `import_nodes` are expected to only be `NodeKind::Import` nodes. fn encode_imports( &self, state: &mut State, - import_nodes: Vec, + import_nodes: Vec, ) -> Result<(), EncodeError> { - let mut aggregator = TypeAggregator::default(); - let mut instantiations = HashMap::new(); - let mut arguments = Vec::new(); - let mut encoded = HashMap::new(); - let mut cache = Default::default(); - let mut checker = SubtypeChecker::new(&mut cache); let mut explicit_imports = HashMap::new(); + let mut implicit_imports = HashMap::new(); + let aggregator = + self.0 + .resolve_imports(import_nodes, &mut implicit_imports, &mut explicit_imports)?; - log::debug!("populating implicit imports"); - - // Enumerate the instantiation nodes and populate the import types - for index in self.0.graph.node_indices() { - let node = &self.0.graph[index]; - if !matches!(node.kind, NodeKind::Instantiation(_)) { - continue; - } - - let package = &self.0[node.package.unwrap()]; - let world = &self.0.types[package.ty()]; - - let unsatisfied_args = world - .imports - .iter() - .enumerate() - .filter(|(i, _)| !node.is_arg_satisfied(*i)); - - // Go through the unsatisfied arguments and import them - for (_, (name, kind)) in unsatisfied_args { - if let Some(import) = self.0.imports.get(name).copied() { - return Err(EncodeError::ImplicitImportConflict { - import: NodeId(import), - instantiation: NodeId(index), - package: PackageKey::new(package), - name: name.to_string(), - }); - } - - instantiations.entry(name).or_insert(index); - - aggregator = aggregator - .aggregate(name, &self.0.types, *kind, &mut checker) - .map_err(|e| EncodeError::ImportTypeMergeConflict { - import: name.clone(), - first: NodeId(instantiations[&name]), - second: NodeId(index), - source: e, - })?; - arguments.push((index, name)); - } - } - - log::debug!("populating explicit imports"); - - for n in import_nodes { - let node = &self.0.graph[n]; - if let NodeKind::Import(name) = &node.kind { - explicit_imports.insert(name.as_str(), n); - aggregator = aggregator - .aggregate(name, self.0.types(), node.item_kind, &mut checker) - .unwrap(); - } - } + let mut encoded = HashMap::new(); // Next encode the imports for (name, kind) in aggregator.imports() { @@ -1565,19 +1596,19 @@ impl<'a> CompositionGraphEncoder<'a> { } // Populate the implicit argument map - for (node, name) in arguments { - let (kind, index) = encoded[name.as_str()]; + for (name, node) in implicit_imports { + let (kind, index) = encoded[name]; state .implicit_args - .entry(node) + .entry(node.0) .or_default() - .push((name.clone(), kind, index)); + .push((name.to_owned(), kind, index)); } // Finally, populate the node indexes with the encoded explicit imports for (name, node_index) in explicit_imports { let (_, encoded_index) = encoded[name]; - state.node_indexes.insert(node_index, encoded_index); + state.node_indexes.insert(node_index.0, encoded_index); } Ok(()) diff --git a/crates/wac-parser/src/resolution.rs b/crates/wac-parser/src/resolution.rs index a5371443..af2b06cc 100644 --- a/crates/wac-parser/src/resolution.rs +++ b/crates/wac-parser/src/resolution.rs @@ -609,7 +609,7 @@ pub enum Error { world: String, /// The span where the error occurred. #[label(primary, "cannot have an import named `{name}`")] - span: SourceSpan, + span: Option, }, /// Missing an export for the target world. #[error("target world `{world}` requires an export named `{name}`")] @@ -2717,21 +2717,14 @@ impl<'a> AstResolver<'a> { // The output is allowed to import a subset of the world's imports checker.invert(); - for (name, import) in state - .graph - .node_ids() - .filter_map(|n| match &state.graph[n].kind() { - NodeKind::Import(name) => Some((name, n)), - _ => None, - }) - { + for (name, import) in state.graph.imports().unwrap() { let expected = world .imports .get(name) .ok_or_else(|| Error::ImportNotInTarget { - name: name.clone(), + name: name.to_owned(), world: path.string.to_owned(), - span: state.import_spans[&import], + span: state.import_spans.get(&import).copied(), })?; checker @@ -2743,7 +2736,7 @@ impl<'a> AstResolver<'a> { ) .map_err(|e| Error::TargetMismatch { kind: ExternKind::Import, - name: name.clone(), + name: name.to_owned(), world: path.string.to_owned(), span: state.import_spans[&import], source: e, From 3fb7ef4f0c437513845c3bd4486c4d7437c74a41 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Wed, 3 Jul 2024 17:42:29 +0200 Subject: [PATCH 2/6] Don't build type aggregator when populating import iterator Signed-off-by: Ryan Levick --- crates/wac-graph/src/graph.rs | 162 ++++++++++++++++------------ crates/wac-parser/src/resolution.rs | 2 +- 2 files changed, 92 insertions(+), 72 deletions(-) diff --git a/crates/wac-graph/src/graph.rs b/crates/wac-graph/src/graph.rs index fcd4c9a3..108583a0 100644 --- a/crates/wac-graph/src/graph.rs +++ b/crates/wac-graph/src/graph.rs @@ -1260,37 +1260,9 @@ impl CompositionGraph { } } - /// Yields an iterator over the resolved imports of the graph. - pub fn imports(&self) -> Result, EncodeError> { - let import_nodes = self - .node_ids() - .filter_map(|n| match &self[n].kind() { - NodeKind::Import(_) => Some(n), - _ => None, - }) - .collect(); - let mut implicit_imports = Default::default(); - let mut explicit_imports = Default::default(); - let _ = self.resolve_imports(import_nodes, &mut implicit_imports, &mut explicit_imports)?; - - Ok(implicit_imports.into_iter().chain(explicit_imports)) - } - - /// Resolves the imports (both implicit and explicit) of the given nodes. - /// - /// Populates hashmaps that map the implicit and explicit import nodes to their import names. - /// Returns a type aggregator that contains the resolved types of the imports. - pub fn resolve_imports<'a>( - &'a self, - import_nodes: Vec, - implicit_imports: &mut HashMap<&'a str, NodeId>, - explicit_imports: &mut HashMap<&'a str, NodeId>, - ) -> Result { - let mut instantiations = HashMap::new(); - let mut aggregator = TypeAggregator::default(); - let mut cache = Default::default(); - let mut checker = SubtypeChecker::new(&mut cache); - log::debug!("populating implicit imports"); + /// Yields an iterator over the resolved imports (both implicit and explicit) of the graph. + pub fn imports(&self) -> impl Iterator { + let mut imports = HashMap::new(); for index in self.graph.node_indices() { let node = &self.graph[index]; if !matches!(node.kind, NodeKind::Instantiation(_)) { @@ -1307,40 +1279,18 @@ impl CompositionGraph { .filter(|(i, _)| !node.is_arg_satisfied(*i)); // Go through the unsatisfied arguments and import them - for (_, (name, kind)) in unsatisfied_args { - if let Some(import) = self.imports.get(name).copied() { - return Err(EncodeError::ImplicitImportConflict { - import: NodeId(import), - instantiation: NodeId(index), - package: PackageKey::new(package), - name: name.to_string(), - }); - } - - instantiations.entry(name).or_insert(index); - - aggregator = aggregator - .aggregate(name, &self.types, *kind, &mut checker) - .map_err(|e| EncodeError::ImportTypeMergeConflict { - import: name.clone(), - first: NodeId(instantiations[&name]), - second: NodeId(index), - source: e, - })?; - implicit_imports.insert(name, NodeId(index)); + for (_, (name, _)) in unsatisfied_args { + imports.insert(name.as_str(), NodeId(index)); } } - log::debug!("populating explicit imports"); - for n in import_nodes { + + for n in self.node_ids() { let node = &self.graph[n.0]; if let NodeKind::Import(name) = &node.kind { - explicit_imports.insert(name.as_str(), n); - aggregator = aggregator - .aggregate(name, self.types(), node.item_kind, &mut checker) - .unwrap(); + imports.insert(name.as_str(), n); } } - Ok(aggregator) + imports.into_iter() } } @@ -1465,9 +1415,8 @@ impl<'a> CompositionGraphEncoder<'a> { .toposort() .map_err(|n| EncodeError::GraphContainsCycle { node: NodeId(n) })? .into_iter() - .map(NodeId) .partition::, _>(|index| { - let node = &self.0.graph[index.0]; + let node = &self.0.graph[*index]; matches!(node.kind, NodeKind::Import(_)) }); @@ -1476,16 +1425,16 @@ impl<'a> CompositionGraphEncoder<'a> { // Encode non-import nodes in the graph in topographical order for n in other_nodes { - let node = &self.0.graph[n.0]; + let node = &self.0.graph[n]; let index = match &node.kind { NodeKind::Definition => self.definition(&mut state, node), - NodeKind::Instantiation(_) => self.instantiation(&mut state, n.0, node, options), - NodeKind::Alias => self.alias(&mut state, n.0), + NodeKind::Instantiation(_) => self.instantiation(&mut state, n, node, options), + NodeKind::Alias => self.alias(&mut state, n), // `other_nodes` does not contain any import nodes NodeKind::Import(_) => unreachable!(), }; - let prev = state.node_indexes.insert(n.0, index); + let prev = state.node_indexes.insert(n, index); assert!(prev.is_none()); } @@ -1578,13 +1527,12 @@ impl<'a> CompositionGraphEncoder<'a> { fn encode_imports( &self, state: &mut State, - import_nodes: Vec, + import_nodes: Vec, ) -> Result<(), EncodeError> { let mut explicit_imports = HashMap::new(); - let mut implicit_imports = HashMap::new(); + let mut implicit_imports = Vec::new(); let aggregator = - self.0 - .resolve_imports(import_nodes, &mut implicit_imports, &mut explicit_imports)?; + self.resolve_imports(import_nodes, &mut implicit_imports, &mut explicit_imports)?; let mut encoded = HashMap::new(); @@ -1600,7 +1548,7 @@ impl<'a> CompositionGraphEncoder<'a> { let (kind, index) = encoded[name]; state .implicit_args - .entry(node.0) + .entry(node) .or_default() .push((name.to_owned(), kind, index)); } @@ -1608,12 +1556,84 @@ impl<'a> CompositionGraphEncoder<'a> { // Finally, populate the node indexes with the encoded explicit imports for (name, node_index) in explicit_imports { let (_, encoded_index) = encoded[name]; - state.node_indexes.insert(node_index.0, encoded_index); + state.node_indexes.insert(node_index, encoded_index); } Ok(()) } + /// Resolves the imports (both implicit and explicit) of the given nodes. + /// + /// Populates hashmaps that map the implicit and explicit import nodes to their import names. + /// Returns a type aggregator that contains the resolved types of the imports. + fn resolve_imports( + &'a self, + import_nodes: Vec, + implicit_imports: &mut Vec<(&'a str, NodeIndex)>, + explicit_imports: &mut HashMap<&'a str, NodeIndex>, + ) -> Result { + let mut instantiations = HashMap::new(); + let mut aggregator = TypeAggregator::default(); + let mut cache = Default::default(); + let mut checker = SubtypeChecker::new(&mut cache); + + log::debug!("populating implicit imports"); + + // Enumerate the instantiation nodes and populate the import types + for index in self.0.graph.node_indices() { + let node = &self.0.graph[index]; + if !matches!(node.kind, NodeKind::Instantiation(_)) { + continue; + } + + let package = &self.0[node.package.unwrap()]; + let world = &self.0.types[package.ty()]; + + let unsatisfied_args = world + .imports + .iter() + .enumerate() + .filter(|(i, _)| !node.is_arg_satisfied(*i)); + + // Go through the unsatisfied arguments and import them + for (_, (name, kind)) in unsatisfied_args { + if let Some(import) = self.0.imports.get(name).copied() { + return Err(EncodeError::ImplicitImportConflict { + import: NodeId(import), + instantiation: NodeId(index), + package: PackageKey::new(package), + name: name.to_string(), + }); + } + + instantiations.entry(name).or_insert(index); + + aggregator = aggregator + .aggregate(name, &self.0.types, *kind, &mut checker) + .map_err(|e| EncodeError::ImportTypeMergeConflict { + import: name.clone(), + first: NodeId(instantiations[&name]), + second: NodeId(index), + source: e, + })?; + implicit_imports.push((name, index)); + } + } + + log::debug!("populating explicit imports"); + + for n in import_nodes { + let node = &self.0.graph[n]; + if let NodeKind::Import(name) = &node.kind { + explicit_imports.insert(name.as_str(), n); + aggregator = aggregator + .aggregate(name, self.0.types(), node.item_kind, &mut checker) + .unwrap(); + } + } + Ok(aggregator) + } + fn definition(&self, state: &mut State, node: &Node) -> u32 { let name = node.export.as_deref().unwrap(); diff --git a/crates/wac-parser/src/resolution.rs b/crates/wac-parser/src/resolution.rs index af2b06cc..013aa1aa 100644 --- a/crates/wac-parser/src/resolution.rs +++ b/crates/wac-parser/src/resolution.rs @@ -2717,7 +2717,7 @@ impl<'a> AstResolver<'a> { // The output is allowed to import a subset of the world's imports checker.invert(); - for (name, import) in state.graph.imports().unwrap() { + for (name, import) in state.graph.imports() { let expected = world .imports .get(name) From 85e06510d709cf7ae6fcecceb0918ae79abbcfb6 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Thu, 4 Jul 2024 14:04:32 +0200 Subject: [PATCH 3/6] Don't assume item is the top level item from the node Signed-off-by: Ryan Levick --- crates/wac-graph/src/graph.rs | 12 +++++++----- crates/wac-parser/src/resolution.rs | 12 ++++++------ 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/crates/wac-graph/src/graph.rs b/crates/wac-graph/src/graph.rs index 108583a0..09fc6852 100644 --- a/crates/wac-graph/src/graph.rs +++ b/crates/wac-graph/src/graph.rs @@ -1261,8 +1261,10 @@ impl CompositionGraph { } /// Yields an iterator over the resolved imports (both implicit and explicit) of the graph. - pub fn imports(&self) -> impl Iterator { - let mut imports = HashMap::new(); + /// + /// The iterator returns the name, the `ItemKind`, and an optional node id for explicit imports. + pub fn imports(&self) -> impl Iterator)> { + let mut imports = Vec::new(); for index in self.graph.node_indices() { let node = &self.graph[index]; if !matches!(node.kind, NodeKind::Instantiation(_)) { @@ -1279,15 +1281,15 @@ impl CompositionGraph { .filter(|(i, _)| !node.is_arg_satisfied(*i)); // Go through the unsatisfied arguments and import them - for (_, (name, _)) in unsatisfied_args { - imports.insert(name.as_str(), NodeId(index)); + for (_, (name, item_kind)) in unsatisfied_args { + imports.push((name.as_str(), *item_kind, None)); } } for n in self.node_ids() { let node = &self.graph[n.0]; if let NodeKind::Import(name) = &node.kind { - imports.insert(name.as_str(), n); + imports.push((name.as_str(), node.item_kind, Some(n))); } } imports.into_iter() diff --git a/crates/wac-parser/src/resolution.rs b/crates/wac-parser/src/resolution.rs index 013aa1aa..c136fe86 100644 --- a/crates/wac-parser/src/resolution.rs +++ b/crates/wac-parser/src/resolution.rs @@ -635,7 +635,7 @@ pub enum Error { world: String, /// The span where the error occurred. #[label(primary, "mismatched type for {kind} `{name}`")] - span: SourceSpan, + span: Option, /// The source of the error. #[source] source: anyhow::Error, @@ -2717,28 +2717,28 @@ impl<'a> AstResolver<'a> { // The output is allowed to import a subset of the world's imports checker.invert(); - for (name, import) in state.graph.imports() { + for (name, item_kind, import_node) in state.graph.imports() { let expected = world .imports .get(name) .ok_or_else(|| Error::ImportNotInTarget { name: name.to_owned(), world: path.string.to_owned(), - span: state.import_spans.get(&import).copied(), + span: import_node.map(|n| state.import_spans[&n]), })?; checker .is_subtype( expected.promote(), state.graph.types(), - state.graph[import].item_kind(), + item_kind, state.graph.types(), ) .map_err(|e| Error::TargetMismatch { kind: ExternKind::Import, name: name.to_owned(), world: path.string.to_owned(), - span: state.import_spans[&import], + span: import_node.map(|n| state.import_spans[&n]), source: e, })?; } @@ -2769,7 +2769,7 @@ impl<'a> AstResolver<'a> { kind: ExternKind::Export, name: name.clone(), world: path.string.to_owned(), - span: state.export_spans[&export], + span: state.export_spans.get(&export).copied(), source: e, })?; } From 9369149385261fdccb34a8ac4337e01a9c90292a Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Thu, 4 Jul 2024 15:43:36 +0200 Subject: [PATCH 4/6] Add implicit imports test Signed-off-by: Ryan Levick --- .../fail/targets-implicit-imports.wac | 20 +++++++++++++++++++ .../fail/targets-implicit-imports.wac.result | 4 ++++ .../fail/targets-implicit-imports/foo/bar.wat | 4 ++++ 3 files changed, 28 insertions(+) create mode 100644 crates/wac-parser/tests/resolution/fail/targets-implicit-imports.wac create mode 100644 crates/wac-parser/tests/resolution/fail/targets-implicit-imports.wac.result create mode 100644 crates/wac-parser/tests/resolution/fail/targets-implicit-imports/foo/bar.wat diff --git a/crates/wac-parser/tests/resolution/fail/targets-implicit-imports.wac b/crates/wac-parser/tests/resolution/fail/targets-implicit-imports.wac new file mode 100644 index 00000000..39eb5fed --- /dev/null +++ b/crates/wac-parser/tests/resolution/fail/targets-implicit-imports.wac @@ -0,0 +1,20 @@ +package test:comp targets test:comp/foo; + +interface indirect-dependency { + variant my-variant { + foo + } +} + +interface direct-dependency { + use indirect-dependency.{my-variant}; + + fun: func() -> my-variant; +} + +world foo { + import a: direct-dependency; +} + + +let i = new foo:bar { ... }; diff --git a/crates/wac-parser/tests/resolution/fail/targets-implicit-imports.wac.result b/crates/wac-parser/tests/resolution/fail/targets-implicit-imports.wac.result new file mode 100644 index 00000000..80e988df --- /dev/null +++ b/crates/wac-parser/tests/resolution/fail/targets-implicit-imports.wac.result @@ -0,0 +1,4 @@ +failed to resolve document + + × import `a` has a mismatched type for target world `test:comp/foo` + ╰─▶ expected instance, found function diff --git a/crates/wac-parser/tests/resolution/fail/targets-implicit-imports/foo/bar.wat b/crates/wac-parser/tests/resolution/fail/targets-implicit-imports/foo/bar.wat new file mode 100644 index 00000000..6e6a9615 --- /dev/null +++ b/crates/wac-parser/tests/resolution/fail/targets-implicit-imports/foo/bar.wat @@ -0,0 +1,4 @@ +(component + (import "a" (func)) + (export "b" (func 0)) +) \ No newline at end of file From abd2984dfb5f959d0188724eb236096c96dcf405 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Fri, 5 Jul 2024 11:23:27 +0200 Subject: [PATCH 5/6] Also include interfaces imported implicitly through uses in check Signed-off-by: Ryan Levick --- crates/wac-parser/src/resolution.rs | 6 +++-- .../fail/targets-implicit-imports.wac | 6 +++-- .../fail/targets-implicit-imports.wac.result | 5 ++-- .../fail/targets-implicit-imports/foo/bar.wat | 21 +++++++++++++--- crates/wac-types/src/component.rs | 24 +++++++++++++++++++ 5 files changed, 53 insertions(+), 9 deletions(-) diff --git a/crates/wac-parser/src/resolution.rs b/crates/wac-parser/src/resolution.rs index c136fe86..a88612ca 100644 --- a/crates/wac-parser/src/resolution.rs +++ b/crates/wac-parser/src/resolution.rs @@ -2712,15 +2712,17 @@ impl<'a> AstResolver<'a> { world: WorldId, ) -> ResolutionResult<()> { let world = &state.graph.types()[world]; + // The interfaces imported implicitly through uses. + let implicit_imported_interfaces = world.implicit_imported_interfaces(state.graph.types()); let mut cache = Default::default(); let mut checker = SubtypeChecker::new(&mut cache); // The output is allowed to import a subset of the world's imports checker.invert(); for (name, item_kind, import_node) in state.graph.imports() { - let expected = world - .imports + let expected = implicit_imported_interfaces .get(name) + .or_else(|| world.imports.get(name)) .ok_or_else(|| Error::ImportNotInTarget { name: name.to_owned(), world: path.string.to_owned(), diff --git a/crates/wac-parser/tests/resolution/fail/targets-implicit-imports.wac b/crates/wac-parser/tests/resolution/fail/targets-implicit-imports.wac index 39eb5fed..ca49423b 100644 --- a/crates/wac-parser/tests/resolution/fail/targets-implicit-imports.wac +++ b/crates/wac-parser/tests/resolution/fail/targets-implicit-imports.wac @@ -2,7 +2,9 @@ package test:comp targets test:comp/foo; interface indirect-dependency { variant my-variant { - foo + foo, + // Extra variant that the instance does not have + bar } } @@ -13,7 +15,7 @@ interface direct-dependency { } world foo { - import a: direct-dependency; + import direct-dependency; } diff --git a/crates/wac-parser/tests/resolution/fail/targets-implicit-imports.wac.result b/crates/wac-parser/tests/resolution/fail/targets-implicit-imports.wac.result index 80e988df..ed618a67 100644 --- a/crates/wac-parser/tests/resolution/fail/targets-implicit-imports.wac.result +++ b/crates/wac-parser/tests/resolution/fail/targets-implicit-imports.wac.result @@ -1,4 +1,5 @@ failed to resolve document - × import `a` has a mismatched type for target world `test:comp/foo` - ╰─▶ expected instance, found function + × import `test:comp/indirect-dependency` has a mismatched type for target world `test:comp/foo` + ├─▶ mismatched type for export `my-variant` + ╰─▶ expected a variant case count of 2, found a count of 1 diff --git a/crates/wac-parser/tests/resolution/fail/targets-implicit-imports/foo/bar.wat b/crates/wac-parser/tests/resolution/fail/targets-implicit-imports/foo/bar.wat index 6e6a9615..be48df94 100644 --- a/crates/wac-parser/tests/resolution/fail/targets-implicit-imports/foo/bar.wat +++ b/crates/wac-parser/tests/resolution/fail/targets-implicit-imports/foo/bar.wat @@ -1,4 +1,19 @@ (component - (import "a" (func)) - (export "b" (func 0)) -) \ No newline at end of file + (type (;0;) + (instance + (type (;0;) (variant (case "foo"))) + (export (;1;) "my-variant" (type (eq 0))) + ) + ) + (import "test:comp/indirect-dependency" (instance (;0;) (type 0))) + (alias export 0 "my-variant" (type (;1;))) + (type (;2;) + (instance + (alias outer 1 1 (type (;0;))) + (export (;1;) "my-variant" (type (eq 0))) + (type (;2;) (func (result 1))) + (export (;0;) "fun" (func (type 2))) + ) + ) + (import "test:comp/direct-dependency" (instance (;1;) (type 2))) +) diff --git a/crates/wac-types/src/component.rs b/crates/wac-types/src/component.rs index 6e2b2efd..608ac2d3 100644 --- a/crates/wac-types/src/component.rs +++ b/crates/wac-types/src/component.rs @@ -983,6 +983,30 @@ impl World { Ok(()) } + + /// The interfaces imported implicitly through uses. + pub fn implicit_imported_interfaces<'a>( + &'a self, + types: &'a Types, + ) -> IndexMap<&str, ItemKind> { + let mut interfaces = IndexMap::new(); + for (_, import) in self.imports.iter() { + if let ItemKind::Instance(interface_id) = import { + let import = &types[*interface_id]; + for (_, used_item) in &import.uses { + let used_interface_id = used_item.interface; + let used_interface = &types[used_interface_id]; + // The id must be set since used interfaces are always named. + let used_interface_name = used_interface.id.as_ref().unwrap(); + interfaces.insert( + used_interface_name.as_str(), + ItemKind::Instance(used_interface_id), + ); + } + } + } + interfaces + } } /// Represents a kind of an extern item. From e27d65223c6015b0caf7aa4c169421cf3ccee448 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Fri, 5 Jul 2024 12:27:20 +0200 Subject: [PATCH 6/6] Also check world uses Signed-off-by: Ryan Levick --- ...-imports.wac => targets-interface-use.wac} | 0 ...esult => targets-interface-use.wac.result} | 0 .../foo/bar.wat | 0 .../resolution/fail/targets-world-use.wac | 13 ++++++++++++ .../fail/targets-world-use.wac.result | 4 ++++ .../fail/targets-world-use/foo/bar.wat | 14 +++++++++++++ crates/wac-types/src/component.rs | 20 +++++++++++-------- 7 files changed, 43 insertions(+), 8 deletions(-) rename crates/wac-parser/tests/resolution/fail/{targets-implicit-imports.wac => targets-interface-use.wac} (100%) rename crates/wac-parser/tests/resolution/fail/{targets-implicit-imports.wac.result => targets-interface-use.wac.result} (100%) rename crates/wac-parser/tests/resolution/fail/{targets-implicit-imports => targets-interface-use}/foo/bar.wat (100%) create mode 100644 crates/wac-parser/tests/resolution/fail/targets-world-use.wac create mode 100644 crates/wac-parser/tests/resolution/fail/targets-world-use.wac.result create mode 100644 crates/wac-parser/tests/resolution/fail/targets-world-use/foo/bar.wat diff --git a/crates/wac-parser/tests/resolution/fail/targets-implicit-imports.wac b/crates/wac-parser/tests/resolution/fail/targets-interface-use.wac similarity index 100% rename from crates/wac-parser/tests/resolution/fail/targets-implicit-imports.wac rename to crates/wac-parser/tests/resolution/fail/targets-interface-use.wac diff --git a/crates/wac-parser/tests/resolution/fail/targets-implicit-imports.wac.result b/crates/wac-parser/tests/resolution/fail/targets-interface-use.wac.result similarity index 100% rename from crates/wac-parser/tests/resolution/fail/targets-implicit-imports.wac.result rename to crates/wac-parser/tests/resolution/fail/targets-interface-use.wac.result diff --git a/crates/wac-parser/tests/resolution/fail/targets-implicit-imports/foo/bar.wat b/crates/wac-parser/tests/resolution/fail/targets-interface-use/foo/bar.wat similarity index 100% rename from crates/wac-parser/tests/resolution/fail/targets-implicit-imports/foo/bar.wat rename to crates/wac-parser/tests/resolution/fail/targets-interface-use/foo/bar.wat diff --git a/crates/wac-parser/tests/resolution/fail/targets-world-use.wac b/crates/wac-parser/tests/resolution/fail/targets-world-use.wac new file mode 100644 index 00000000..e4c95ed8 --- /dev/null +++ b/crates/wac-parser/tests/resolution/fail/targets-world-use.wac @@ -0,0 +1,13 @@ +package test:comp targets test:comp/foo; + +interface indirect-dependency { + // The resource is actually named "other-resource" in the instance + resource my-resource {} +} + +world foo { + use indirect-dependency.{my-resource}; + import my-func: func() -> my-resource; +} + +let i = new foo:bar { ... }; diff --git a/crates/wac-parser/tests/resolution/fail/targets-world-use.wac.result b/crates/wac-parser/tests/resolution/fail/targets-world-use.wac.result new file mode 100644 index 00000000..05a2db75 --- /dev/null +++ b/crates/wac-parser/tests/resolution/fail/targets-world-use.wac.result @@ -0,0 +1,4 @@ +failed to resolve document + + × import `test:comp/indirect-dependency` has a mismatched type for target world `test:comp/foo` + ╰─▶ instance has unexpected resource export `other-resource` diff --git a/crates/wac-parser/tests/resolution/fail/targets-world-use/foo/bar.wat b/crates/wac-parser/tests/resolution/fail/targets-world-use/foo/bar.wat new file mode 100644 index 00000000..848bf460 --- /dev/null +++ b/crates/wac-parser/tests/resolution/fail/targets-world-use/foo/bar.wat @@ -0,0 +1,14 @@ +(component + (type (;0;) + (instance + (export (;0;) "other-resource" (type (sub resource))) + ) + ) + (import "test:comp/indirect-dependency" (instance (;0;) (type 0))) + (alias export 0 "other-resource" (type (;1;))) + (import "my-resource" (type (;2;) (eq 1))) + (type (;3;) (own 2)) + (type (;4;) (func (result 3))) + (import "my-func" (func (;0;) (type 4))) + (alias export 0 "other-resource" (type (;5;))) +) diff --git a/crates/wac-types/src/component.rs b/crates/wac-types/src/component.rs index 608ac2d3..54d7239c 100644 --- a/crates/wac-types/src/component.rs +++ b/crates/wac-types/src/component.rs @@ -990,18 +990,22 @@ impl World { types: &'a Types, ) -> IndexMap<&str, ItemKind> { let mut interfaces = IndexMap::new(); + let mut add_interface_for_used_type = |used_item: &UsedType| { + let used_interface_id = used_item.interface; + // The id must be set since used interfaces are always named. + let used_interface_name = types[used_interface_id].id.as_deref().unwrap(); + interfaces.insert(used_interface_name, ItemKind::Instance(used_interface_id)); + }; + + for (_, used_type) in self.uses.iter() { + add_interface_for_used_type(used_type); + } + for (_, import) in self.imports.iter() { if let ItemKind::Instance(interface_id) = import { let import = &types[*interface_id]; for (_, used_item) in &import.uses { - let used_interface_id = used_item.interface; - let used_interface = &types[used_interface_id]; - // The id must be set since used interfaces are always named. - let used_interface_name = used_interface.id.as_ref().unwrap(); - interfaces.insert( - used_interface_name.as_str(), - ItemKind::Instance(used_interface_id), - ); + add_interface_for_used_type(used_item); } } }