[interpreter] Abstract script runner over engine #2030
Conversation
conrad-watt
left a comment
conrad-watt
left a comment
There was a problem hiding this comment.
Generally looks great! One comment below.
Also, a higher level question - where can I find the code where the Runner.Make functor is getting called with the existing reference interpreter definitions as the "engine"? I'm assuming this must be somewhere, to preserve the existing functionality.
| sig | ||
| val register_instance : Ast.name -> Engine.moduleinst -> unit | ||
| val register_virtual : Ast.name -> | ||
| (Ast.name -> Types.externtype -> Engine.externinst option) -> |
There was a problem hiding this comment.
I think my main question is about the use of Types.externtype here. This appears to require that the plugged-in engine needs to provide a way to convert all of its types into the reference interpreter's concept of types, which might be burdensome for GC types. Can you give me some more context on why this is the right design as opposed to having the engine bring its own concept of external type and type matching as part of the module signature?
There was a problem hiding this comment.
Ah, note that (1) this is not something the engine needs to implement, but a host who wants to register host modules like spectest, and (2) this is in a callback, so a host registering a virtual host module receives the type from the runner and can just choose to ignore it.
In general, the type is necessary, since nothing in the Wasm spec forbids hosts from deciding to resolve imports based on not just their name but also their type — they could e.g. implement overloading for imports that way. Not that I'd recommend that...
There was a problem hiding this comment.
a host registering a virtual host module receives the type from the runner and can just choose to ignore it.
Doesn't this still mean that, as part of the "coding work" that needs to be done to integrate a new engine with the runner functor, someone needs to write a function that converts the engine's representation of externtype to the host's representation (which is Types.externtype)? And am I correct in understanding that this conversion might need a "deep" traversal of the relevant GC types?
If I've got the above correct, I'm thinking about an alternative design where the engine exposes its externtype representation and a type matching function as part of its interface. Would this be sufficient for the runner to implement the import resolving logic it wants?
There was a problem hiding this comment.
Doesn't this still mean that, as part of the "coding work" that needs to be done to integrate a new engine with the runner functor, someone needs to write a function that converts the engine's representation of
externtypeto the host's representation (which isTypes.externtype)? And am I correct in understanding that this conversion might need a "deep" traversal of the relevant GC types?
Yes, but I'm afraid it has to do that anyway. For example, (reference) values contain types as well.
The runner inspects and sometimes constructs both value and type representations in many places. So there has to be some interface.
The only alternative to converting both to and from a "common" representation would be some kind of extensive reflection/introspection interface that the engine has to implement. But that would essentially have to do the same work, just with every constructor abstracted into a pair of construct/deconstruct interface functions. That would not be simpler for either side.
rossberg
left a comment
rossberg
left a comment
There was a problem hiding this comment.
Yes, the functor application is in run.ml, which thereby is a drop-in replacement for the old run.ml.
| sig | ||
| val register_instance : Ast.name -> Engine.moduleinst -> unit | ||
| val register_virtual : Ast.name -> | ||
| (Ast.name -> Types.externtype -> Engine.externinst option) -> |
There was a problem hiding this comment.
Ah, note that (1) this is not something the engine needs to implement, but a host who wants to register host modules like spectest, and (2) this is in a callback, so a host registering a virtual host module receives the type from the runner and can just choose to ignore it.
In general, the type is necessary, since nothing in the Wasm spec forbids hosts from deciding to resolve imports based on not just their name but also their type — they could e.g. implement overloading for imports that way. Not that I'd recommend that...
Functorise script runner so that other backend engines can be plugged in.
Bummer, I purposefully git-moved the old run.ml to runner.ml, but because I also introduced a new run.ml, git diff fails to recognise this and doesn't show the diff for runner.ml against old run.ml.