Remove the type-driven ability for duplicates in a Linker#2789
Conversation
When `Linker` was first created it was attempted to be created with the ability to instantiate any wasm modules, including those with duplicate import strings of different types. In an effort to support this a `Linker` supports defining the same names twice so long as they're defined with differently-typed values. This ended up causing wast testsuite failures module linking is enabled, however, because the wrong error message is returned. While it would be possible to fix this there's already the possibility for confusing error messages today due to the `Linker` trying to take on this type-level complexity. In a way this is yet-another type checker for wasm imports, but sort of a bad one because it only supports things like globals/functions, and otherwise you can only define one `Memory`, for example, with a particular name. This commit completely removes this feature from `Linker` to simplify the implementation and make error messages more straightforward. This means that any error message coming from a `Linker` is purely "this thing wasn't defined" rather than a hybrid of "maybe the types didn't match?". I think this also better aligns with the direction that we see conventional wasm modules going which is that duplicate imports are not ever present.
Subscribe to Label Actioncc @peterhuene DetailsThis issue or pull request has been labeled: "wasmtime:api"Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
|
This looks reasonable to me. My understanding of the duplicate-imports discussion is that there are core-wasm use cases where linking based on the name identifiers alone isn't sufficient, however it also seems like core-wasm types and signatures also aren't sufficient for those use cases, because they're too low-level in general, so they'll need to do something else anyway. |
|
Oh that reminds me of another reason to do this. Currently |
sunfishcode
left a comment
There was a problem hiding this comment.
Good point. Another consideration here is that exports that differ only in type represent interfaces which can't be implemented by wasm modules (without custom linking), which makes them difficult to virtualize.
When
Linkerwas first created it was attempted to be created with theability to instantiate any wasm modules, including those with duplicate
import strings of different types. In an effort to support this a
Linkersupports defining the same names twice so long as they'redefined with differently-typed values.
This ended up causing wast testsuite failures module linking is enabled,
however, because the wrong error message is returned. While it would be
possible to fix this there's already the possibility for confusing error
messages today due to the
Linkertrying to take on this type-levelcomplexity. In a way this is yet-another type checker for wasm imports,
but sort of a bad one because it only supports things like
globals/functions, and otherwise you can only define one
Memory, forexample, with a particular name.
This commit completely removes this feature from
Linkerto simplifythe implementation and make error messages more straightforward. This
means that any error message coming from a
Linkeris purely "thisthing wasn't defined" rather than a hybrid of "maybe the types didn't
match?". I think this also better aligns with the direction that we see
conventional wasm modules going which is that duplicate imports are not
ever present.