[test] Add boundary cast tests for ToWebAssemblyValue #439
Conversation
|
|
||
| test(() => { | ||
| for (const nullfunc of [exports.noneArg, exports.nofuncArg, exports.noexternArg]) { | ||
| assert_throws_js(TypeError, () => nullfunc(exports.makeStruct())); |
There was a problem hiding this comment.
I added a few more test cases as I realized tests for bottom types were missing.
Here, V8 (commit 27dd6178089d11cabb23c4cd5e9902440be7e5f5) does not throw in the noextern case of the test while Spidermonkey and JSC do. This test is checking all of the bottom types and the mismatch is just on the extern bottom.
For some context, exports.noexternArg is a (ref noextern) -> [] function and is being called in this test case as nullfunc on various arguments. This should be triggering a cast to noextern during the call.
I believe casting to noextern should fail for any of the arguments but null (if it's a nullable ref, as in the loop below this one).
There was a problem hiding this comment.
@jakobkummerow could the above be a V8 bug?
There was a problem hiding this comment.
I believe casting to noextern should fail for any of the arguments but null (if it's a nullable ref, as in the loop below this one).
I think you're right, but the spec is wrong about that: on the extern ref path, it skips the subtype check, which is wrong. Please see #442 for a fix.
|
I'm going to be out of office for two weeks but if anyone has feedback about this or any of the other JS API PRs I can take a look and address them once I'm back. |
|
@rossberg I think the latest commit should address your comments. There are merge conflicts but I can rebase to fix once the PR looks ok. |
Mostly tests the ToWebAssemblyValue metafunction
takikawa
force-pushed
the
gc-js-api-casts-test
branch
from
bd4768d to
4e87dd2
Compare
|
This is now rebased and should merge cleanly. |
This PR adds JS API tests for exercising
ToWebAssemblyValuewith all the categories of GC types. It adds on top of the previous two JS API tests PRs, so it should be merged after them (just the last commit is new).With these three tests, I think we will have good test coverage of the JS API, but if anyone sees any missing cases that should be covered that would be helpful to know.
The tests pass on JSC (w/ some unlanded patches), V8, and SpiderMonkey (w/ final opcodes) with one exception that I'll comment below.