Add initial parser support for harmony iteration

Add initial parser support for harmony iteration

This commit adds initial parser support for harmony iteration.
Specifically, it will parse:

  for (x of y) {}
  for (let x of y) {}
  for (var x of y) {}

The semantics are still unimplemented.

TEST=mjsunit/harmony/for-of-syntax
BUG=v8:2214
R=rossberg@chromium.org

Committed: http://code.google.com/p/v8/source/detail?r=14984

[wingo, 2013-05-20 16:38:47]

An initial patch for for-of syntax.  PTAL :)

[wingo, 2013-05-21 08:17:32]

New patch uses subtyping instead of the additional enumerated value --
ForInStatement is a ForEachStatement, as is ForOfStatement.

[wingo, 2013-06-06 09:11:36]

A kind ping on this issue :)

[Michael Starzinger, 2013-06-06 09:32:37]

Andreas has volunteered to look at this one. Thanks.

[Michael Starzinger, 2013-06-06 09:36:34]

Just one drive-by comment.

https://codereview.chromium.org/15300018/diff/3001/src/ast.h
File src/ast.h (right):

https://codereview.chromium.org/15300018/diff/3001/src/ast.h#newcode2839
src/ast.h:2839: default:
Please avoid default cases in switches over enums.

[rossberg, 2013-06-06 10:32:32]

LGTM with nits

https://codereview.chromium.org/15300018/diff/3001/src/ast.h
File src/ast.h (right):

https://codereview.chromium.org/15300018/diff/3001/src/ast.h#newcode860
src/ast.h:860: void Initialize(Expression* each,
Nit: doesn't this fit on one line?

https://codereview.chromium.org/15300018/diff/3001/src/full-codegen.cc
File src/full-codegen.cc (right):

https://codereview.chromium.org/15300018/diff/3001/src/full-codegen.cc#newcod...
src/full-codegen.cc:1395: void
FullCodeGenerator::VisitForOfStatement(ForOfStatement* stmt) {
Normally I'd want a TODO here, but since that's in flight already it doesn't
really matter...

https://codereview.chromium.org/15300018/diff/3001/src/parser.cc
File src/parser.cc (left):

https://codereview.chromium.org/15300018/diff/3001/src/parser.cc#oldcode2946
src/parser.cc:2946: if (loop) loop->Initialize(expression, enumerable, body);
Weird, what was the purpose of this condition?

https://codereview.chromium.org/15300018/diff/3001/src/scanner.h
File src/scanner.h (right):

https://codereview.chromium.org/15300018/diff/3001/src/scanner.h#newcode181
src/scanner.h:181: bool is_contextual_keyword(Vector<const char> keyword) {
Yes, that's better. :)

https://codereview.chromium.org/15300018/diff/3001/test/mjsunit/harmony/itera...
File test/mjsunit/harmony/iteration-syntax.js (right):

https://codereview.chromium.org/15300018/diff/3001/test/mjsunit/harmony/itera...
test/mjsunit/harmony/iteration-syntax.js:50: // Alack, this appears to be valid.
However, "for (of x)" or "for (of of)" shouldn't be.

[wingo, 2013-06-06 13:23:47]

On 2013/06/06 09:36:34, Michael Starzinger wrote:
> Just one drive-by comment.
> 
> https://codereview.chromium.org/15300018/diff/3001/src/ast.h
> File src/ast.h (right):
> 
> https://codereview.chromium.org/15300018/diff/3001/src/ast.h#newcode2839
> src/ast.h:2839: default:
> Please avoid default cases in switches over enums.

Unhappily, without this case GCC complains about "control reached end of
non-void function".

[Sven Panne, 2013-06-06 13:31:52]

On 2013/06/06 13:23:47, wingo wrote:
> On 2013/06/06 09:36:34, Michael Starzinger wrote:
> > Just one drive-by comment.
> > 
> > https://codereview.chromium.org/15300018/diff/3001/src/ast.h
> > File src/ast.h (right):
> > 
> > https://codereview.chromium.org/15300018/diff/3001/src/ast.h#newcode2839
> > src/ast.h:2839: default:
> > Please avoid default cases in switches over enums.
> 
> Unhappily, without this case GCC complains about "control reached end of
> non-void function".

Nevertheless, don't use the "default" case, just move its contents to the end of
the function. Now we *and* GCC are happy. :)

[wingo, 2013-06-06 14:09:50]

Fixed nits.  I did some spelunking to find the purpose of the "if (loop)" foo,
and I think it's just a bogus check.  I can roll that into another patch though,
if that's the right thing to do.

https://codereview.chromium.org/15300018/diff/3001/src/ast.h
File src/ast.h (right):

https://codereview.chromium.org/15300018/diff/3001/src/ast.h#newcode860
src/ast.h:860: void Initialize(Expression* each,
On 2013/06/06 10:32:33, rossberg wrote:
> Nit: doesn't this fit on one line?

Ah, yep.  Artifact from a previous patch; fixed.

https://codereview.chromium.org/15300018/diff/3001/src/ast.h#newcode2839
src/ast.h:2839: default:
On 2013/06/06 09:36:36, Michael Starzinger wrote:
> Please avoid default cases in switches over enums.

Fixed as per Sven's recommendation.  I think it was my inner schemer somehow
modelling switch as a "match" in tail position, forgetting I had "return"
available to me :)

https://codereview.chromium.org/15300018/diff/3001/src/full-codegen.cc
File src/full-codegen.cc (right):

https://codereview.chromium.org/15300018/diff/3001/src/full-codegen.cc#newcod...
src/full-codegen.cc:1395: void
FullCodeGenerator::VisitForOfStatement(ForOfStatement* stmt) {
On 2013/06/06 10:32:33, rossberg wrote:
> Normally I'd want a TODO here, but since that's in flight already it doesn't
> really matter...

Well, why not :) Done.

https://codereview.chromium.org/15300018/diff/3001/src/parser.cc
File src/parser.cc (left):

https://codereview.chromium.org/15300018/diff/3001/src/parser.cc#oldcode2946
src/parser.cc:2946: if (loop) loop->Initialize(expression, enumerable, body);
On 2013/06/06 10:32:33, rossberg wrote:
> Weird, what was the purpose of this condition?

I... I don't know!  I think this is another vestige from an earlier patch, but
I'm almost certain it's correct.  The factory constructor cannot possibly return
a non-null value.  Likewise there are null checks later in the function for
normal For; I'll remove those in the next patch.  This code was present in the
initial V8 import in 2008.

https://codereview.chromium.org/15300018/diff/3001/test/mjsunit/harmony/itera...
File test/mjsunit/harmony/iteration-syntax.js (right):

https://codereview.chromium.org/15300018/diff/3001/test/mjsunit/harmony/itera...
test/mjsunit/harmony/iteration-syntax.js:50: // Alack, this appears to be valid.
On 2013/06/06 10:32:33, rossberg wrote:
> However, "for (of x)" or "for (of of)" shouldn't be.

Done.

[rossberg, 2013-06-06 14:25:17]

Still LGTM

[wingo, 2013-06-06 14:38:40]

Committed patchset #4 manually as r14984 (presubmit successful).