Implement ES6 completion semantics (--harmony-completion).

[es6] Implement completion value reform (--harmony-completion).

This CL depends on #1362363002.

R=rossberg
BUG=

Committed: https://crrev.com/7a0a68208336472d3f38cf63750b683b879a1c27
Cr-Commit-Position: refs/heads/master@{#31180}

[rossberg, 2015-09-28 14:09:22]

Looks good so far. I believe we can fix the problem with the for-let desugaring
by changing it to the following:

  //  {
  //    let/const x = i;
  //    temp_x = x;
  //    first = 1;
  //    outer: for (;;) {
  //      let/const x = temp_x;
  //      { // init block
  //        // This block's only function is to ensure that the statements it
  //        // contains do not affect the normal completion value. This is
  //        // accomplished by setting its ignore_completion_value bit.
  //        // No new lexical scope is introduced, so lexically scoped variables
  //        // declared here will be scoped to the outer for loop.
  //        if (first == 1) {
  //          first = 0;
  //        } else {
  //          next;
  //        }
  //        flag = 1;
  //      }
  //      labels: for (; flag == 1; flag = 0, temp_x = x) {
  //        { // init block
  //          if (!cond) break outer;
  //        }
  //        body
  //      }
  //      { // init block
  //        if (flag == 1) break
  //      }
  //    }
  //  }

https://codereview.chromium.org/1361403003/diff/1/src/rewriter.cc
File src/rewriter.cc (right):

https://codereview.chromium.org/1361403003/diff/1/src/rewriter.cc#newcode125
src/rewriter.cc:125: is_set_ = true;
Why not move this into AssignUndefinedBeforeNode?

[neis, 2015-10-03 08:45:49]

On 2015/09/28 14:09:22, rossberg wrote:
> Looks good so far. I believe we can fix the problem with the for-let
desugaring
> by changing it to the following:
> 
>   //  {
>   //    let/const x = i;
>   //    temp_x = x;
>   //    first = 1;
>   //    outer: for (;;) {
>   //      let/const x = temp_x;
>   //      { // init block
>   //        // This block's only function is to ensure that the statements it
>   //        // contains do not affect the normal completion value. This is
>   //        // accomplished by setting its ignore_completion_value bit.
>   //        // No new lexical scope is introduced, so lexically scoped
variables
>   //        // declared here will be scoped to the outer for loop.
>   //        if (first == 1) {
>   //          first = 0;
>   //        } else {
>   //          next;
>   //        }
>   //        flag = 1;
>   //      }
>   //      labels: for (; flag == 1; flag = 0, temp_x = x) {
>   //        { // init block
>   //          if (!cond) break outer;
>   //        }
>   //        body
>   //      }
>   //      { // init block
>   //        if (flag == 1) break
>   //      }
>   //    }
>   //  }

As discussed offline, this doesn't quite work.  But I think we can fix it by
moving the condition check out of the inner loop.

[neis, 2015-10-05 10:57:28]

https://codereview.chromium.org/1361403003/diff/1/src/rewriter.cc
File src/rewriter.cc (right):

https://codereview.chromium.org/1361403003/diff/1/src/rewriter.cc#newcode125
src/rewriter.cc:125: is_set_ = true;
On 2015/09/28 14:09:22, rossberg wrote:
> Why not move this into AssignUndefinedBeforeNode?

I prefer the is_set_ logic to be explicit in the Visit functions.

[neis, 2015-10-05 11:15:07]

Patchset #2 (id:20001) has been deleted

[neis, 2015-10-05 11:20:02]

Patch Set 3 fixes the for-let desugaring and adapts some tests.  PHAL.

[rossberg, 2015-10-05 11:33:00]

https://codereview.chromium.org/1361403003/diff/60001/src/parser.cc
File src/parser.cc (right):

https://codereview.chromium.org/1361403003/diff/60001/src/parser.cc#newcode3355
src/parser.cc:3355: //         let/const x = temp_x;
Can you just move this declaration out of the block, to make its scope clearer
and make that comment obsolete?

https://codereview.chromium.org/1361403003/diff/60001/src/parser.cc#newcode3373
src/parser.cc:3373: // We write {{ ... }} for blocks whose completion value is
ignored.
Nit: move this comment to the top, since it's easy to miss here.

https://codereview.chromium.org/1361403003/diff/60001/test/mjsunit/harmony/bl...
File test/mjsunit/harmony/block-for-sloppy.js (right):

https://codereview.chromium.org/1361403003/diff/60001/test/mjsunit/harmony/bl...
test/mjsunit/harmony/block-for-sloppy.js:28: // Flags: --no-legacy-const
--harmony-sloppy --harmony-sloppy-let --harmony-completion
Nit: line too long. You can have several Flags lines.

https://codereview.chromium.org/1361403003/diff/60001/test/mjsunit/harmony/bl...
test/mjsunit/harmony/block-for-sloppy.js:193: assertEquals(undefined, eval("for
(let i = 0; true; i++) { i; if (i >= 3) break; }"));
Nit: line length

https://codereview.chromium.org/1361403003/diff/60001/test/mjsunit/harmony/bl...
test/mjsunit/harmony/block-for-sloppy.js:194: assertEquals(undefined, eval("for
(let i = 0; true; i++) { if (i >= 3) break; i; }"));
Dito

https://codereview.chromium.org/1361403003/diff/60001/test/mjsunit/harmony/co...
File test/mjsunit/harmony/completion.js (right):

https://codereview.chromium.org/1361403003/diff/60001/test/mjsunit/harmony/co...
test/mjsunit/harmony/completion.js:8: function assertUndef(x) {
Can we also have tests that check for the correct non-undef values in other
cases, especially with uses of break?

https://codereview.chromium.org/1361403003/diff/60001/test/mjsunit/harmony/co...
test/mjsunit/harmony/completion.js:26: assertUndef(eval('42; for (var x = 1; x;
x--) ;'));  // ES5: 42
No tests for for-let, for-of, for-in?

https://codereview.chromium.org/1361403003/diff/60001/test/mjsunit/harmony/co...
test/mjsunit/harmony/completion.js:50: // Some combinations
Can we have more of those? There probably are various cases with breaks
somewhere in the middle that you want to test.

https://codereview.chromium.org/1361403003/diff/60001/test/mjsunit/harmony/co...
test/mjsunit/harmony/completion.js:60:
/////////////////////////////////////////////////////////////////////////////////
Nit: line length

https://codereview.chromium.org/1361403003/diff/60001/test/mjsunit/harmony/co...
test/mjsunit/harmony/completion.js:89: assertUndef(eval("try{1; var i = 0; do {
++i; 2; throw ''; } while(i!=1);} catch(e){}"));
Line length

[neis, 2015-10-07 11:58:40]

https://codereview.chromium.org/1361403003/diff/60001/src/parser.cc
File src/parser.cc (right):

https://codereview.chromium.org/1361403003/diff/60001/src/parser.cc#newcode3355
src/parser.cc:3355: //         let/const x = temp_x;
On 2015/10/05 11:33:00, rossberg wrote:
> Can you just move this declaration out of the block, to make its scope clearer
> and make that comment obsolete?

Done.

https://codereview.chromium.org/1361403003/diff/60001/src/parser.cc#newcode3373
src/parser.cc:3373: // We write {{ ... }} for blocks whose completion value is
ignored.
On 2015/10/05 11:33:00, rossberg wrote:
> Nit: move this comment to the top, since it's easy to miss here.

Done.

https://codereview.chromium.org/1361403003/diff/60001/test/mjsunit/harmony/bl...
File test/mjsunit/harmony/block-for-sloppy.js (right):

https://codereview.chromium.org/1361403003/diff/60001/test/mjsunit/harmony/bl...
test/mjsunit/harmony/block-for-sloppy.js:28: // Flags: --no-legacy-const
--harmony-sloppy --harmony-sloppy-let --harmony-completion
On 2015/10/05 11:33:00, rossberg wrote:
> Nit: line too long. You can have several Flags lines.

Done.

https://codereview.chromium.org/1361403003/diff/60001/test/mjsunit/harmony/bl...
test/mjsunit/harmony/block-for-sloppy.js:193: assertEquals(undefined, eval("for
(let i = 0; true; i++) { i; if (i >= 3) break; }"));
On 2015/10/05 11:33:00, rossberg wrote:
> Nit: line length

Done.

https://codereview.chromium.org/1361403003/diff/60001/test/mjsunit/harmony/bl...
test/mjsunit/harmony/block-for-sloppy.js:194: assertEquals(undefined, eval("for
(let i = 0; true; i++) { if (i >= 3) break; i; }"));
On 2015/10/05 11:33:00, rossberg wrote:
> Dito

Done.

https://codereview.chromium.org/1361403003/diff/60001/test/mjsunit/harmony/co...
File test/mjsunit/harmony/completion.js (right):

https://codereview.chromium.org/1361403003/diff/60001/test/mjsunit/harmony/co...
test/mjsunit/harmony/completion.js:8: function assertUndef(x) {
On 2015/10/05 11:33:00, rossberg wrote:
> Can we also have tests that check for the correct non-undef values in other
> cases, especially with uses of break?

I added some. There's also a bunch in harmony/block-for-sloppy.

https://codereview.chromium.org/1361403003/diff/60001/test/mjsunit/harmony/co...
test/mjsunit/harmony/completion.js:26: assertUndef(eval('42; for (var x = 1; x;
x--) ;'));  // ES5: 42
On 2015/10/05 11:33:00, rossberg wrote:
> No tests for for-let, for-of, for-in?

Done.

https://codereview.chromium.org/1361403003/diff/60001/test/mjsunit/harmony/co...
test/mjsunit/harmony/completion.js:50: // Some combinations
On 2015/10/05 11:33:00, rossberg wrote:
> Can we have more of those? There probably are various cases with breaks
> somewhere in the middle that you want to test.

Added some.  There's also a bunch in harmony/block-for-sloppy and in the stuff
copied from webkit.

https://codereview.chromium.org/1361403003/diff/60001/test/mjsunit/harmony/co...
test/mjsunit/harmony/completion.js:60:
/////////////////////////////////////////////////////////////////////////////////
On 2015/10/05 11:33:00, rossberg wrote:
> Nit: line length

Done.

https://codereview.chromium.org/1361403003/diff/60001/test/mjsunit/harmony/co...
test/mjsunit/harmony/completion.js:89: assertUndef(eval("try{1; var i = 0; do {
++i; 2; throw ''; } while(i!=1);} catch(e){}"));
On 2015/10/05 11:33:00, rossberg wrote:
> Line length

Done.

[rossberg, 2015-10-07 12:28:46]

LGTM modulo comments

https://codereview.chromium.org/1361403003/diff/80001/src/parser.cc
File src/parser.cc (right):

https://codereview.chromium.org/1361403003/diff/80001/src/parser.cc#newcode3346
src/parser.cc:3346: // and rewrite it as follows.  Here we write {{ ... }} for
blocks whose
Nit: ...for "init-blocks", whose...

https://codereview.chromium.org/1361403003/diff/80001/test/mjsunit/harmony/co...
File test/mjsunit/harmony/completion.js (right):

https://codereview.chromium.org/1361403003/diff/80001/test/mjsunit/harmony/co...
test/mjsunit/harmony/completion.js:5: // Flags: --harmony-completion --harmony
--no-legacy-const
Don't use the generic --harmony flag in tests.

https://codereview.chromium.org/1361403003/diff/80001/test/mjsunit/harmony/co...
test/mjsunit/harmony/completion.js:81: assertUndef(eval('42; try { throw Error }
catch(e) { } finally { };'));  // ES5: 42
Nit: Add one for finally without catch

https://codereview.chromium.org/1361403003/diff/80001/test/mjsunit/harmony/co...
test/mjsunit/harmony/completion.js:98: '42; while (1) { try { throw "bla" }
catch(e) {666} finally {0; break} }'))
Also:

'42; while (1) { try { 666 } finally {0; break} }'
'42; while (1) { try { 666; break } finally {0; break} }'
'42; lab: try { 666; break lab } finally {0; break lab}'

[neis, 2015-10-08 12:57:25]

https://codereview.chromium.org/1361403003/diff/80001/src/parser.cc
File src/parser.cc (right):

https://codereview.chromium.org/1361403003/diff/80001/src/parser.cc#newcode3346
src/parser.cc:3346: // and rewrite it as follows.  Here we write {{ ... }} for
blocks whose
On 2015/10/07 12:28:46, rossberg wrote:
> Nit: ...for "init-blocks", whose...

Done.

https://codereview.chromium.org/1361403003/diff/80001/test/mjsunit/harmony/co...
File test/mjsunit/harmony/completion.js (right):

https://codereview.chromium.org/1361403003/diff/80001/test/mjsunit/harmony/co...
test/mjsunit/harmony/completion.js:5: // Flags: --harmony-completion --harmony
--no-legacy-const
On 2015/10/07 12:28:46, rossberg wrote:
> Don't use the generic --harmony flag in tests.

Done.

https://codereview.chromium.org/1361403003/diff/80001/test/mjsunit/harmony/co...
test/mjsunit/harmony/completion.js:81: assertUndef(eval('42; try { throw Error }
catch(e) { } finally { };'));  // ES5: 42
On 2015/10/07 12:28:46, rossberg wrote:
> Nit: Add one for finally without catch

Done.

https://codereview.chromium.org/1361403003/diff/80001/test/mjsunit/harmony/co...
test/mjsunit/harmony/completion.js:98: '42; while (1) { try { throw "bla" }
catch(e) {666} finally {0; break} }'))
On 2015/10/07 12:28:46, rossberg wrote:
> Also:
> 
> '42; while (1) { try { 666 } finally {0; break} }'
> '42; while (1) { try { 666; break } finally {0; break} }'
> '42; lab: try { 666; break lab } finally {0; break lab}'

Good point. Done.

[neis, 2015-10-08 13:32:56]

The CQ bit was checked by neis@chromium.org

[neis, 2015-10-08 13:32:56]

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/1361403003/#ps140001
(title: "Proper rebase.")

[commit-bot: I haz the power, 2015-10-08 13:33:04]

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

[neis, 2015-10-08 13:33:38]

The CQ bit was unchecked by neis@chromium.org

[neis, 2015-10-08 13:34:15]

The CQ bit was checked by neis@chromium.org

[commit-bot: I haz the power, 2015-10-08 13:34:28]

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

[commit-bot: I haz the power, 2015-10-08 13:56:53]

Committed patchset #7 (id:140001)

[commit-bot: I haz the power, 2015-10-08 13:57:16]

Patchset 7 (id:??) landed as
https://crrev.com/7a0a68208336472d3f38cf63750b683b879a1c27
Cr-Commit-Position: refs/heads/master@{#31180}