Update call_indirect text syntax to match spec update#1281
Conversation
Function type gets its own element rather than being a part of the call_indirect (see WebAssembly/spec#599)
|
lgtm but looks like the spec tests we run need to be updated too. |
|
This doesn't seem to support the inline format: |
|
Ah you're right; I was mostly trying to resolve the backwards
incompatibility that was causing waterfall breakage. Maybe I'll add the new
functionality in a separate change.
…On Fri, Nov 10, 2017, 7:22 PM Ben Smith ***@***.***> wrote:
This doesn't seem to support the inline format: (call_indirect (param
i32) (result i32) ...) etc.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1281 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABEiKDBPKdk6bruVTyIDd86Q1PDM_CqPks5s1RMCgaJpZM4QaTot>
.
|
|
@kripken Have our spec tests really not been updated since 2016? I was going to try to update them here but if it's been that long then it probably makes more sense to do that in a separate change because there will certainly be a lot of unrelated stuff. The script I ran to update the tests for this PR also updated our copy of the spec tests, so I could check that in for now to get the tools all compatible again. But we should probably do another pull from the spec repo to get all the new tests (and also of course we've made changes to the IR since then so it's probably worth revisiting anyway). |
|
@dschuff: Well, we sort of gave up on updating them since wasm became a stack machine and things diverged. We definitely can't support a bunch of things in the upstream tests. And we don't claim to support the upstream text format at this point. But the spec tests have been updated with various specific changes, current commit looks like it's from August 27 2017. So we might want to do that for this change. Alternatively, do we need to update this, since we still won't be able to support the full upstream text format anyhow? |
|
Since we do support a subset of the upstream text format, I think it makes sense to make this change, so that we continue to support the same subset (instead of just an outdated subset). And probably also to make the change that @binji mentioned, since that's probably a small change too. So I updated the spec tests to their new-format equivalent. |
) Function type gets its own element rather than being a part of the call_indirect (see WebAssembly/spec#599) EOSIO note: Only cherry-pick the .cpp changes for this. Trying to pull up the tests is not feasible. We don't use the tests anyways.
Update call_indirect text syntax to match spec update (WebAssembly#1281)
3 participants
Function type gets its own element rather than being a part of the call_indirect
(see WebAssembly/spec#599)