-
Notifications
You must be signed in to change notification settings - Fork 506
[js-api] Fix result types in string builtins #2054
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The string builtins that return strings should return non-nullable `(ref extern)`` rather than nullable `externref`. This change reflects current implementation reality in both Chrome and FireFox.
Ms2ger
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems fine. I'd like @sideshowbarker's review of the echidna failures before merging, though
jakobkummerow
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
FWIW, (ref extern) is what the proposal's Overview has always said. Dunno why the final spec ended up diverging from that; I'm guessing that wasn't intentional.
|
cc @rmahdav |
Our charter has expired, so we no longer have perms to (auto)publish to TR space. I’ve sent a request for an extension of the charter. If/when that gets OKed, the echidna failures should go away. |
eqrion
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
No more echidna failures. We got our charter extension — so it’s back again to working now. All green 🍀 |
|
Thanks! |
The string builtins that return strings should return non-nullable
(ref extern)rather than nullableexternref. This change reflects current implementation reality in both Chrome and FireFox.