Disallow generator declarations in certain locations

Disallow generator declarations in certain locations

The legacy function declaration locations from Annex B 3.2 and 3.4 do not
apply for generator declarations. This patch cracks down on those usages,
which is tested for by new incoming test262 tests.

BUG=v8:4824
LOG=Y
R=adamk

Committed: https://crrev.com/63b935428c29f60c35cf3a8234385c95251aea8a
Cr-Commit-Position: refs/heads/master@{#35835}

[Dan Ehrenberg, 2016-04-20 02:14:49]

The CQ bit was checked by littledan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-20 02:15:00]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1900033003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1900033003/1

[commit-bot: I haz the power, 2016-04-20 02:50:42]

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

[commit-bot: I haz the power, 2016-04-20 02:50:42]

Dry run: This issue passed the CQ dry run.

[adamk, 2016-04-20 19:09:28]

https://codereview.chromium.org/1900033003/diff/1/src/parsing/parser.cc
File src/parsing/parser.cc (left):

https://codereview.chromium.org/1900033003/diff/1/src/parsing/parser.cc#oldco...
src/parsing/parser.cc:2446: return ParseFunctionDeclaration(labels, ok);
Was this passing of labels just wrong?

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

https://codereview.chromium.org/1900033003/diff/1/src/parsing/parser.cc#newco...
src/parsing/parser.cc:2401: return ParseFunctionDeclaration(pos, is_generator,
NULL, CHECK_OK);
Nit: nullptr

https://codereview.chromium.org/1900033003/diff/1/src/parsing/parser.h
File src/parsing/parser.h (right):

https://codereview.chromium.org/1900033003/diff/1/src/parsing/parser.h#newcod...
src/parsing/parser.h:764: Statement* ParseFunctionDeclaration(ZoneList<const
AstRawString*>* names,
Perhaps these could be called "ParseHoistableDeclaration" instead of naming the
new thing "ParseOnlyFunctionDeclaration"?

https://codereview.chromium.org/1900033003/diff/1/src/parsing/preparser.cc
File src/parsing/preparser.cc (right):

https://codereview.chromium.org/1900033003/diff/1/src/parsing/preparser.cc#ne...
src/parsing/preparser.cc:568: pos, FunctionLiteral::kDeclaration,
This is a lot of boilerplate to copy; what if you split the existing
ParseFunctionDeclaration into two, as it is for the Parser? Also consider the
same renaming.

[Dan Ehrenberg, 2016-04-26 22:24:19]

https://codereview.chromium.org/1900033003/diff/1/src/parsing/parser.cc
File src/parsing/parser.cc (left):

https://codereview.chromium.org/1900033003/diff/1/src/parsing/parser.cc#oldco...
src/parsing/parser.cc:2446: return ParseFunctionDeclaration(labels, ok);
On 2016/04/20 at 19:09:28, adamk wrote:
> Was this passing of labels just wrong?

Actually, yes. We were confusing labels for exported names. Maybe this would've
been visible if you made a label as the same name as a function in a weird
place, or something.

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

https://codereview.chromium.org/1900033003/diff/1/src/parsing/parser.cc#newco...
src/parsing/parser.cc:2401: return ParseFunctionDeclaration(pos, is_generator,
NULL, CHECK_OK);
On 2016/04/20 at 19:09:28, adamk wrote:
> Nit: nullptr

Fixed

https://codereview.chromium.org/1900033003/diff/1/src/parsing/parser.h
File src/parsing/parser.h (right):

https://codereview.chromium.org/1900033003/diff/1/src/parsing/parser.h#newcod...
src/parsing/parser.h:764: Statement* ParseFunctionDeclaration(ZoneList<const
AstRawString*>* names,
On 2016/04/20 at 19:09:28, adamk wrote:
> Perhaps these could be called "ParseHoistableDeclaration" instead of naming
the new thing "ParseOnlyFunctionDeclaration"?

Done

https://codereview.chromium.org/1900033003/diff/1/src/parsing/preparser.cc
File src/parsing/preparser.cc (right):

https://codereview.chromium.org/1900033003/diff/1/src/parsing/preparser.cc#ne...
src/parsing/preparser.cc:568: pos, FunctionLiteral::kDeclaration,
On 2016/04/20 at 19:09:28, adamk wrote:
> This is a lot of boilerplate to copy; what if you split the existing
ParseFunctionDeclaration into two, as it is for the Parser? Also consider the
same renaming.

Done

[adamk, 2016-04-26 23:55:57]

https://codereview.chromium.org/1900033003/diff/20001/src/parsing/parser.cc
File src/parsing/parser.cc (right):

https://codereview.chromium.org/1900033003/diff/20001/src/parsing/parser.cc#n...
src/parsing/parser.cc:2390: Statement* Parser::ParseHoistableDeclaration(bool*
ok) {
I was suggesting that this should be called ParseFunctionDeclaration, sorry I
wasn't clear. Here's what I meant by my original comment:

ParseFunctionDeclaration -> ParseHoistableDeclaration
ParseOnlyFunctionDeclaration -> ParseFunctionDeclaration

[Dan Ehrenberg, 2016-04-27 00:18:11]

https://codereview.chromium.org/1900033003/diff/20001/src/parsing/parser.cc
File src/parsing/parser.cc (right):

https://codereview.chromium.org/1900033003/diff/20001/src/parsing/parser.cc#n...
src/parsing/parser.cc:2390: Statement* Parser::ParseHoistableDeclaration(bool*
ok) {
On 2016/04/26 at 23:55:57, adamk wrote:
> I was suggesting that this should be called ParseFunctionDeclaration, sorry I
wasn't clear. Here's what I meant by my original comment:
> 
> ParseFunctionDeclaration -> ParseHoistableDeclaration
> ParseOnlyFunctionDeclaration -> ParseFunctionDeclaration

No, you were clear; overzealous find/replace. Fixed.

[adamk, 2016-04-27 18:24:50]

lgtm

[Dan Ehrenberg, 2016-04-27 18:28:36]

The CQ bit was checked by littledan@chromium.org

[commit-bot: I haz the power, 2016-04-27 18:28:46]

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

[commit-bot: I haz the power, 2016-04-27 18:30:13]

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

[commit-bot: I haz the power, 2016-04-27 18:30:14]

Try jobs failed on following builders:
  v8_linux_arm64_rel_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...)
  v8_linux_mips64el_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...)
  v8_linux_nodcheck_rel_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng/bu...)

[Dan Ehrenberg, 2016-04-27 18:52:03]

The CQ bit was checked by littledan@chromium.org

[Dan Ehrenberg, 2016-04-27 18:52:04]

The patchset sent to the CQ was uploaded after l-g-t-m from adamk@chromium.org
Link to the patchset: https://codereview.chromium.org/1900033003/#ps60001
(title: "Rebase")

[commit-bot: I haz the power, 2016-04-27 18:52:08]

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

[commit-bot: I haz the power, 2016-04-27 19:18:20]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-05-01 22:01:31]

Description was changed from

==========
Disallow generator declarations in certain locations

The legacy function declaration locations from Annex B 3.2 and 3.4 do not
apply for generator declarations. This patch cracks down on those usages,
which is tested for by new incoming test262 tests.

BUG=v8:4824
LOG=Y
R=adamk
==========

to

==========
Disallow generator declarations in certain locations

The legacy function declaration locations from Annex B 3.2 and 3.4 do not
apply for generator declarations. This patch cracks down on those usages,
which is tested for by new incoming test262 tests.

BUG=v8:4824
LOG=Y
R=adamk

Committed: https://crrev.com/63b935428c29f60c35cf3a8234385c95251aea8a
Cr-Commit-Position: refs/heads/master@{#35835}
==========

[commit-bot: I haz the power, 2016-05-01 22:01:32]

Patchset 4 (id:??) landed as
https://crrev.com/63b935428c29f60c35cf3a8234385c95251aea8a
Cr-Commit-Position: refs/heads/master@{#35835}