-
-
Notifications
You must be signed in to change notification settings - Fork 413
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
[Merged by Bors] - First prototype for new JsString
using UTF-16
#1659
Conversation
In addition, the utf16_literal crate should be useful to convert runtime strings from |
Test262 conformance changesVM implementation
Fixed tests (30):
Broken tests (8):
|
Codecov Report
@@ Coverage Diff @@
## main #1659 +/- ##
==========================================
- Coverage 40.93% 40.82% -0.11%
==========================================
Files 238 240 +2
Lines 22509 22743 +234
==========================================
+ Hits 9214 9285 +71
- Misses 13295 13458 +163
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Benchmark for 4aee7ceClick to view benchmark
|
Hi @jedel1043! Do you think you would be able to get back to this? This would be very nice to have :) |
I'm still working on it locally :) |
Benchmark for f902625Click to view benchmark
|
Benchmark for c3d66b0Click to view benchmark
|
Time to see how many tests break... 😅 |
69b3ce9
to
cdb7d38
Compare
Well, I expected to see more failed tests tbh, I'll probably fix those today or tomorrow. It could also be that we cannot parse string literals with UTF-16 codepoints; the interner automatically converts unpaired surrogates to On another note, the interner doesn't support UTF-16 strings, so we will need to contribute to |
Yes, but they might not want to add this feature, so we might end up needing to write our own interner. Let's see if we can open them an issue. |
a89d440
to
70eca88
Compare
Benchmark for c115fd5Click to view benchmark
|
Benchmark for 14d3cf4Click to view benchmark
|
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.
Looks very good :) It's a pitty the performance decrease, but we can merge this now and then spend some time enhancing the performance.
Check my comments and see if they make sense, before merging
Benchmark for 5dd62b2Click to view benchmark
|
bors r+ |
I think it's time to address the elephant in the room. This Pull Request will (hopefully!) solve part of #736. This is a complete rewrite of `JsString`, but instead of storing `u8` bytes it stores `u16` words. The `encode!` macro (renamed to `utf16!` for simplicity) from the `const-utf16` crate allows us to create UTF-16 encoded arrays at compilation time. `JsString` implements `Deref<Target=[u16]>` to unlock the slice methods and possibly make some manipulations easier. However, we would need to create our own library of utilities for `JsString`.
Pull request successfully merged into main. Build succeeded: |
JsString
using UTF-16 JsString
using UTF-16
I think it's time to address the elephant in the room.
This Pull Request will (hopefully!) solve part of #736.
This is a complete rewrite of
JsString
, but instead of storingu8
bytes it storesu16
words. Theencode!
macro (renamed toutf16!
for simplicity) from theconst-utf16
crate allows us to create UTF-16 encoded arrays at compilation time.JsString
implementsDeref<Target=[u16]>
to unlock the slice methods and possibly make some manipulations easier. However, we would need to create our own library of utilities forJsString
.