[parser] disallow language mode directive in body of function with non-simple parameters

[parser] disallow language mode directive in body of function with non-simple parameters

TC39 agreed to disallow "use strict" directives in function body when
non-simple parameter lists are used.

This is a continuation of caitp's CL https://codereview.chromium.org/1281163002/
with some refactorings removed for now.

Still TODO: there is a lot of duplication between the is_simple field of
FormalParametersBase and the NonSimpleParameter property ExpressionClassifier
keeps track of. It should be possible to remove the former with a minor
refactoring of arrow function parsing. This will be attempted in a follow-up CL.

BUG=
LOG=N

Committed: https://crrev.com/77394fa05a63a539ac4e6858d99cc85ec6867512
Cr-Commit-Position: refs/heads/master@{#30388}

[conradw, 2015-08-21 12:27:04]

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

[conradw, 2015-08-21 12:27:04]

PTAL

[rossberg, 2015-08-21 12:40:16]

You might want to consider changing the CL title, because the actual revert is
not even in it, IIUC.

[conradw, 2015-08-21 12:43:27]

On 2015/08/21 12:40:16, rossberg wrote:
> You might want to consider changing the CL title, because the actual revert is
> not even in it, IIUC.

done

[rossberg, 2015-08-24 13:11:08]

https://codereview.chromium.org/1300103005/diff/100001/src/expression-classif...
File src/expression-classifier.h (right):

https://codereview.chromium.org/1300103005/diff/100001/src/expression-classif...
src/expression-classifier.h:235: // parameter
Nit: period

https://codereview.chromium.org/1300103005/diff/100001/src/preparser.cc
File src/preparser.cc (right):

https://codereview.chromium.org/1300103005/diff/100001/src/preparser.cc#newco...
src/preparser.cc:270: if (!scope_->HasSimpleParameters()) {
Nit: merge conditions

https://codereview.chromium.org/1300103005/diff/100001/src/preparser.cc#newco...
src/preparser.cc:271: // A block declaration scope as a child scope of a
function scope
Not sure how the comment relates to the code. Which block scope?

https://codereview.chromium.org/1300103005/diff/100001/src/preparser.h
File src/preparser.h (right):

https://codereview.chromium.org/1300103005/diff/100001/src/preparser.h#newcod...
src/preparser.h:2860: bool is_simple =
arrow_formals_classifier.is_simple_parameter_list();
Nit: avoid the extra variable by storing to parameters.is_simple directly.

https://codereview.chromium.org/1300103005/diff/100001/test/cctest/test-parsi...
File test/cctest/test-parsing.cc (right):

https://codereview.chromium.org/1300103005/diff/100001/test/cctest/test-parsi...
test/cctest/test-parsing.cc:6885: // A block declaration scope as a child scope
of a function scope
Same here.

https://codereview.chromium.org/1300103005/diff/100001/test/mjsunit/harmony/d...
File test/mjsunit/harmony/destructuring.js (right):

https://codereview.chromium.org/1300103005/diff/100001/test/mjsunit/harmony/d...
test/mjsunit/harmony/destructuring.js:752: });
Better call this function!

https://codereview.chromium.org/1300103005/diff/100001/test/mjsunit/harmony/d...
test/mjsunit/harmony/destructuring.js:759: });
Same here

https://codereview.chromium.org/1300103005/diff/100001/test/mjsunit/harmony/d...
test/mjsunit/harmony/destructuring.js:790: });
And here

https://codereview.chromium.org/1300103005/diff/100001/test/mjsunit/harmony/r...
File test/mjsunit/harmony/rest-params.js (right):

https://codereview.chromium.org/1300103005/diff/100001/test/mjsunit/harmony/r...
test/mjsunit/harmony/rest-params.js:149: "use strict";
This changes the test. Make sure that the caller remains in sloppy mode.

[conradw, 2015-08-25 11:31:21]

https://codereview.chromium.org/1300103005/diff/100001/src/expression-classif...
File src/expression-classifier.h (right):

https://codereview.chromium.org/1300103005/diff/100001/src/expression-classif...
src/expression-classifier.h:235: // parameter
On 2015/08/24 13:11:08, rossberg wrote:
> Nit: period

Done.

https://codereview.chromium.org/1300103005/diff/100001/src/preparser.cc
File src/preparser.cc (right):

https://codereview.chromium.org/1300103005/diff/100001/src/preparser.cc#newco...
src/preparser.cc:270: if (!scope_->HasSimpleParameters()) {
On 2015/08/24 13:11:08, rossberg wrote:
> Nit: merge conditions

Done.

https://codereview.chromium.org/1300103005/diff/100001/src/preparser.cc#newco...
src/preparser.cc:271: // A block declaration scope as a child scope of a
function scope
On 2015/08/24 13:11:08, rossberg wrote:
> Not sure how the comment relates to the code. Which block scope?

I think this is referring to HasSimpleParameters looking up the flag in the
ClosureScope. I've reworded it anyway.

https://codereview.chromium.org/1300103005/diff/100001/src/preparser.h
File src/preparser.h (right):

https://codereview.chromium.org/1300103005/diff/100001/src/preparser.h#newcod...
src/preparser.h:2860: bool is_simple =
arrow_formals_classifier.is_simple_parameter_list();
On 2015/08/24 13:11:08, rossberg wrote:
> Nit: avoid the extra variable by storing to parameters.is_simple directly.

Done.

https://codereview.chromium.org/1300103005/diff/100001/test/cctest/test-parsi...
File test/cctest/test-parsing.cc (right):

https://codereview.chromium.org/1300103005/diff/100001/test/cctest/test-parsi...
test/cctest/test-parsing.cc:6885: // A block declaration scope as a child scope
of a function scope
On 2015/08/24 13:11:08, rossberg wrote:
> Same here.

Done.

https://codereview.chromium.org/1300103005/diff/100001/test/mjsunit/harmony/d...
File test/mjsunit/harmony/destructuring.js (right):

https://codereview.chromium.org/1300103005/diff/100001/test/mjsunit/harmony/d...
test/mjsunit/harmony/destructuring.js:752: });
On 2015/08/24 13:11:08, rossberg wrote:
> Better call this function!

Done.

https://codereview.chromium.org/1300103005/diff/100001/test/mjsunit/harmony/d...
test/mjsunit/harmony/destructuring.js:759: });
On 2015/08/24 13:11:08, rossberg wrote:
> Same here

Done.

https://codereview.chromium.org/1300103005/diff/100001/test/mjsunit/harmony/d...
test/mjsunit/harmony/destructuring.js:790: });
On 2015/08/24 13:11:08, rossberg wrote:
> And here

Done.

https://codereview.chromium.org/1300103005/diff/100001/test/mjsunit/harmony/r...
File test/mjsunit/harmony/rest-params.js (right):

https://codereview.chromium.org/1300103005/diff/100001/test/mjsunit/harmony/r...
test/mjsunit/harmony/rest-params.js:149: "use strict";
On 2015/08/24 13:11:08, rossberg wrote:
> This changes the test. Make sure that the caller remains in sloppy mode.

Done.

[rossberg, 2015-08-25 12:35:32]

lgtm

[conradw, 2015-08-25 12:36:06]

On 2015/08/25 12:35:32, rossberg wrote:
> lgtm

this will need to wait for the other arrow stuff to be relanded, however

[conradw, 2015-08-26 09:41:37]

The CQ bit was checked by conradw@chromium.org

[commit-bot: I haz the power, 2015-08-26 09:41:44]

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

[commit-bot: I haz the power, 2015-08-26 09:42:38]

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

[commit-bot: I haz the power, 2015-08-26 09:42:38]

Try jobs failed on following builders:
  v8_linux64_asan_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel/builds/...)
  v8_linux_arm_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel/builds/7405)
  v8_linux_dbg on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg/builds/8019)
  v8_linux_gcc_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)
  v8_linux_mipsel_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...)
  v8_linux_nodcheck_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel/build...)
  v8_linux_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel/builds/8955)
  v8_mac_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel/builds/9079)
  v8_presubmit on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/5231)

[conradw, 2015-08-26 13:56:04]

The CQ bit was checked by conradw@chromium.org

[conradw, 2015-08-26 13:56:04]

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

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

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

[commit-bot: I haz the power, 2015-08-26 14:09:03]

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

[commit-bot: I haz the power, 2015-08-26 14:09:04]

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

[conradw, 2015-08-26 14:50:13]

The CQ bit was checked by conradw@chromium.org

[conradw, 2015-08-26 14:50:13]

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/1300103005/#ps180001
(title: "flag implications don't work in test suite?")

[commit-bot: I haz the power, 2015-08-26 14:50:19]

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

[commit-bot: I haz the power, 2015-08-26 14:59:10]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2015-08-26 14:59:28]

Patchset 10 (id:??) landed as
https://crrev.com/77394fa05a63a539ac4e6858d99cc85ec6867512
Cr-Commit-Position: refs/heads/master@{#30388}