From fb68a1fd548d254590b94cb3d3a84f7515734130 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Tue, 30 Apr 2024 17:45:56 +0200 Subject: [PATCH 1/2] Add a test for a dependency issue The issue is that dependencies of implicit imports and explicit imports are not merged and thus if you've already imported a dependency of an implicit import, the dependency of the explicit import will effectively be ignored. This is an issue when the explicit import dependency has different items than the version imported as part of the implicit import. --- .../component/deps/types.wit | 11 ++++++++ .../component/world.wit | 10 ++++++++ .../merging-import-dependencies/graph.json | 25 +++++++++++++++++++ .../import/deps/types.wit | 11 ++++++++ .../import/world.wit | 10 ++++++++ 5 files changed, 67 insertions(+) create mode 100644 crates/wac-graph/tests/graphs/merging-import-dependencies/component/deps/types.wit create mode 100644 crates/wac-graph/tests/graphs/merging-import-dependencies/component/world.wit create mode 100644 crates/wac-graph/tests/graphs/merging-import-dependencies/graph.json create mode 100644 crates/wac-graph/tests/graphs/merging-import-dependencies/import/deps/types.wit create mode 100644 crates/wac-graph/tests/graphs/merging-import-dependencies/import/world.wit diff --git a/crates/wac-graph/tests/graphs/merging-import-dependencies/component/deps/types.wit b/crates/wac-graph/tests/graphs/merging-import-dependencies/component/deps/types.wit new file mode 100644 index 0000000..2f0b3c3 --- /dev/null +++ b/crates/wac-graph/tests/graphs/merging-import-dependencies/component/deps/types.wit @@ -0,0 +1,11 @@ +package foo:dependency; + +interface types { + record my-record { + foo: string + } +} + +world w { + export types; +} \ No newline at end of file diff --git a/crates/wac-graph/tests/graphs/merging-import-dependencies/component/world.wit b/crates/wac-graph/tests/graphs/merging-import-dependencies/component/world.wit new file mode 100644 index 0000000..e57822f --- /dev/null +++ b/crates/wac-graph/tests/graphs/merging-import-dependencies/component/world.wit @@ -0,0 +1,10 @@ +package foo:component; + +world w { + export my-interface; +} + +interface my-interface{ + use foo:dependency/types.{my-record}; + my-func: func() -> my-record; +} \ No newline at end of file diff --git a/crates/wac-graph/tests/graphs/merging-import-dependencies/graph.json b/crates/wac-graph/tests/graphs/merging-import-dependencies/graph.json new file mode 100644 index 0000000..53c5145 --- /dev/null +++ b/crates/wac-graph/tests/graphs/merging-import-dependencies/graph.json @@ -0,0 +1,25 @@ +{ + "packages": [ + { + "name": "foo:import", + "path": "import" + }, + { + "name": "test:component", + "path": "component" + } + ], + "nodes": [ + { + "type": "import", + "name": "foo:test-import/my-interface", + "package": 0, + "export": "foo:test-import/my-interface" + }, + { + "type": "instantiation", + "package": 1 + } + ], + "arguments": [] +} \ No newline at end of file diff --git a/crates/wac-graph/tests/graphs/merging-import-dependencies/import/deps/types.wit b/crates/wac-graph/tests/graphs/merging-import-dependencies/import/deps/types.wit new file mode 100644 index 0000000..f0e2448 --- /dev/null +++ b/crates/wac-graph/tests/graphs/merging-import-dependencies/import/deps/types.wit @@ -0,0 +1,11 @@ +package foo:dependency; + +interface types { + variant my-variant { + x + } +} + +world w { + export types; +} \ No newline at end of file diff --git a/crates/wac-graph/tests/graphs/merging-import-dependencies/import/world.wit b/crates/wac-graph/tests/graphs/merging-import-dependencies/import/world.wit new file mode 100644 index 0000000..583da6b --- /dev/null +++ b/crates/wac-graph/tests/graphs/merging-import-dependencies/import/world.wit @@ -0,0 +1,10 @@ +package foo:test-import; + +world w { + export my-interface; +} + +interface my-interface{ + use foo:dependency/types.{my-variant}; + my-func: func() -> my-variant; +} \ No newline at end of file From 4577cddd01fe9ebddb094605367a14249c02ff1b Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Thu, 2 May 2024 12:08:07 +0200 Subject: [PATCH 2/2] Fix dependency issue Make sure explicit imports are aggregated with implicit imports. Signed-off-by: Ryan Levick --- crates/wac-graph/src/graph.rs | 63 ++++++++++++++----- .../merging-import-dependencies/encoded.wat | 47 ++++++++++++++ 2 files changed, 94 insertions(+), 16 deletions(-) create mode 100644 crates/wac-graph/tests/graphs/merging-import-dependencies/encoded.wat diff --git a/crates/wac-graph/src/graph.rs b/crates/wac-graph/src/graph.rs index 8da6144..7d9b34e 100644 --- a/crates/wac-graph/src/graph.rs +++ b/crates/wac-graph/src/graph.rs @@ -1377,22 +1377,28 @@ impl<'a> CompositionGraphEncoder<'a> { fn encode(self, options: EncodeOptions) -> Result, EncodeError> { let mut state = State::new(); - // First populate the state with the implicit instantiation arguments - self.populate_implicit_args(&mut state)?; - - // Encode each node in the graph in topographical order - for n in self + // Separate import nodes from other nodes keeping topological order + let (import_nodes, other_nodes) = self .toposort() .map_err(|n| EncodeError::GraphContainsCycle { node: NodeId(n) })? - { + .into_iter() + .partition::, _>(|index| { + let node = &self.0.graph[*index]; + matches!(node.kind, NodeKind::Import(_)) + }); + + // First populate the state with both implicit instantiation arguments and explicit imports + self.encode_imports(&mut state, import_nodes)?; + + // Encode non-import nodes in the graph in topographical order + for n in other_nodes { let node = &self.0.graph[n]; let index = match &node.kind { NodeKind::Definition => self.definition(&mut state, node), - NodeKind::Import(name) => { - self.import(&mut state, name, &self.0.types, node.item_kind) - } 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, index); @@ -1482,13 +1488,19 @@ impl<'a> CompositionGraphEncoder<'a> { Ok(finish_stack) } - fn populate_implicit_args(&self, state: &mut State) -> Result<(), EncodeError> { + /// Encode both implicit and explicit imports. + fn encode_imports( + &self, + state: &mut State, + 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(); log::debug!("populating implicit imports"); @@ -1502,13 +1514,14 @@ impl<'a> CompositionGraphEncoder<'a> { let package = &self.0[node.package.unwrap()]; let world = &self.0.types[package.ty()]; - // Go through the unsatisfied arguments and import them - for (_, (name, kind)) in world + let unsatisfied_args = world .imports .iter() .enumerate() - .filter(|(i, _)| !node.is_arg_satisfied(*i)) - { + .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), @@ -1532,14 +1545,26 @@ impl<'a> CompositionGraphEncoder<'a> { } } + 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(); + } + } + // Next encode the imports for (name, kind) in aggregator.imports() { - log::debug!("import `{name}` is being implicitly imported"); + log::debug!("import `{name}` is being imported"); let index = self.import(state, name, aggregator.types(), kind); encoded.insert(name, (kind.into(), index)); } - // Finally populate the implicit argument map + // Populate the implicit argument map for (node, name) in arguments { let (kind, index) = encoded[name.as_str()]; state @@ -1549,6 +1574,12 @@ impl<'a> CompositionGraphEncoder<'a> { .push((name.clone(), 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); + } + Ok(()) } diff --git a/crates/wac-graph/tests/graphs/merging-import-dependencies/encoded.wat b/crates/wac-graph/tests/graphs/merging-import-dependencies/encoded.wat new file mode 100644 index 0000000..c344958 --- /dev/null +++ b/crates/wac-graph/tests/graphs/merging-import-dependencies/encoded.wat @@ -0,0 +1,47 @@ +(component + (type (;0;) + (instance + (type (;0;) (record (field "foo" string))) + (export (;1;) "my-record" (type (eq 0))) + (type (;2;) (variant (case "x"))) + (export (;3;) "my-variant" (type (eq 2))) + ) + ) + (import "foo:dependency/types" (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;) "my-func" (func (type 2))) + ) + ) + (import "foo:test-import/my-interface" (instance (;1;) (type 2))) + (type (;3;) + (component + (type (;0;) + (instance + (type (;0;) (record (field "foo" string))) + (export (;1;) "my-record" (type (eq 0))) + ) + ) + (import "foo:dependency/types" (instance (;0;) (type 0))) + (alias export 0 "my-record" (type (;1;))) + (type (;2;) + (instance + (alias outer 1 1 (type (;0;))) + (export (;1;) "my-record" (type (eq 0))) + (type (;2;) (func (result 1))) + (export (;0;) "my-func" (func (type 2))) + ) + ) + (export (;1;) "foo:component/my-interface" (instance (type 2))) + ) + ) + (import "unlocked-dep=" (component (;0;) (type 3))) + (instance (;2;) (instantiate 0 + (with "foo:dependency/types" (instance 0)) + ) + ) +)