[es6] Fix completion values of for loops with lexical variables

[es6] Fix completion values of for loops with lexical variables

Currently, the desugaring of for loops of the form for
(let/const ...; bla; bla) causes them to always have a
completion value of 1, regardless of whether the loop body
is executed or not. This CL fixes this, realigning
initializer blocks as a more general purpose way to avoid
the completion value rewriter (since that's all they really
do anyway).

BUG=

Committed: https://crrev.com/1bb051b8a4b81aea753715a833b425d120fb5da6
Cr-Commit-Position: refs/heads/master@{#29108}

[conradw, 2015-06-17 19:01:03]

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

[conradw, 2015-06-17 19:01:04]

PTAL

[rossberg, 2015-06-18 05:43:21]

LGTM

https://codereview.chromium.org/1177053006/diff/1/src/ast.h
File src/ast.h (right):

https://codereview.chromium.org/1177053006/diff/1/src/ast.h#newcode487
src/ast.h:487: bool ignore_completion_values() const { return
ignore_completion_values_; }
Nit: why the plural?

https://codereview.chromium.org/1177053006/diff/1/src/parser.cc
File src/parser.cc (right):

https://codereview.chromium.org/1177053006/diff/1/src/parser.cc#newcode3227
src/parser.cc:3227: //    { // Nothing inside this block can affect the normal
completion value.
Nit: adjust indentation. Also, the comment seems a bit unclear. Maybe add s.th
like "This is an XY block: ..."

[conradw, 2015-06-18 10:53:32]

https://codereview.chromium.org/1177053006/diff/1/src/ast.h
File src/ast.h (right):

https://codereview.chromium.org/1177053006/diff/1/src/ast.h#newcode487
src/ast.h:487: bool ignore_completion_values() const { return
ignore_completion_values_; }
On 2015/06/18 05:43:21, rossberg wrote:
> Nit: why the plural?

The idea I had in my head was that the block causes each statement inside to
have its completion value ignored. This matches the way the rewriter updates the
value of .return at the level of each individual "base" ExpressionStatement.

The block itself isn't really part of the structure of the program like a normal
block is, since it's neither part of a production nor associated with a scope,
so I thought it would be misleading to talk about the block's "completion
value".

https://codereview.chromium.org/1177053006/diff/1/src/parser.cc
File src/parser.cc (right):

https://codereview.chromium.org/1177053006/diff/1/src/parser.cc#newcode3227
src/parser.cc:3227: //    { // Nothing inside this block can affect the normal
completion value.
On 2015/06/18 05:43:21, rossberg wrote:
> Nit: adjust indentation. Also, the comment seems a bit unclear. Maybe add s.th
> like "This is an XY block: ..."

rewritten

[rossberg, 2015-06-18 10:58:07]

https://codereview.chromium.org/1177053006/diff/1/src/ast.h
File src/ast.h (right):

https://codereview.chromium.org/1177053006/diff/1/src/ast.h#newcode487
src/ast.h:487: bool ignore_completion_values() const { return
ignore_completion_values_; }
On 2015/06/18 10:53:32, conradw wrote:
> On 2015/06/18 05:43:21, rossberg wrote:
> > Nit: why the plural?
> 
> The idea I had in my head was that the block causes each statement inside to
> have its completion value ignored. This matches the way the rewriter updates
the
> value of .return at the level of each individual "base" ExpressionStatement.
> 
> The block itself isn't really part of the structure of the program like a
normal
> block is, since it's neither part of a production nor associated with a scope,
> so I thought it would be misleading to talk about the block's "completion
> value".

Well, I would argue that's an implementation detail of the rewriter, not a
description of the meaning of the block as such. (Imagine there was no rewriter,
and we compiled these blocks directly.)

[conradw, 2015-06-18 11:52:04]

The CQ bit was checked by conradw@chromium.org

[conradw, 2015-06-18 11:52:05]

The patchset sent to the CQ was uploaded after l-g-t-m from
rossberg@chromium.org
Link to the patchset: https://codereview.chromium.org/1177053006/#ps40001
(title: "offline feedback")

[commit-bot: I haz the power, 2015-06-18 11:52:30]

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

[commit-bot: I haz the power, 2015-06-18 11:54:09]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-06-18 11:54:24]

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