Conversation
Address feedback from #72, including by using (exact, deftype) pairs instead of full heaptypes where possible in the interpreter. Update the overview accordingly. Also fix #74 by introducing a new externtype binary encoding for exact function imports rather than using an s33 in the existing function import variant.
|
Happy to update the binary encoding here pending the result of the discussion in #74. |
interpreter/valid/valid.ml
Outdated
| | MemoryX x -> ExternMemoryT (memory c x) | ||
| | TableX x -> ExternTableT (table c x) | ||
| | FuncX x -> ExternFuncT (func c x) | ||
| | FuncX x -> ExternFuncT (Inexact, Def (snd (func c x))) |
| 0x05 x:typeidx => func exact x | ||
| ``` | ||
|
|
||
| Note that we do not add support for exactly exported functions. |
There was a problem hiding this comment.
I'm a bit lost, what is the point of exact imports if there are no exact exports?
That said, an export wouldn't require the exact attribute, that's inherited from what's exported. (Another reason why the encoding of the attribute should be separate from the im/export kind opcode range.)
There was a problem hiding this comment.
The static types of exports don't seem to matter too much because instantiation checks the dynamic types of imports rather than their static types. I expect this to start mattering more once we have some form of static module linking, for example in the component model.
Assuming we do want to express exact exports, though, the choice about whether to use the exact attribute on exports is not obvious. As you say, we can always just export with the precise internal type, exact or not, without the exact attribute on the export. On the other hand, requiring the exact attribute for exact exports would allow for "hiding" the exactness of an exported function by choosing not to use the exact attribute on the export. That has the potential benefit that refining the type of the function export as the module evolves would not break linking, so perhaps @lukewagner has a strong opinion that we should support this? OTOH, I'm not sure how useful this would be in practice since function subtyping is not typically used outside of GC languages.
There was a problem hiding this comment.
The implementation in this PR is conservative in that it always exports functions with inexact types, but I'm not sure how to test this given that it doesn't make a difference for instantiation.
There was a problem hiding this comment.
First of all, if we wanted to enable some form of type hiding at exports, then the right design would be to allow giving a full function type. That would allow exporting with any supertype of choice. Exactness is just one possible aspect of this.
But none of this makes any sense as long as imports are checked against the original dynamic type anyway — which is not something we are able to change. So "type hiding" for exports doesn't actually hide anything in any meaningful or useful manner. Even the seemingly hiding "static" export type, including exactness, can be "cast" back to the original type by funnelling the function through an auxiliary module that imports and reexports it with its original type.
All this makes the idea of such after-the-fact type hiding on exports entirely broken and pointless.
A correct implementation of such a feature would actually have to modify the dynamic type of the function. But that can only be done by wrapping it into a new function object. Which in turn is something a module can already do manually, by exporting a wrapper function. In either case, hiding exactness would still be broken.
The bottom line is that after-the-fact type hiding on exports, for exactness or otherwise, does not work in Wasm. If somebody needs to restrict the type of exports, then export the right thing. If we want to avoid exactness foot guns, then the only solution is to axe exact imports. I am very much in favour of that, until we understand the problem and the benefit better. It would also simplify the proposal and make it more conservative.
There was a problem hiding this comment.
I think we're in violent agreement about the meaninglessness of the static export type :)
Not doing any kind of type hiding seems fine to me.
I will update this so that the export type is the same as the internal type. This does not require the exact bit to be set on the export, so I will continue disallowing its use there.
Does that sound good?
There was a problem hiding this comment.
Sounds good to me. I'd still vote for removing exact imports as well, as I don't think they carry their weight and there is no in-language use-case for them. But happy to discuss separately.
interpreter/binary/decode.ml
Outdated
| | 0x02 -> ExternMemoryT (memorytype s) | ||
| | 0x03 -> ExternGlobalT (globaltype s) | ||
| | 0x04 -> ExternTagT (tagtype s) | ||
| | 0x05 -> ExternFuncT (Exact, typeuse idx s) |
There was a problem hiding this comment.
Yes, I think this should change to 0x40.
There was a problem hiding this comment.
0x20 to allow for reinterpreting this as a signed LEB in the future, right?
2 participants
Address feedback from #72, including by using (exact, deftype) pairs
instead of full heaptypes where possible in the interpreter. Update the
overview accordingly.
Also fix #74 by introducing a new externtype binary encoding for exact
function imports rather than using an s33 in the existing function
import variant.