Merge pull request #101 from rylev/panic-repro

Fix panic when resource's owning interface is not otherwise implicitly imported

crates/wac-graph/src/graph.rs

@@ -1533,7 +1533,7 @@ impl<'a> CompositionGraphEncoder<'a> {
         }
 
         // Next encode the imports
-        for (name, kind) in aggregator.iter() {
+        for (name, kind) in aggregator.imports() {
             log::debug!("import `{name}` is being implicitly imported");
             let index = self.import(state, name, aggregator.types(), kind);
             encoded.insert(name, (kind.into(), index));

crates/wac-graph/tests/encoding.rs

@@ -171,6 +171,7 @@ impl GraphFile {
                         path = path.display()
                     )
                 })?,
+                None if path.is_dir() => Self::load_wit_package(test_case, &path)?,
                 _ => bail!(
                     "unexpected file extension for package file `{path}`",
                     path = package.path.display()
@@ -331,6 +332,7 @@ fn encoding() -> Result<()> {
             })?
             .into_composition_graph(&path, test_case)
             .and_then(|graph| {
+                println!("{:?}", graph);
                 graph
                     .encode(EncodeOptions {
                         // We import the component definitions instead

crates/wac-graph/tests/graphs/implicit-resource-import/bar/deps/types.wit

@@ -0,0 +1,9 @@
+package foo:dependency;
+
+interface types {
+    resource x;
+}
+
+world w {
+    export types;
+}

crates/wac-graph/tests/graphs/implicit-resource-import/bar/world.wit

@@ -0,0 +1,5 @@
+package foo:bar;
+
+world w {
+    export foo:dependency/types;
+}

crates/wac-graph/tests/graphs/implicit-resource-import/baz/deps/types.wit

@@ -0,0 +1,9 @@
+package foo:dependency;
+
+interface types {
+    resource x;
+}
+
+world w {
+    export types;
+}

crates/wac-graph/tests/graphs/implicit-resource-import/baz/world.wit

@@ -0,0 +1,8 @@
+package foo:baz;
+
+world w {
+    use foo:dependency/types.{x};
+    import my-func: func() -> x;
+    // The test will fail whether this is here or not
+    import foo:dependency/types;
+}

crates/wac-graph/tests/graphs/implicit-resource-import/encoded.wat

@@ -0,0 +1,57 @@
+(component
+  (type (;0;)
+    (instance
+      (export (;0;) "x" (type (sub resource)))
+    )
+  )
+  (import "foo:dependency/types" (instance (;0;) (type 0)))
+  (alias export 0 "x" (type (;1;)))
+  (import "x" (type (;2;) (eq 1)))
+  (alias export 0 "x" (type (;3;)))
+  (type (;4;)
+    (instance
+      (alias outer 1 3 (type (;0;)))
+      (export (;1;) "x" (type (eq 0)))
+      (type (;2;) (own 1))
+      (type (;3;) (func (result 2)))
+      (export (;0;) "my-func" (func (type 3)))
+    )
+  )
+  (import "foo:test-import/my-interface" (instance (;1;) (type 4)))
+  (type (;5;)
+    (component
+      (type (;0;)
+        (instance
+          (export (;0;) "x" (type (sub resource)))
+        )
+      )
+      (export (;0;) "foo:dependency/types" (instance (type 0)))
+    )
+  )
+  (import "unlocked-dep=<test:bar>" (component (;0;) (type 5)))
+  (instance (;2;) (instantiate 0))
+  (alias export 1 "my-func" (func (;0;)))
+  (alias export 2 "foo:dependency/types" (instance (;3;)))
+  (type (;6;)
+    (component
+      (type (;0;)
+        (instance
+          (export (;0;) "x" (type (sub resource)))
+        )
+      )
+      (import "foo:dependency/types" (instance (;0;) (type 0)))
+      (alias outer 1 3 (type (;1;)))
+      (import "x" (type (;2;) (eq 1)))
+      (type (;3;) (own 2))
+      (type (;4;) (func (result 3)))
+      (import "my-func" (func (;0;) (type 4)))
+    )
+  )
+  (import "unlocked-dep=<test:baz>" (component (;1;) (type 6)))
+  (instance (;4;) (instantiate 1
+      (with "foo:dependency/types" (instance 3))
+      (with "my-func" (func 0))
+      (with "x" (type 2))
+    )
+  )
+)

crates/wac-graph/tests/graphs/implicit-resource-import/graph.json

@@ -0,0 +1,54 @@
+{
+    "packages": [
+        {
+            "name": "foo:import",
+            "path": "import"
+        },
+        {
+            "name": "test:bar",
+            "path": "bar"
+        },
+        {
+            "name": "test:baz",
+            "path": "baz"
+        }
+    ],
+    "nodes": [
+        {
+            "type": "import",
+            "name": "foo:test-import/my-interface",
+            "package": 0,
+            "export": "foo:test-import/my-interface"
+        },
+        {
+            "type": "instantiation",
+            "package": 1
+        },
+        {
+            "type": "alias",
+            "source": 0,
+            "export": "my-func"
+        },
+        {
+            "type": "alias",
+            "source": 1,
+            "export": "foo:dependency/types"
+        },
+        {
+            "type": "instantiation",
+            "package": 2
+        }
+    ],
+    "arguments": [
+        {
+            "source": 2,
+            "target": 4,
+            "name": "my-func"
+        },
+        {
+            "source": 3,
+            "target": 4,
+            "name": "foo:dependency/types"
+        }
+    ]
+}

crates/wac-graph/tests/graphs/implicit-resource-import/import/deps/types.wit

@@ -0,0 +1,9 @@
+package foo:dependency;
+
+interface types {
+    resource x;
+}
+
+world w {
+    export types;
+}

crates/wac-graph/tests/graphs/implicit-resource-import/import/world.wit

@@ -0,0 +1,10 @@
+package foo:test-import;
+
+world w {
+    export my-interface;
+}
+
+interface my-interface{
+    use foo:dependency/types.{x};
+    my-func: func() -> x;
+}

crates/wac-types/src/aggregator.rs

@@ -23,7 +23,7 @@ pub struct TypeAggregator {
     /// The aggregated types collection.
     types: Types,
     /// The map from import name to aggregated item kind.
-    names: IndexMap<String, ItemKind>,
+    imports: IndexMap<String, ItemKind>,
     /// A map from foreign type to remapped local type.
     remapped: HashMap<Type, Type>,
     /// A map of interface names to remapped interface id.
@@ -41,9 +41,9 @@ impl TypeAggregator {
         &self.types
     }
 
-    /// Iterates the named types in the aggregator.
-    pub fn iter(&self) -> impl Iterator<Item = (&str, ItemKind)> {
-        self.names.iter().map(|(n, k)| (n.as_str(), *k))
+    /// Iterates the imported types in the aggregator.
+    pub fn imports(&self) -> impl Iterator<Item = (&str, ItemKind)> {
+        self.imports.iter().map(|(n, k)| (n.as_str(), *k))
     }
 
     /// Aggregates a item kind from a specified type collection using the given
@@ -60,13 +60,13 @@ impl TypeAggregator {
         // First check if this import has already been remapped into our
         // types collection.
         // If it has already been remapped, do a merge; otherwise, remap it.
-        if let Some(existing) = self.names.get(name).copied() {
+        if let Some(existing) = self.imports.get(name).copied() {
             self.merge_item_kind(existing, types, kind, checker)?;
             return Ok(self);
         }
 
-        let ty = self.remap_item_kind(types, kind, checker)?;
-        let prev = self.names.insert(name.to_string(), ty);
+        let remapped = self.remap_item_kind(types, kind, checker)?;
+        let prev = self.imports.insert(name.to_string(), remapped);
         assert!(prev.is_none());
         Ok(self)
     }
@@ -549,11 +549,27 @@ impl TypeAggregator {
             alias: resource
                 .alias
                 .map(|a| -> Result<_> {
+                    let owner = a
+                        .owner
+                        .map(|id| {
+                            // There's no need to merge the interface here as
+                            // merging is done as part of the interface remapping
+                            self.remap_interface(types, id, checker)
+                        })
+                        .transpose()?;
+                    // If there is an owning interface, ensure it is imported
+                    if let Some(owner) = owner {
+                        let name = self.types()[owner]
+                            .id
+                            .as_deref()
+                            .expect("interface has no id");
+                        if !self.imports.contains_key(name) {
+                            self.imports
+                                .insert(name.to_owned(), ItemKind::Instance(owner));
+                        }
+                    }
                     Ok(ResourceAlias {
-                        owner: a
-                            .owner
-                            .map(|id| self.remap_interface(types, id, checker))
-                            .transpose()?,
+                        owner,
                         source: self.remap_resource(types, a.source, checker)?,
                     })
                 })