Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(27)

Issue 640193002: Allow identifier code points from supplementary multilingual planes. (Closed)

Created:
6 years, 2 months ago by Yang
Modified:
5 years, 10 months ago
CC:
v8-dev
Project:
v8
Visibility:
Public.

Description

Allow identifier code points from supplementary multilingual planes. ES5.1 section 6 ("Source Text"): "Throughout the rest of this document, the phrase โ€œcode unitโ€ and the word โ€œcharacterโ€ will be used to refer to a 16-bit unsigned value used to represent a single 16-bit unit of text." This changed in ES6 draft section 10.1 ("Source Text"): "The ECMAScript code is expressed using Unicode, version 5.1 or later. ECMAScript source text is a sequence of code points. All Unicode code point values from U+0000 to U+10FFFF, including surrogate code points, may occur in source text where permitted by the ECMAScript grammars." This patch is to reflect this spec change. BUG=v8:3617 LOG=Y R=jochen@chromium.org Committed: https://code.google.com/p/v8/source/detail?r=24510

Patch Set 1 #

Patch Set 2 : add missing file #

Total comments: 1

Patch Set 3 : parser fix #

Total comments: 4

Patch Set 4 : #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+165 lines, -11 lines) Patch
M BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M src/char-predicates.h View 3 chunks +15 lines, -1 line 0 comments Download
A src/char-predicates.cc View 1 1 chunk +42 lines, -0 lines 0 comments Download
M src/scanner.h View 1 2 2 chunks +29 lines, -5 lines 1 comment Download
A test/intl/general/smp-identifier.js View 1 2 3 1 chunk +43 lines, -0 lines 0 comments Download
A + test/mjsunit/parse-surrogates.js View 1 2 1 chunk +3 lines, -5 lines 0 comments Download
M test/unittests/unicode/unicode-predicates-unittest.cc View 1 chunk +31 lines, -0 lines 0 comments Download
M tools/gyp/v8.gyp View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 19 (2 generated)
Yang
6 years, 2 months ago (2014-10-09 08:35:55 UTC) #2
jochen (gone - plz use gerrit)
lgtm
6 years, 2 months ago (2014-10-09 09:24:47 UTC) #3
mathias
How about adding some tests to test/mjsunit/var.js? E.g. assertDoesNotThrow("var ๐ผ;"); // U+1043C DESERET SMALL LETTER ...
6 years, 2 months ago (2014-10-09 10:40:16 UTC) #4
mathias
On 2014/10/09 10:40:16, mathias wrote: > How about adding some tests to test/mjsunit/var.js? E.g. > ...
6 years, 2 months ago (2014-10-09 11:53:57 UTC) #5
Yang
On 2014/10/09 11:53:57, mathias wrote: > On 2014/10/09 10:40:16, mathias wrote: > > How about ...
6 years, 2 months ago (2014-10-09 11:54:51 UTC) #6
Yang
On 2014/10/09 11:54:51, Yang wrote: > On 2014/10/09 11:53:57, mathias wrote: > > On 2014/10/09 ...
6 years, 2 months ago (2014-10-09 11:55:51 UTC) #7
Yang
On 2014/10/09 11:55:51, Yang wrote: > On 2014/10/09 11:54:51, Yang wrote: > > On 2014/10/09 ...
6 years, 2 months ago (2014-10-09 12:02:31 UTC) #8
mathias
On 2014/10/09 11:55:51, Yang wrote: > On 2014/10/09 11:54:51, Yang wrote: > > On 2014/10/09 ...
6 years, 2 months ago (2014-10-09 12:03:02 UTC) #9
Yang
On 2014/10/09 12:03:02, mathias wrote: > On 2014/10/09 11:55:51, Yang wrote: > > On 2014/10/09 ...
6 years, 2 months ago (2014-10-09 12:21:29 UTC) #10
Yang
On 2014/10/09 12:21:29, Yang wrote: > On 2014/10/09 12:03:02, mathias wrote: > > On 2014/10/09 ...
6 years, 2 months ago (2014-10-09 14:40:26 UTC) #11
mathias
https://codereview.chromium.org/640193002/diff/40001/test/intl/general/smp-identifier.js File test/intl/general/smp-identifier.js (right): https://codereview.chromium.org/640193002/diff/40001/test/intl/general/smp-identifier.js#newcode9 test/intl/general/smp-identifier.js:9: String.fromCharCode(c & 0x3FF | 0xDC00); Since `--harmony-strings` is set ...
6 years, 2 months ago (2014-10-09 14:45:42 UTC) #12
Yang
https://codereview.chromium.org/640193002/diff/40001/test/intl/general/smp-identifier.js File test/intl/general/smp-identifier.js (right): https://codereview.chromium.org/640193002/diff/40001/test/intl/general/smp-identifier.js#newcode9 test/intl/general/smp-identifier.js:9: String.fromCharCode(c & 0x3FF | 0xDC00); On 2014/10/09 14:45:42, mathias ...
6 years, 2 months ago (2014-10-09 14:50:18 UTC) #13
mathias
On 2014/10/09 14:50:18, Yang wrote: > https://codereview.chromium.org/640193002/diff/40001/test/mjsunit/parse-surrogates.js > File test/mjsunit/parse-surrogates.js (right): > > https://codereview.chromium.org/640193002/diff/40001/test/mjsunit/parse-surrogates.js#newcode7 > ...
6 years, 2 months ago (2014-10-09 14:52:18 UTC) #14
Yang
On 2014/10/09 14:52:18, mathias wrote: > On 2014/10/09 14:50:18, Yang wrote: > > > https://codereview.chromium.org/640193002/diff/40001/test/mjsunit/parse-surrogates.js ...
6 years, 2 months ago (2014-10-09 15:01:31 UTC) #15
mathias
On 2014/10/09 15:01:31, Yang wrote: > On 2014/10/09 14:52:18, mathias wrote: > > On 2014/10/09 ...
6 years, 2 months ago (2014-10-09 15:07:08 UTC) #16
Yang
Committed patchset #4 (id:60001) manually as 24510 (presubmit successful).
6 years, 2 months ago (2014-10-10 07:13:57 UTC) #17
marja
5 years, 10 months ago (2015-01-29 15:39:38 UTC) #19
Message was sent while issue was closed.
some efficiency whine

verwaest reports that the code load score without this branch is 5% better...

https://codereview.chromium.org/640193002/diff/60001/src/scanner.h
File src/scanner.h (right):

https://codereview.chromium.org/640193002/diff/60001/src/scanner.h#newcode532
src/scanner.h:532: if (unibrow::Utf16::IsLeadSurrogate(c0_)) {
Looks like code load doesn't like adding this branch to Advance. For more info,
-> verwaest@.

Powered by Google App Engine
This is Rietveld 408576698