[es6] allow any LeftHandSideExpression in for-of loops

[es6] allow any LeftHandSideExpression in for-of loops

Fix an earlier regression which forbid non-VariableProxy LHS from being
used in for-of loops. Like for-in loops, the spec allows any LHS to be used,
with the sole exception that ObjectLiterals and ArrayLiterals must be valid
AssignmentPatterns.

Also fixes a bug in TurboFan which resulted in incorrectly replacing a variable load with a constant value in some instances, due to the AstLoopAssignmentAnalyzer failing to record the assignment to ForOfStatement's value.

BUG=v8:4418, v8:2720
LOG=N
R=wingo@igalia.com, littledan@chromium.org, adamk@chromium.org, bmeurer@chromium.org

Committed: https://crrev.com/096125deaafe50c2c82cbf795dd9eee5264cbbf7
Cr-Commit-Position: refs/heads/master@{#31816}

[caitp (gmail), 2015-11-04 18:58:50]

Quick fix for v8:4418 --- the for-loop parsing uses ParseExpression() rather
than ParseLeftHandSideExpression() to parse the LHS, in order to accommodate
C-style for-loop parsing. However, all of the obvious cases that could be caused
by that seem to be reported as SyntaxErrors already (reporting comma-separated
Expressions and assignments as invalid LHSes).

[caitp (gmail), 2015-11-04 18:59:47]

On 2015/11/04 18:58:50, caitp wrote:
> Quick fix for v8:4418 --- the for-loop parsing uses ParseExpression() rather
> than ParseLeftHandSideExpression() to parse the LHS, in order to accommodate
> C-style for-loop parsing. However, all of the obvious cases that could be
caused
> by that seem to be reported as SyntaxErrors already (reporting comma-separated
> Expressions and assignments as invalid LHSes).

oops, wingo@chromium.org? my bad. Fixing that :>

[caitp (gmail), 2015-11-04 19:00:08]

Description was changed from

==========
[es6] allow any LeftHandSideExpression in for-of loops

Fix an earlier regression which forbid non-VariableProxy LHS from being
used in for-of loops. Like for-in loops, the spec allows any LHS to be used,
with the sole exception that ObjectLiterals and ArrayLiterals must be valid
AssignmentPatterns.

BUG=v8:4418, v8:2720
LOG=N
R=wingo@chromium.org, littledan@chromium.org, adamk@chromium.org
==========

to

==========
[es6] allow any LeftHandSideExpression in for-of loops

Fix an earlier regression which forbid non-VariableProxy LHS from being
used in for-of loops. Like for-in loops, the spec allows any LHS to be used,
with the sole exception that ObjectLiterals and ArrayLiterals must be valid
AssignmentPatterns.

BUG=v8:4418, v8:2720
LOG=N
R=wingo@igalia.com, littledan@chromium.org, adamk@chromium.org
==========

[caitp (gmail), 2015-11-04 19:00:08]

caitpotter88@gmail.com changed reviewers:
+ wingo@igalia.com
- wingo@chromium.org

[adamk, 2015-11-04 19:29:15]

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

[commit-bot: I haz the power, 2015-11-04 19:29:20]

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

[adamk, 2015-11-04 19:32:33]

lgtm if tests pass

[caitp (gmail), 2015-11-04 19:42:41]

On 2015/11/04 19:32:33, adamk wrote:
> lgtm if tests pass

I guess there are failures, looking...

[caitp (gmail), 2015-11-04 19:55:04]

On 2015/11/04 19:42:41, caitp wrote:
> On 2015/11/04 19:32:33, adamk wrote:
> > lgtm if tests pass
> 
> I guess there are failures, looking...

The tests are good, this seems to be a bug in the compilers (affecting both
crankshaft and TF)

[caitp (gmail), 2015-11-04 20:53:00]

On 2015/11/04 19:55:04, caitp wrote:
> On 2015/11/04 19:42:41, caitp wrote:
> > On 2015/11/04 19:32:33, adamk wrote:
> > > lgtm if tests pass
> > 
> > I guess there are failures, looking...
> 
> The tests are good, this seems to be a bug in the compilers (affecting both
> crankshaft and TF)

I suspect the compiler issue is, the ALAA fails to record the assignment to the
`each` variable, because this code isn't actually in the AST.  Probably an easy
fix, though

[caitp (gmail), 2015-11-04 20:56:01]

On 2015/11/04 20:53:00, caitp wrote:
> On 2015/11/04 19:55:04, caitp wrote:
> > On 2015/11/04 19:42:41, caitp wrote:
> > > On 2015/11/04 19:32:33, adamk wrote:
> > > > lgtm if tests pass
> > > 
> > > I guess there are failures, looking...
> > 
> > The tests are good, this seems to be a bug in the compilers (affecting both
> > crankshaft and TF)
> 
> I suspect the compiler issue is, the ALAA fails to record the assignment to
the
> `each` variable, because this code isn't actually in the AST.  Probably an
easy
> fix, though

yup, bingo. Sending a fix...

[caitp (gmail), 2015-11-04 21:01:17]

The CQ bit was checked by caitpotter88@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2015-11-04 21:01:25]

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

[caitp (gmail), 2015-11-04 21:05:07]

Description was changed from

==========
[es6] allow any LeftHandSideExpression in for-of loops

Fix an earlier regression which forbid non-VariableProxy LHS from being
used in for-of loops. Like for-in loops, the spec allows any LHS to be used,
with the sole exception that ObjectLiterals and ArrayLiterals must be valid
AssignmentPatterns.

BUG=v8:4418, v8:2720
LOG=N
R=wingo@igalia.com, littledan@chromium.org, adamk@chromium.org
==========

to

==========
[es6] allow any LeftHandSideExpression in for-of loops

Fix an earlier regression which forbid non-VariableProxy LHS from being
used in for-of loops. Like for-in loops, the spec allows any LHS to be used,
with the sole exception that ObjectLiterals and ArrayLiterals must be valid
AssignmentPatterns.

Also fixes a bug in TurboFan and Crankshaft which resulted in incorrectly
replacing a variable load with a constant value in some instances, due to the
AstLoopAssignmentAnalyzer failing to record the assignment to ForOfStatement's
value.

BUG=v8:4418, v8:2720
LOG=N
R=wingo@igalia.com, littledan@chromium.org, adamk@chromium.org
==========

[commit-bot: I haz the power, 2015-11-04 21:20:33]

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

[commit-bot: I haz the power, 2015-11-04 21:20:34]

Dry run: This issue passed the CQ dry run.

[caitp (gmail), 2015-11-04 21:21:21]

The CQ bit was checked by caitpotter88@gmail.com

[caitp (gmail), 2015-11-04 21:21:22]

The patchset sent to the CQ was uploaded after l-g-t-m from adamk@chromium.org
Link to the patchset: https://codereview.chromium.org/1411873004/#ps40001
(title: "Fix bug in loop-assignment-analyzer which resulted in incorrect code")

[commit-bot: I haz the power, 2015-11-04 21:21:31]

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

[commit-bot: I haz the power, 2015-11-04 21:23:53]

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

[commit-bot: I haz the power, 2015-11-04 21:23:54]

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

[caitp (gmail), 2015-11-04 21:24:05]

Description was changed from

==========
[es6] allow any LeftHandSideExpression in for-of loops

Fix an earlier regression which forbid non-VariableProxy LHS from being
used in for-of loops. Like for-in loops, the spec allows any LHS to be used,
with the sole exception that ObjectLiterals and ArrayLiterals must be valid
AssignmentPatterns.

Also fixes a bug in TurboFan and Crankshaft which resulted in incorrectly
replacing a variable load with a constant value in some instances, due to the
AstLoopAssignmentAnalyzer failing to record the assignment to ForOfStatement's
value.

BUG=v8:4418, v8:2720
LOG=N
R=wingo@igalia.com, littledan@chromium.org, adamk@chromium.org
==========

to

==========
[es6] allow any LeftHandSideExpression in for-of loops

Fix an earlier regression which forbid non-VariableProxy LHS from being
used in for-of loops. Like for-in loops, the spec allows any LHS to be used,
with the sole exception that ObjectLiterals and ArrayLiterals must be valid
AssignmentPatterns.

Also fixes a bug in TurboFan and Crankshaft which resulted in incorrectly
replacing a variable load with a constant value in some instances, due to the
AstLoopAssignmentAnalyzer failing to record the assignment to ForOfStatement's
value.

BUG=v8:4418, v8:2720
LOG=N
R=wingo@igalia.com, littledan@chromium.org, adamk@chromium.org,
bmeurer@chromium.org
==========

[caitp (gmail), 2015-11-04 21:24:05]

caitpotter88@gmail.com changed reviewers:
+ bmeurer@chromium.org

[caitp (gmail), 2015-11-04 21:24:50]

Oops, yeah. Can someone from src/compilers please check the ALAA change?

[adamk, 2015-11-04 21:40:59]

Is the mention of Crankshaft correct in the CL description? I'd think this
should only affect TurboFan.

[caitp (gmail), 2015-11-04 21:42:09]

On 2015/11/04 21:40:59, adamk wrote:
> Is the mention of Crankshaft correct in the CL description? I'd think this
> should only affect TurboFan.

Yeah, I guess crankshaft has a bailout on VisitForOfStatement, fixing.

[caitp (gmail), 2015-11-04 21:42:27]

Description was changed from

==========
[es6] allow any LeftHandSideExpression in for-of loops

Fix an earlier regression which forbid non-VariableProxy LHS from being
used in for-of loops. Like for-in loops, the spec allows any LHS to be used,
with the sole exception that ObjectLiterals and ArrayLiterals must be valid
AssignmentPatterns.

Also fixes a bug in TurboFan and Crankshaft which resulted in incorrectly
replacing a variable load with a constant value in some instances, due to the
AstLoopAssignmentAnalyzer failing to record the assignment to ForOfStatement's
value.

BUG=v8:4418, v8:2720
LOG=N
R=wingo@igalia.com, littledan@chromium.org, adamk@chromium.org,
bmeurer@chromium.org
==========

to

==========
[es6] allow any LeftHandSideExpression in for-of loops

Fix an earlier regression which forbid non-VariableProxy LHS from being
used in for-of loops. Like for-in loops, the spec allows any LHS to be used,
with the sole exception that ObjectLiterals and ArrayLiterals must be valid
AssignmentPatterns.

Also fixes a bug in TurboFan which resulted in incorrectly replacing a variable
load with a constant value in some instances, due to the
AstLoopAssignmentAnalyzer failing to record the assignment to ForOfStatement's
value.

BUG=v8:4418, v8:2720
LOG=N
R=wingo@igalia.com, littledan@chromium.org, adamk@chromium.org,
bmeurer@chromium.org
==========

[Benedikt Meurer, 2015-11-05 06:45:50]

LGTM on compiler.

[Benedikt Meurer, 2015-11-05 06:46:07]

The CQ bit was checked by bmeurer@chromium.org

[commit-bot: I haz the power, 2015-11-05 06:46:13]

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

[commit-bot: I haz the power, 2015-11-05 06:47:31]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-11-05 06:48:03]

Patchset 3 (id:??) landed as
https://crrev.com/096125deaafe50c2c82cbf795dd9eee5264cbbf7
Cr-Commit-Position: refs/heads/master@{#31816}