ES6 unicode extensions, part 1.

ES6 unicode extensions, part 1.

Allows \u{xxxxx} in variable names and string literals (not yet in regexps).

Everything's behind the --harmony-unicode flag.

BUG=

[marja, 2014-11-13 11:54:02]

marja@chromium.org changed reviewers:
+ arv@chromium.org, caitpotter88@gmail.com, mathias@qiwi.be,
rossberg@chromium.org

[marja, 2014-11-13 11:54:03]

ptal

[caitp (gmail), 2014-11-13 15:09:18]

lgtm

lgtm

https://codereview.chromium.org/716423002/diff/40001/src/scanner.cc
File src/scanner.cc (right):

https://codereview.chromium.org/716423002/diff/40001/src/scanner.cc#newcode83
src/scanner.cc:83: if (x > max_value) return -1;
I don't think this should block landing this, but I think it would be an
improvement if this error condition were handled in the parser, so that an error
could be reported (like "SyntaxError: Unicode escape sequences cannot have a
value exceeding U+10FFFF" or something).

So you'd want to make sure the value doesn't overflow, but max_value checking
could be verified in the parser. Again though, not a blocker, I just think that
would be easier on people. SpiderMonkey outputs some pretty nice static errors
for a lot of these, but V8 is just saying "Unexpected token ____" which is not
that great.

Not a blocker but something to think about

https://codereview.chromium.org/716423002/diff/40001/test/cctest/test-parsing.cc
File test/cctest/test-parsing.cc (right):

https://codereview.chromium.org/716423002/diff/40001/test/cctest/test-parsing...
test/cctest/test-parsing.cc:4322: "var foob\\u{c481r = 0;",
Might be worth adding a test for  `var \\not-u` being treated as a syntax error.

[caitp (gmail), 2014-11-13 15:09:20]

lgtm

lgtm

[marja, 2014-11-13 15:18:05]

(Thanks for having a look)

https://codereview.chromium.org/716423002/diff/40001/test/cctest/test-parsing.cc
File test/cctest/test-parsing.cc (right):

https://codereview.chromium.org/716423002/diff/40001/test/cctest/test-parsing...
test/cctest/test-parsing.cc:4322: "var foob\\u{c481r = 0;",
On 2014/11/13 15:09:18, caitp wrote:
> Might be worth adding a test for  `var \\not-u` being treated as a syntax
error.

By var \not-u do you mean var \X where X is some letter other than u? Or
something else?

[caitp (gmail), 2014-11-13 15:29:48]

On 2014/11/13 15:18:05, marja wrote:
> (Thanks for having a look)
> 
>
https://codereview.chromium.org/716423002/diff/40001/test/cctest/test-parsing.cc
> File test/cctest/test-parsing.cc (right):
> 
>
https://codereview.chromium.org/716423002/diff/40001/test/cctest/test-parsing...
> test/cctest/test-parsing.cc:4322: "var foob\\u{c481r = 0;",
> On 2014/11/13 15:09:18, caitp wrote:
> > Might be worth adding a test for  `var \\not-u` being treated as a syntax
> error.
> 
> By var \not-u do you mean var \X where X is some letter other than u? Or
> something else?

Yeah exactly, it would have caught the little typo bugs in the other CL, so it
might be worth having it in case it accidentally breaks in the future

[arv (Not doing code reviews), 2014-11-13 18:44:26]

LGTM

[mathias, 2014-11-14 08:50:10]

lgtm

[rossberg, 2014-11-14 09:59:08]

lgtm

https://codereview.chromium.org/716423002/diff/40001/src/scanner.cc
File src/scanner.cc (right):

https://codereview.chromium.org/716423002/diff/40001/src/scanner.cc#newcode83
src/scanner.cc:83: if (x > max_value) return -1;
On 2014/11/13 15:09:18, caitp wrote:
> I don't think this should block landing this, but I think it would be an
> improvement if this error condition were handled in the parser, so that an
error
> could be reported (like "SyntaxError: Unicode escape sequences cannot have a
> value exceeding U+10FFFF" or something).
> 
> So you'd want to make sure the value doesn't overflow, but max_value checking
> could be verified in the parser. Again though, not a blocker, I just think
that
> would be easier on people. SpiderMonkey outputs some pretty nice static errors
> for a lot of these, but V8 is just saying "Unexpected token ____" which is not
> that great.
> 
> Not a blocker but something to think about

Alternatively, we should enable the scanner to give proper error messages.

[marja, 2014-12-02 10:26:20]

https://codereview.chromium.org/716423002/diff/40001/test/cctest/test-parsing.cc
File test/cctest/test-parsing.cc (right):

https://codereview.chromium.org/716423002/diff/40001/test/cctest/test-parsing...
test/cctest/test-parsing.cc:4322: "var foob\\u{c481r = 0;",
On 2014/11/13 15:09:18, caitp wrote:
> Might be worth adding a test for  `var \\not-u` being treated as a syntax
error.

Done.

[marja, 2014-12-02 10:26:23]

The CQ bit was checked by marja@chromium.org

[commit-bot: I haz the power, 2014-12-02 10:27:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/716423002/80001

[commit-bot: I haz the power, 2014-12-02 10:58:15]

Committed patchset #5 (id:80001)