Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: panic when adding an implicit import with a use #55

Open
peterhuene opened this issue Feb 9, 2024 · 2 comments
Open

Bug: panic when adding an implicit import with a use #55

peterhuene opened this issue Feb 9, 2024 · 2 comments
Assignees

Comments

@peterhuene
Copy link
Member

peterhuene commented Feb 9, 2024

This issue was discovered by @salmans.

Let's say we have the following WIT:

package foo:bar;

interface a {
    type x = u32;
}

interface b {
    use a.{x};
    f: func() -> x;
}

world repro {
    import b;
    export run: func();
}

and we have a component named repro:comp1 that targets the foo:bar/repro world.

We can instantiate this component in WAC using implicit imports:

package repro:wac;

let i = new repro:comp1 { ... };

This encodes just fine because the repro:comp1 imports foo:bar/a before foo:bar/b, so an implicit import for foo:bar/a is added before the implicit import for foo:bar/b; when the implicit import foo:bar/b is added, foo:bar/a is already imported so it can properly remap some internal data structures for the dependency from foo:bar/b to foo:bar/a.

However, let's say we have a repro:comp2 component:

(component (;0;)
  (type (;0;) u32)
  (export (;1;) "x" (type 0))
)

And we now instantiate repro:comp1 like this:

package repro:wac;

let i = new repro:comp1 {
  a: new repro:comp2 { ... },
  ...
};

This results in a panic:

thread 'main' panicked at crates/wac-parser/src/resolution/ast.rs:2039:40:
IndexMap: key not found

This is because foo:bar/a is no longer being implicitly imported, so the expectation that foo:bar/b can find it as an implicit import is violated.

A naive fix would be to recurse on the dependencies of an implicit import to ensure they are also implicitly imported so that the data structures can be remapped, but that's not exactly what we want here.

What we want is for the instance of repro:comp2 to also satisfy the dependency that foo:bar/b has on foo:bar/a, so that the exported type is equivilant between the import of foo:bar/a and foo:bar/b for repro:comp1; this is more relevant when the type in question is a resource and not a u32, for example.

@peterhuene
Copy link
Member Author

peterhuene commented Feb 9, 2024

I think this is what the expected WAT would look like for the implicit import of foo:bar/b:

(alias export 0 "x" (type (;0;))) ;; assume instance 0 is that of `repro:comp2`
(type (;1;)
  (instance
    (alias outer 1 0 (type (;0;)))
    (export (;1;) "x" (type (eq 0)))
    (type (;2;) (own 1))
    (type (;3;) (func (result 2)))
    (export (;0;) "f" (func (type 3)))
  )
)
(import "component:testing/b" (instance (;1;) (type 1)))

My question would then be: assuming we were actually using a resource here and not a u32, could this import be satisfied in the future given that instance 0 is internal to the composition? I'm still not 100% confident in my understanding of how realized resource type equivalence works.

@lukewagner
Copy link

Great question, this is a really interesting case. It's useful to imagine that x is a resource type since it makes the constraints more clear-cut. So, in your WAC:

let i = new repro:comp1 {
  a: new repro:comp2 { ... },
  ...
};

comp2 may be defining and implementing the resource type x and that resource-implementation of x is specific to that particular instance of comp2 (b/c the resources may be implemented in that particular instance's linear memory). Thus, if we were to attempt to implicitly propagate the import of b.f to the parent component, we'd have no way to describe the required type in the type of the imports of the parent component (intuitively, b/c the instance of comp2 is only created after the imports of the parent have been satisfied, and so the type doesn't exist soon enough).

What I'd suggest is that ... gives an error pointing to the un-satisfiable import of foo:bar/b (and, if you're feeling generous, which used resource type could not be transitively implicitly imported b/c it was already satisfied).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants