[interpreter] Support nested invoke/get in test scripts #1590
Conversation
rossberg
force-pushed
the
nested-actions
branch
from
dc9c107
to
97a9473
Compare
|
Okay, JS conversion is completely broken right now. To support multiple imports in Wasm wrappers, the implementation needs a complete refactor. I'll look into it, but might no find time for a little while. |
| (assert_return (get $m2 "g") (i32.const 42)) | ||
|
|
||
| (set "g" (i32.const 43)) | ||
| (assert_return (set "g" (i32.const 43))) |
There was a problem hiding this comment.
So set is not like a tee, it returns empty?
There was a problem hiding this comment.
Right, it's like global.set.
| (assert_return (invoke $m1 "inc" (i32.const 2)) (i32.const 3)) | ||
| (assert_return (invoke $m1 "inc" (get $m1 "g")) (i32.const 42)) | ||
| (assert_return (invoke $m1 "inc" (get $m2 "g")) (i32.const 46)) | ||
| (assert_return (invoke $m1 "inc" (invoke $m1 "inc" (get "g"))) (i32.const 47)) |
There was a problem hiding this comment.
I'm wondering if intentionally limiting the grammar to not allow nested invokes (or even set) would be good. One the one hand, it simplifies the test runner so it doesn't have to keep an internal stack, and also forces tests to flatten out their nested calls, saving their intermediate results in globals.
There was a problem hiding this comment.
Actually, I have often missed the ability to simply plug two function calls together for an assertion. Not being able to do that on script level requires contortions exactly like those from not being able to access a global. So it would be a pity to exclude it. It's just a trivial recursive expression evaluator after all, the changes to runners ought to be minimal (see run.ml).
The real complexity with this extension is in translating tests to JS. But that all has to do with the fact that synthesised Wasm modules for assertions now require an arbitrary number of imports, which isn't changed by limiting expression depth.
(Also, I ran into a couple of V8/node bugs, such as #1597, and worse, deterministic bus errors on my new Arm MacBook. :( )
|
I did a rewrite of most of the JS converter and it now can handle wrapper modules with an arbitrary number of imports. It also made the code more readable. |
|
@titzer, are you content with this? If so, can you approve the PR? |
Also adds
setaction to mutate globals. See script.wast for examples.@titzer, PTAL. Should fix #1568.
This was straightforward to implement, except for the JS conversion, which caused quite some headache because wrapper modules generated for actions on types inexpressible in JS are no longer closed now but need to take their arguments as imports. This is largely untested.