Allow identifier code points from supplementary multilingual planes.

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

[Yang, 2014-10-09 08:35:54]

yangguo@chromium.org changed reviewers:
+ jochen@chromium.org, mathias@qiwi.be

[jochen (gone - plz use gerrit), 2014-10-09 09:24:47]

lgtm

[mathias, 2014-10-09 10:40:16]

How about adding some tests to test/mjsunit/var.js? E.g.

    assertDoesNotThrow("var 𐐼;"); // U+1043C DESERET SMALL LETTER DEE)

https://codereview.chromium.org/640193002/diff/20001/src/char-predicates.cc
File src/char-predicates.cc (right):

https://codereview.chromium.org/640193002/diff/20001/src/char-predicates.cc#n...
src/char-predicates.cc:1: // Copyright 2011 the V8 project authors. All rights
reserved.
2014, no?

[mathias, 2014-10-09 11:53:57]

On 2014/10/09 10:40:16, mathias wrote:
> How about adding some tests to test/mjsunit/var.js? E.g.
> 
>     assertDoesNotThrow("var 𐐼;"); // U+1043C DESERET SMALL LETTER DEE)

Not sure if this is relevant at all, but `uchar.h` still mentions Unicode 3.0.0
rather than 7.0.0. Search for “(The Unicode Standard, Version 3.0, chapter 5.16
Identifiers).”.
 
Could that be why the above test fails at the moment?

[Yang, 2014-10-09 11:54:51]

On 2014/10/09 11:53:57, mathias wrote:
> On 2014/10/09 10:40:16, mathias wrote:
> > How about adding some tests to test/mjsunit/var.js? E.g.
> > 
> >     assertDoesNotThrow("var 𐐼;"); // U+1043C DESERET SMALL LETTER DEE)
> 
> Not sure if this is relevant at all, but `uchar.h` still mentions Unicode
3.0.0
> rather than 7.0.0. Search for “(The Unicode Standard, Version 3.0, chapter
5.16
> Identifiers).”.
>  
> Could that be why the above test fails at the moment?

The reason it's failing is because the parser is still using UTF16 for
everything. I'll have to fix that first.

[Yang, 2014-10-09 11:55:51]

On 2014/10/09 11:54:51, Yang wrote:
> On 2014/10/09 11:53:57, mathias wrote:
> > On 2014/10/09 10:40:16, mathias wrote:
> > > How about adding some tests to test/mjsunit/var.js? E.g.
> > > 
> > >     assertDoesNotThrow("var 𐐼;"); // U+1043C DESERET SMALL LETTER DEE)
> > 
> > Not sure if this is relevant at all, but `uchar.h` still mentions Unicode
> 3.0.0
> > rather than 7.0.0. Search for “(The Unicode Standard, Version 3.0, chapter
> 5.16
> > Identifiers).”.
> >  
> > Could that be why the above test fails at the moment?
> 
> The reason it's failing is because the parser is still using UTF16 for
> everything. I'll have to fix that first.

btw my uchar.h says

#define U_UNICODE_VERSION "6.3"

[Yang, 2014-10-09 12:02:31]

On 2014/10/09 11:55:51, Yang wrote:
> On 2014/10/09 11:54:51, Yang wrote:
> > On 2014/10/09 11:53:57, mathias wrote:
> > > On 2014/10/09 10:40:16, mathias wrote:
> > > > How about adding some tests to test/mjsunit/var.js? E.g.
> > > > 
> > > >     assertDoesNotThrow("var 𐐼;"); // U+1043C DESERET SMALL LETTER DEE)
> > > 
> > > Not sure if this is relevant at all, but `uchar.h` still mentions Unicode
> > 3.0.0
> > > rather than 7.0.0. Search for “(The Unicode Standard, Version 3.0, chapter
> > 5.16
> > > Identifiers).”.
> > >  
> > > Could that be why the above test fails at the moment?
> > 
> > The reason it's failing is because the parser is still using UTF16 for
> > everything. I'll have to fix that first.
> 
> btw my uchar.h says
> 
> #define U_UNICODE_VERSION "6.3"

Ah. I see what you mean with Unicode Standard Version 3.0. It defines ID_Start
as categories Lu, Ll, Lt, Lm, Lo, Nl. This doesn't include Other_ID_Start and
doesn't exclude properties Pattern_*. But that's fine, since those differences
are only in the BMP. We only resort to ICU for code points in SMPs.

[mathias, 2014-10-09 12:03:02]

On 2014/10/09 11:55:51, Yang wrote:
> On 2014/10/09 11:54:51, Yang wrote:
> > On 2014/10/09 11:53:57, mathias wrote:
> > > On 2014/10/09 10:40:16, mathias wrote:
> > > > How about adding some tests to test/mjsunit/var.js? E.g.
> > > > 
> > > >     assertDoesNotThrow("var 𐐼;"); // U+1043C DESERET SMALL LETTER DEE)
> > > 
> > > Not sure if this is relevant at all, but `uchar.h` still mentions Unicode
> > 3.0.0
> > > rather than 7.0.0. Search for “(The Unicode Standard, Version 3.0, chapter
> > 5.16
> > > Identifiers).”.
> > >  
> > > Could that be why the above test fails at the moment?
> > 
> > The reason it's failing is because the parser is still using UTF16 for
> > everything. I'll have to fix that first.
> 
> btw my uchar.h says
> 
> #define U_UNICODE_VERSION "6.3"

I was looking at line 2972 — just a comment, but it’s kinda confusing. I thought
we were using Unicode 7.0.0 now?

[Yang, 2014-10-09 12:21:29]

On 2014/10/09 12:03:02, mathias wrote:
> On 2014/10/09 11:55:51, Yang wrote:
> > On 2014/10/09 11:54:51, Yang wrote:
> > > On 2014/10/09 11:53:57, mathias wrote:
> > > > On 2014/10/09 10:40:16, mathias wrote:
> > > > > How about adding some tests to test/mjsunit/var.js? E.g.
> > > > > 
> > > > >     assertDoesNotThrow("var 𐐼;"); // U+1043C DESERET SMALL LETTER DEE)
> > > > 
> > > > Not sure if this is relevant at all, but `uchar.h` still mentions
Unicode
> > > 3.0.0
> > > > rather than 7.0.0. Search for “(The Unicode Standard, Version 3.0,
chapter
> > > 5.16
> > > > Identifiers).”.
> > > >  
> > > > Could that be why the above test fails at the moment?
> > > 
> > > The reason it's failing is because the parser is still using UTF16 for
> > > everything. I'll have to fix that first.
> > 
> > btw my uchar.h says
> > 
> > #define U_UNICODE_VERSION "6.3"
> 
> I was looking at line 2972 — just a comment, but it’s kinda confusing. I
thought
> we were using Unicode 7.0.0 now?

The tables in src/unicode.cc are 7.0.0, but ICU hasn't caught up yet. We use ICU
for locales, and in this case, SMP.

[Yang, 2014-10-09 14:40:26]

On 2014/10/09 12:21:29, Yang wrote:
> On 2014/10/09 12:03:02, mathias wrote:
> > On 2014/10/09 11:55:51, Yang wrote:
> > > On 2014/10/09 11:54:51, Yang wrote:
> > > > On 2014/10/09 11:53:57, mathias wrote:
> > > > > On 2014/10/09 10:40:16, mathias wrote:
> > > > > > How about adding some tests to test/mjsunit/var.js? E.g.
> > > > > > 
> > > > > >     assertDoesNotThrow("var 𐐼;"); // U+1043C DESERET SMALL LETTER
DEE)
> > > > > 
> > > > > Not sure if this is relevant at all, but `uchar.h` still mentions
> Unicode
> > > > 3.0.0
> > > > > rather than 7.0.0. Search for “(The Unicode Standard, Version 3.0,
> chapter
> > > > 5.16
> > > > > Identifiers).”.
> > > > >  
> > > > > Could that be why the above test fails at the moment?
> > > > 
> > > > The reason it's failing is because the parser is still using UTF16 for
> > > > everything. I'll have to fix that first.
> > > 
> > > btw my uchar.h says
> > > 
> > > #define U_UNICODE_VERSION "6.3"
> > 
> > I was looking at line 2972 — just a comment, but it’s kinda confusing. I
> thought
> > we were using Unicode 7.0.0 now?
> 
> The tables in src/unicode.cc are 7.0.0, but ICU hasn't caught up yet. We use
ICU
> for locales, and in this case, SMP.

Uploaded the fix for the parser.

[mathias, 2014-10-09 14:45:42]

https://codereview.chromium.org/640193002/diff/40001/test/intl/general/smp-id...
File test/intl/general/smp-identifier.js (right):

https://codereview.chromium.org/640193002/diff/40001/test/intl/general/smp-id...
test/intl/general/smp-identifier.js:9: String.fromCharCode(c & 0x3FF | 0xDC00);
Since `--harmony-strings` is set anyway, why not use `String.fromCodePoint`
instead of `toSurrogatePair`?

https://codereview.chromium.org/640193002/diff/40001/test/mjsunit/parse-surro...
File test/mjsunit/parse-surrogates.js (right):

https://codereview.chromium.org/640193002/diff/40001/test/mjsunit/parse-surro...
test/mjsunit/parse-surrogates.js:7: assertThrows("'\\u000\uD801\uABCD'",
SyntaxError);
AFAICT, lone surrogates in strings must still be supported in ES6.

[Yang, 2014-10-09 14:50:18]

https://codereview.chromium.org/640193002/diff/40001/test/intl/general/smp-id...
File test/intl/general/smp-identifier.js (right):

https://codereview.chromium.org/640193002/diff/40001/test/intl/general/smp-id...
test/intl/general/smp-identifier.js:9: String.fromCharCode(c & 0x3FF | 0xDC00);
On 2014/10/09 14:45:42, mathias wrote:
> Since `--harmony-strings` is set anyway, why not use `String.fromCodePoint`
> instead of `toSurrogatePair`?

Apparently flags are not parsed in intl/ tests. I'm kinda too lazy to introduce
it here when I found out, but forgot to remove that line. I did now.

https://codereview.chromium.org/640193002/diff/40001/test/mjsunit/parse-surro...
File test/mjsunit/parse-surrogates.js (right):

https://codereview.chromium.org/640193002/diff/40001/test/mjsunit/parse-surro...
test/mjsunit/parse-surrogates.js:7: assertThrows("'\\u000\uD801\uABCD'",
SyntaxError);
On 2014/10/09 14:45:42, mathias wrote:
> AFAICT, lone surrogates in strings must still be supported in ES6.

But they are still not valid identifiers, right?

[mathias, 2014-10-09 14:52:18]

On 2014/10/09 14:50:18, Yang wrote:
>
https://codereview.chromium.org/640193002/diff/40001/test/mjsunit/parse-surro...
> File test/mjsunit/parse-surrogates.js (right):
> 
>
https://codereview.chromium.org/640193002/diff/40001/test/mjsunit/parse-surro...
> test/mjsunit/parse-surrogates.js:7: assertThrows("'\\u000\uD801\uABCD'",
> SyntaxError);
> On 2014/10/09 14:45:42, mathias wrote:
> > AFAICT, lone surrogates in strings must still be supported in ES6.
> 
> But they are still not valid identifiers, right?

Right.

[Yang, 2014-10-09 15:01:31]

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-surro...
> > File test/mjsunit/parse-surrogates.js (right):
> > 
> >
>
https://codereview.chromium.org/640193002/diff/40001/test/mjsunit/parse-surro...
> > test/mjsunit/parse-surrogates.js:7: assertThrows("'\\u000\uD801\uABCD'",
> > SyntaxError);
> > On 2014/10/09 14:45:42, mathias wrote:
> > > AFAICT, lone surrogates in strings must still be supported in ES6.
> > 
> > But they are still not valid identifiers, right?
> 
> Right.

So, that test is actually to test the hexcode parser. Note that running the
following script works.

'\\u000\uD801\uABCD'

But running

eval("'\\u000\uD801\uABCD'")

throws. The reason is that what the eval actually receives is something to the
effect of
'\u000<garbage character>'. When parsing the (assumably) escaped character, we
find that the last digit of that hex number is actually not a digit, hence we
throw. This behavior did not change with this patch.

Firefox also throws with "SyntaxError: malformed Unicode character escape
sequence".

[mathias, 2014-10-09 15:07:08]

On 2014/10/09 15:01:31, Yang wrote:
> 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-surro...
> > > File test/mjsunit/parse-surrogates.js (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/640193002/diff/40001/test/mjsunit/parse-surro...
> > > test/mjsunit/parse-surrogates.js:7: assertThrows("'\\u000\uD801\uABCD'",
> > > SyntaxError);
> > > On 2014/10/09 14:45:42, mathias wrote:
> > > > AFAICT, lone surrogates in strings must still be supported in ES6.
> > > 
> > > But they are still not valid identifiers, right?
> > 
> > Right.
> 
> So, that test is actually to test the hexcode parser. Note that running the
> following script works.
> 
> '\\u000\uD801\uABCD'
> 
> But running
> 
> eval("'\\u000\uD801\uABCD'")
> 
> throws. The reason is that what the eval actually receives is something to the
> effect of
> '\u000<garbage character>'. When parsing the (assumably) escaped character, we
> find that the last digit of that hex number is actually not a digit, hence we
> throw. This behavior did not change with this patch.
> 
> Firefox also throws with "SyntaxError: malformed Unicode character escape
> sequence".

Thanks for clarifying — I hadn’t spotted that there were only three `0`s.

[Yang, 2014-10-10 07:13:57]

Committed patchset #4 (id:60001) manually as 24510 (presubmit successful).

[marja, 2015-01-29 15:39:37]

marja@chromium.org changed reviewers:
+ marja@chromium.org

[marja, 2015-01-29 15:39:38]

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@.