Add warnings about UTF-16 vs UTF-8 strings #1416
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Pauan
suggested changes
Apr 2, 2019
alexcrichton
force-pushed
the
js-string-valid-utf16
branch
from
588a54c to
c8fdfac
Compare
Pauan
suggested changes
Apr 3, 2019
alexcrichton
force-pushed
the
js-string-valid-utf16
branch
from
c8fdfac to
d212fac
Compare
Pauan
suggested changes
Apr 4, 2019
alexcrichton
force-pushed
the
js-string-valid-utf16
branch
from
d212fac to
2452d25
Compare
|
👍 Thanks for the thorough review! |
Pauan
approved these changes
Apr 4, 2019
This commit aims to address rustwasm#1348 via a number of strategies: * Documentation is updated to warn about UTF-16 vs UTF-8 problems between JS and Rust. Notably documenting that `as_string` and handling of arguments is lossy when there are lone surrogates. * A `JsString::is_valid_utf16` method was added to test whether `as_string` is lossless or not. The intention is that most default behavior of `wasm-bindgen` will remain, but where necessary bindings will use `JsString` instead of `str`/`String` and will manually check for `is_valid_utf16` as necessary. It's also hypothesized that this is relatively rare and not too performance critical, so an optimized intrinsic for `is_valid_utf16` is not yet provided. Closes rustwasm#1348
alexcrichton
force-pushed
the
js-string-valid-utf16
branch
from
2452d25 to
44738e0
Compare
|
Thanks @Pauan! |
alexcrichton
added a commit
that referenced
this pull request
Apr 8, 2019
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit aims to address #1348 via a number of strategies:
Documentation is updated to warn about UTF-16 vs UTF-8 problems
between JS and Rust. Notably documenting that
as_stringand handlingof arguments is lossy when there are lone surrogates.
A
JsString::is_valid_utf16method was added to test whetheras_stringis lossless or not.The intention is that most default behavior of
wasm-bindgenwillremain, but where necessary bindings will use
JsStringinstead ofstr/Stringand will manually check foris_valid_utf16asnecessary. It's also hypothesized that this is relatively rare and not
too performance critical, so an optimized intrinsic for
is_valid_utf16is not yet provided.
Closes #1348