Closed Bug 1314037 Opened 8 years ago Closed 8 years ago

Support extended Unicode sequences in identifiers

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

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)

Support the \u{...} escape sequences for identifiers.
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)
Attachment #8806003 - Flags: review?(arai.unmht)
Attachment #8806002 - Flags: review?(arai.unmht) → review+
Attachment #8806003 - Flags: review?(arai.unmht) → review+
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
(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.
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)
(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!
Keywords: checkin-needed
Blocks: 917436
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
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
Depends on: CVE-2017-7813
No longer depends on: CVE-2017-7813
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: