Skip to content

Conversation

@SoniEx2
Copy link
Collaborator

@SoniEx2 SoniEx2 commented Sep 23, 2024

Fixes #2453 in a bit of a silly way. (Conveniently, we already have tests for this, but nobody noticed they were broken.)

@SoniEx2
Copy link
Collaborator Author

SoniEx2 commented Sep 23, 2024

cc @Janrupf

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]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does [(ref -1)] mean here?

Not a big deal but isn't the old error message a little more readable?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@sbc100 sbc100 enabled auto-merge (squash) September 23, 2024 20:51
@sbc100 sbc100 merged commit 3fd8c70 into WebAssembly:main Sep 23, 2024
sbc100 pushed a commit that referenced this pull request Sep 24, 2024
Same issue as #2471 but for `call_ref`.

We don't believe there's a prior issue for this.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Module fails to compile without a proper error message

2 participants