Closed
Bug 1314037
Opened 8 years ago
Closed 8 years ago
Support extended Unicode sequences in identifiers
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla52
Tracking | Status | |
---|---|---|
firefox52 | --- | fixed |
People
(Reporter: anba, Assigned: anba)
References
(Blocks 1 open bug)
Details
(Keywords: dev-doc-complete)
Attachments
(2 files)
33.51 KB,
patch
|
arai
:
review+
|
Details | Diff | Splinter Review |
13.49 KB,
patch
|
arai
:
review+
|
Details | Diff | Splinter Review |
Support the \u{...} escape sequences for identifiers.
Assignee | ||
Comment 1•8 years ago
|
||
We're using up to three different types to represent UTF-16 code points (uint32_t, int32_t and size_t) in different parts of the engine. Ideally, we should only use a single type. And uint32_t seems to be the best choice: - int32_t requires additionally checks for negative numbers - size_t is unnecessarily large on 64bit frontend/TokenStream changes: - Added explicit `unicode::` namespace qualifiers, because other parts seem to prefer this style when calling methods from vm/Unicode.h - Renamed some parameters to "codePoint" to make it explicit when a UTF-16 code point is used - Replaced manual UTF-16 encoding/decoding with methods from vm/Unicode.h irregexp/RegExpParser changes: - Replaced size_t with either `widechar` (for UTF-16 code points) or `char16_t` (for UTF-16 code units) jsstr changes: - Moved UTF-16 helper methods to vm/Unicode.h (I should have done this from the start :-/ ) - Also added explicit `unicode::` namespace qualifiers - Also removed manual UTF-16 encoding/decoding vm/Unicode changes: - Changed size_t to either uint32_t (UTF-16 code points) or char16_t (UTF-16 code units) - The UTF-16 encoding/decoding methods no longer use the "canonical form" (e.g. decoding with `(lead - LeadSurrogateMin) * 1024 + (trail - TrailSurrogateMin) + NonBMPMin`), but instead were updated to use the slightly more optimized forms (`(lead << 10) + trail + (NonBMPMin - (LeadSurrogateMin << 10) - TrailSurrogateMin)`). Based on the assembly this seems to save a sub/add instruction.
Attachment #8806002 -
Flags: review?(arai.unmht)
Assignee | ||
Comment 2•8 years ago
|
||
Attachment #8806003 -
Flags: review?(arai.unmht)
Updated•8 years ago
|
Attachment #8806002 -
Flags: review?(arai.unmht) → review+
Updated•8 years ago
|
Attachment #8806003 -
Flags: review?(arai.unmht) → review+
Updated•8 years ago
|
Keywords: dev-doc-needed
Assignee | ||
Comment 3•8 years ago
|
||
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a8133cc6144f27a0e4a4577b06bf16219fba384d
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/c9b3a1252363 Part 1: Use uniform types for UTF-16 code units and code points. r=arai https://hg.mozilla.org/integration/mozilla-inbound/rev/0223902c2353 Part 2: Support extended Unicode escape sequences in identifiers. r=arai
Keywords: checkin-needed
I had to back these out for win64 WPT failures like https://treeherder.mozilla.org/logviewer.html#?job_id=38559683&repo=mozilla-inbound https://hg.mozilla.org/integration/mozilla-inbound/rev/0cb96d5b5a57
Flags: needinfo?(andrebargull)
Comment 6•8 years ago
|
||
(In reply to Wes Kocher (:KWierso) from comment #5) > I had to back these out for win64 WPT failures like > https://treeherder.mozilla.org/logviewer.html#?job_id=38559683&repo=mozilla- > inbound > > https://hg.mozilla.org/integration/mozilla-inbound/rev/0cb96d5b5a57 Hi, I am also clarifying if these are still expected as explained in bug 1309527 comment 14.
Comment 7•8 years ago
|
||
The test case is actually disabled before bug 1309527: http://searchfox.org/mozilla-central/rev/f5c9e9a249637c9abd88754c8963ecb3838475cb/testing/web-platform/meta/IndexedDB/idbobjectstore-rename-store.html.ini#3 It's my fault not verifying it in windows 8 in bug 1309527. Sorry about that.
Flags: needinfo?(andrebargull)
Assignee | ||
Comment 8•8 years ago
|
||
(In reply to Bevis Tseng[:bevistseng][:btseng] from comment #7) > The test case is actually disabled before bug 1309527: > http://searchfox.org/mozilla-central/rev/ > f5c9e9a249637c9abd88754c8963ecb3838475cb/testing/web-platform/meta/IndexedDB/ > idbobjectstore-rename-store.html.ini#3 > > It's my fault not verifying it in windows 8 in bug 1309527. > Sorry about that. Ah, I see. Thanks for the heads up!
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/80bedfc21ee6 Part 1: Use uniform types for UTF-16 code units and code points. r=arai https://hg.mozilla.org/integration/mozilla-inbound/rev/900466e640ca Part 2: Support extended Unicode escape sequences in identifiers. r=arai
Keywords: checkin-needed
Comment 10•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/80bedfc21ee6 https://hg.mozilla.org/mozilla-central/rev/900466e640ca
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Comment 11•7 years ago
|
||
https://developer.mozilla.org/en-US/Firefox/Releases/52#JavaScript
Keywords: dev-doc-needed → dev-doc-complete
Updated•7 years ago
|
Depends on: CVE-2017-7813
Updated•7 years ago
|
No longer depends on: CVE-2017-7813
You need to log in
before you can comment on or make changes to this bug.
Description
•