[Strings] Adopt new instruction binary encoding#5714
Conversation
And rename StringNewReplaceArray to StringNewLossyUTF8Array. Following changes to the stringref proposals (WebAssembly/stringref#46)
Match the V8 implementation (https://chromium-review.googlesource.com/c/v8/v8/+/3892695). See also WebAssembly/stringref#46.
|
Thanks for the PR! You say V8 already implemented this change - is it backwards compatible? AFAIK our state before this PR works in V8, and the linked V8 CL is quite old, so I'm surprised things have been working so far for people using Binaryen+V8... |
|
No, this changes is not backwards compatible. I investigated this because |
|
Interesting... maybe it happens to work out that way. |
kripken
left a comment
There was a problem hiding this comment.
Looks right to me. I'm not 100% sure we understand why the same binaries seem to work before and after this change (that is, with v8 and binaryen using incompatible versions) but it seems like this should only help. And if we see breakage after it lands we can back it out and investigate further.
See WebAssembly/stringref#46. This format is already adopted by V8: https://chromium-review.googlesource.com/c/v8/v8/+/3892695. The text format is left unchanged (see WebAssembly#5607 for a discussion on the subject). I have also added support for string.encode_lossy_utf8 and string.encode_lossy_utf8 array (by allowing the replace policy for Binaryen's string.encode_wtf8 instruction).
See WebAssembly/stringref#46.
This format is already adopted by V8: https://chromium-review.googlesource.com/c/v8/v8/+/3892695.
The text format is left unchanged (see #5607 for a discussion on the subject).
I have also added support for
string.encode_lossy_utf8andstring.encode_lossy_utf8 array(by allowing thereplacepolicy for Binaryen'sstring.encode_wtf8instruction).