Fix error message for ref.is_null #2471
Conversation
|
cc @Janrupf |
test/spec/ref_is_null.txt
Outdated
| @@ -4,9 +4,10 @@ | |||
| init(externref:1) => | |||
| deinit() => | |||
| out/test/spec/ref_is_null.wast:52: assert_invalid passed: | |||
| out/test/spec/ref_is_null/ref_is_null.1.wasm:000001b: error: type mismatch in ref.is_null, expected reference but got [i32] | |||
| out/test/spec/ref_is_null/ref_is_null.1.wasm:000001b: error: type mismatch in ref.is_null, expected [(ref -1)] but got [i32] | |||
There was a problem hiding this comment.
What does [(ref -1)] mean here?
Not a big deal but isn't the old error message a little more readable?
There was a problem hiding this comment.
It's a "reference" of an invalid type (kInvalidIndex). We don't know which proposal defines Type::Reference (typed references?) and we're misusing it a little (or arguably a lot), but it should be fairly safe since it's just for the error message.
There was a problem hiding this comment.
How was it the reference to printed before? Can we find a way to keep it that way? expected [(ref -1)] is not very clear is it?
There was a problem hiding this comment.
The old code printed its own message, without going through PopAndCheck1Type/PrintStackIfFailed like everything else. We believe the consistency is important.
We can try to turn it into expected [reference], but it'll require some care.
Same issue as #2471 but for `call_ref`. We don't believe there's a prior issue for this.
Fixes #2453 in a bit of a silly way. (Conveniently, we already have tests for this, but nobody noticed they were broken.)