Fix desugaring of let bindings in for loops to handle continue properly

Fix desugaring of let bindings in for loops to handle continue properly

This requires putting the original loop's body inside an inner for loop (with
the same labels as the original loop) and re-binding the temp variables in its
"next" expression. A second flag is added to the desugared code to ensure the
loop body executes at most once per loop.

BUG=v8:3683
LOG=y

Committed: https://chromium.googlesource.com/v8/v8/+/e98e70674690fdfe8f7c23ff666d150eb7283cee

[arv (Not doing code reviews), 2014-11-13 16:15:42]

arv@chromium.org changed reviewers:
+ arv@chromium.org

[arv (Not doing code reviews), 2014-11-13 16:15:43]

I wish there was a way to unit test what the generated code looks like.

https://codereview.chromium.org/720863002/diff/100001/src/parser.cc
File src/parser.cc (right):

https://codereview.chromium.org/720863002/diff/100001/src/parser.cc#newcode3089
src/parser.cc:3089: Expression* const1 = factory()->NewSmiLiteral(1,
RelocInfo::kNoPosition);
Maybe we should make these default to RelocInfo::kNoPosition? Would clean up the
code a bit.

[adamk, 2014-11-13 20:33:25]

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

[adamk, 2014-11-13 20:34:02]

This is now ready for review.

[arv (Not doing code reviews), 2014-11-13 21:00:50]

LGTM

[rossberg, 2014-11-14 10:58:01]

LGTM modulo comments

https://codereview.chromium.org/720863002/diff/120001/src/parser.cc
File src/parser.cc (right):

https://codereview.chromium.org/720863002/diff/120001/src/parser.cc#newcode3175
src/parser.cc:3175: loop->Initialize(NULL, flag_cond, compound_next_statement,
body_or_stop);
Perhaps add a comment explaining that this reuses the original loop node, and
particularly, retains its labels.

https://codereview.chromium.org/720863002/diff/120001/test/mjsunit/harmony/re...
File test/mjsunit/harmony/regress/regress-3683.js (right):

https://codereview.chromium.org/720863002/diff/120001/test/mjsunit/harmony/re...
test/mjsunit/harmony/regress/regress-3683.js:18: label: for (let x = 0; x < 10;)
{
Can we add additional tests with nested for-let loops, and both inner and outer
continues?

[adamk, 2014-11-14 19:03:36]

https://codereview.chromium.org/720863002/diff/120001/src/parser.cc
File src/parser.cc (right):

https://codereview.chromium.org/720863002/diff/120001/src/parser.cc#newcode3175
src/parser.cc:3175: loop->Initialize(NULL, flag_cond, compound_next_statement,
body_or_stop);
On 2014/11/14 10:58:01, rossberg wrote:
> Perhaps add a comment explaining that this reuses the original loop node, and
> particularly, retains its labels.

Added a comment and explained why we re-use.

https://codereview.chromium.org/720863002/diff/120001/test/mjsunit/harmony/re...
File test/mjsunit/harmony/regress/regress-3683.js (right):

https://codereview.chromium.org/720863002/diff/120001/test/mjsunit/harmony/re...
test/mjsunit/harmony/regress/regress-3683.js:18: label: for (let x = 0; x < 10;)
{
On 2014/11/14 10:58:01, rossberg wrote:
> Can we add additional tests with nested for-let loops, and both inner and
outer
> continues?

Added a variety of tests, and commented each one.

[adamk, 2014-11-14 19:03:52]

The CQ bit was checked by adamk@chromium.org

[commit-bot: I haz the power, 2014-11-14 19:04:52]

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

[commit-bot: I haz the power, 2014-11-14 19:33:01]

Committed patchset #8 (id:140001)