Sloppy-mode let parsing

Sloppy-mode let parsing

This patch makes 'let' a contextual keyword in both strict and sloppy mode.
It behaves as a keyword when used at the beginning of a StatementListItem
or lexical declaration at the beginning of a for statement, if it is followed
by an identifier, [ or {. Implementing this change requires an extra token
look-ahead by the parser which is only invoked in certain cases (so as to
avoid parsing RegExps as ECMAScript tokens). This might result in a slowdown
of the scanner, but performance testing of this patch hasn't yet found much
of a regression.

BUG=v8:3305
LOG=Y
R=adamk

[Dan Ehrenberg, 2015-08-14 20:10:16]

The CQ bit was checked by littledan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-08-14 20:10:30]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1295883002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1295883002/1

[commit-bot: I haz the power, 2015-08-14 20:13:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-14 20:13:31]

Dry run: Try jobs failed on following builders:
  v8_presubmit on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/4936)

[Dan Ehrenberg, 2015-08-14 20:21:41]

The CQ bit was checked by littledan@chromium.org to run a CQ dry run

[Dan Ehrenberg, 2015-08-14 20:21:48]

The CQ bit was unchecked by littledan@chromium.org

[Dan Ehrenberg, 2015-08-14 20:21:52]

The CQ bit was checked by littledan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-08-14 20:22:00]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1295883002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1295883002/1

[Dan Ehrenberg, 2015-08-14 20:32:58]

littledan@chromium.org changed reviewers:
+ adamk@chromium.org

[Dan Ehrenberg, 2015-08-14 20:32:58]

This is currently more of an RFC because of the indeterminate performance
impact. It should not be merged before the 46 branch.

[Dan Ehrenberg, 2015-08-14 20:49:59]

The CQ bit was checked by littledan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-08-14 20:50:09]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1295883002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1295883002/40001

[commit-bot: I haz the power, 2015-08-14 21:19:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-14 21:19:00]

Dry run: This issue passed the CQ dry run.

[Dan Ehrenberg, 2015-08-14 21:27:17]

Actually, the performance runs didn't show a big regression: On 
v8_linux64_haswell_perf_try, CodeLoad was up 0.1, and on  v8_linux32_perf_try,
CodeLoad was down just 0.4%. However, these tests seem to be very volatile. Does
anyone have an idea for getting more reliable results before submitting?

[Dan Ehrenberg, 2015-08-14 21:27:53]

littledan@chromium.org changed reviewers:
+ rossberg@chromium.org

[Dan Ehrenberg, 2015-08-14 21:42:23]

The CQ bit was checked by littledan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-08-14 21:42:40]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1295883002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1295883002/60001

[commit-bot: I haz the power, 2015-08-14 22:24:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-14 22:24:13]

Dry run: This issue passed the CQ dry run.

[adamk, 2015-08-15 00:28:31]

Looking good, especially like that the ugliness is mostly contained in the
Scanner. Mostly naming concerns below.

https://codereview.chromium.org/1295883002/diff/60001/src/preparser.h
File src/preparser.h (right):

https://codereview.chromium.org/1295883002/diff/60001/src/preparser.h#newcode...
src/preparser.h:3744: bool ParserBase<Traits>::IsLetKeyword() {
How about "IsNextLetKeyword()"? I think that makes it clearer what this is
operating on.

https://codereview.chromium.org/1295883002/diff/60001/src/preparser.h#newcode...
src/preparser.h:3749: Token::Value uber_next = PeekAhead();
next_next, maybe? See later comment.

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

https://codereview.chromium.org/1295883002/diff/60001/src/scanner.h#newcode547
src/scanner.h:547: (current_.literal_chars == &literal_buffers_[0])
Why switch to using an array for this? The advantage of separate names is that a
typo is a compile-time error, instead of an OOB access at runtime. I'd prefer to
go back unless there's a particular need for using an array.

https://codereview.chromium.org/1295883002/diff/60001/src/scanner.h#newcode750
src/scanner.h:750: TokenDesc uber_next_;  // desc for the token after next
(push-back)
I think "next_next_" is clearer in English. That's my vote for a name unless
Andreas has a better idea.

[Dan Ehrenberg, 2015-08-17 17:37:31]

The CQ bit was checked by littledan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-08-17 17:37:45]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1295883002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1295883002/80001

[commit-bot: I haz the power, 2015-08-17 18:04:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-17 18:04:42]

Dry run: This issue passed the CQ dry run.

[Dan Ehrenberg, 2015-08-17 18:05:02]

I ran Octane locally three times with and without this patch. I measured a 2.3%
regression caused by the patch in CodeLoad. Where do we go from here? I am
having trouble figuring out a clean strategy for avoiding the conditional by
pushing back all the way into the stream, though I can imagine some messy ways.
How bad would a regression of this magnitude be?

https://codereview.chromium.org/1295883002/diff/60001/src/preparser.h
File src/preparser.h (right):

https://codereview.chromium.org/1295883002/diff/60001/src/preparser.h#newcode...
src/preparser.h:3744: bool ParserBase<Traits>::IsLetKeyword() {
On 2015/08/15 at 00:28:31, adamk wrote:
> How about "IsNextLetKeyword()"? I think that makes it clearer what this is
operating on.

Done

https://codereview.chromium.org/1295883002/diff/60001/src/preparser.h#newcode...
src/preparser.h:3749: Token::Value uber_next = PeekAhead();
On 2015/08/15 at 00:28:31, adamk wrote:
> next_next, maybe? See later comment.

Done

[adamk, 2015-08-17 18:24:39]

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

https://codereview.chromium.org/1295883002/diff/60001/src/scanner.h#newcode547
src/scanner.h:547: (current_.literal_chars == &literal_buffers_[0])
On 2015/08/15 00:28:31, adamk wrote:
> Why switch to using an array for this? The advantage of separate names is that
a
> typo is a compile-time error, instead of an OOB access at runtime. I'd prefer
to
> go back unless there's a particular need for using an array.

I didn't see this addressed in the latest patch.

[adamk, 2015-08-17 18:25:26]

cc vogelheim@, who might have some insight into evaluating performance
regressions related to CodeLoad.

[Dan Ehrenberg, 2015-08-17 18:35:19]

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

https://codereview.chromium.org/1295883002/diff/60001/src/scanner.h#newcode547
src/scanner.h:547: (current_.literal_chars == &literal_buffers_[0])
On 2015/08/17 at 18:24:39, adamk wrote:
> On 2015/08/15 00:28:31, adamk wrote:
> > Why switch to using an array for this? The advantage of separate names is
that a
> > typo is a compile-time error, instead of an OOB access at runtime. I'd
prefer to
> > go back unless there's a particular need for using an array.
> 
> I didn't see this addressed in the latest patch.

Oops, fixed.

[Dan Ehrenberg, 2015-08-17 18:35:23]

The CQ bit was checked by littledan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-08-17 18:35:42]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1295883002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1295883002/100001

[commit-bot: I haz the power, 2015-08-17 19:22:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-17 19:22:28]

Dry run: This issue passed the CQ dry run.