[parser] treat MethodDefinitions in ObjectPatterns as SyntaxErrors

[parser] treat MethodDefinitions in ObjectPatterns as SyntaxErrors

BUG=v8:4585
LOG=N
R=adamk@chromium.org, rossberg@chromium.org

Committed: https://crrev.com/5058f68596bbaf460d21d7c20fd3ad684659a07d
Cr-Commit-Position: refs/heads/master@{#32477}

[caitp (gmail), 2015-12-01 13:08:32]

caitpotter88@gmail.com changed reviewers:
+ mstarzinger@chromium.org

[caitp (gmail), 2015-12-01 13:08:32]

quick fix for anyone interested

[adamk, 2015-12-01 18:27:57]

Thanks for the fix! This was, apparently, totally busted. Just a few comments on
testing.

https://codereview.chromium.org/1488043002/diff/1/test/mjsunit/harmony/regres...
File test/mjsunit/harmony/regress/regress-4585.js (right):

https://codereview.chromium.org/1488043002/diff/1/test/mjsunit/harmony/regres...
test/mjsunit/harmony/regress/regress-4585.js:7: assertThrows(`for(const {
function(){} } = this) {}`, SyntaxError);
Can you put these tests in test-parsing instead? That'll give us coverage of the
preparser as well.

https://codereview.chromium.org/1488043002/diff/1/test/mjsunit/harmony/regres...
test/mjsunit/harmony/regress/regress-4585.js:8: assertThrows(`var { function()
{} } = this;`, SyntaxError);
Also can you use 'method' instead of 'function'? I realize they parse the same,
but it makes the test a little confusing (looks like a function literal in a
block).

https://codereview.chromium.org/1488043002/diff/1/test/mjsunit/harmony/regres...
test/mjsunit/harmony/regress/regress-4585.js:9: assertThrows(`for(var { get
foo() {} } = this) {}`, SyntaxError);
Also add a test for 'set foo(arg)'

[caitp (gmail), 2015-12-01 19:23:39]

On 2015/12/01 18:27:57, adamk wrote:
> Thanks for the fix! This was, apparently, totally busted. Just a few comments
on
> testing.
> 
>
https://codereview.chromium.org/1488043002/diff/1/test/mjsunit/harmony/regres...
> File test/mjsunit/harmony/regress/regress-4585.js (right):
> 
>
https://codereview.chromium.org/1488043002/diff/1/test/mjsunit/harmony/regres...
> test/mjsunit/harmony/regress/regress-4585.js:7: assertThrows(`for(const {
> function(){} } = this) {}`, SyntaxError);
> Can you put these tests in test-parsing instead? That'll give us coverage of
the
> preparser as well.
> 
>
https://codereview.chromium.org/1488043002/diff/1/test/mjsunit/harmony/regres...
> test/mjsunit/harmony/regress/regress-4585.js:8: assertThrows(`var { function()
> {} } = this;`, SyntaxError);
> Also can you use 'method' instead of 'function'? I realize they parse the
same,
> but it makes the test a little confusing (looks like a function literal in a
> block).
> 
>
https://codereview.chromium.org/1488043002/diff/1/test/mjsunit/harmony/regres...
> test/mjsunit/harmony/regress/regress-4585.js:9: assertThrows(`for(var { get
> foo() {} } = this) {}`, SyntaxError);
> Also add a test for 'set foo(arg)'

Added test-parsing test cases as well

[adamk, 2015-12-01 19:28:11]

lgtm

[caitp (gmail), 2015-12-01 19:30:43]

The CQ bit was checked by caitpotter88@gmail.com

[commit-bot: I haz the power, 2015-12-01 19:30:53]

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

[commit-bot: I haz the power, 2015-12-01 20:33:05]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-12-01 20:33:18]

Description was changed from

==========
[parser] treat MethodDefinitions in ObjectPatterns as SyntaxErrors

BUG=v8:4585
LOG=N
R=adamk@chromium.org, rossberg@chromium.org
==========

to

==========
[parser] treat MethodDefinitions in ObjectPatterns as SyntaxErrors

BUG=v8:4585
LOG=N
R=adamk@chromium.org, rossberg@chromium.org

Committed: https://crrev.com/5058f68596bbaf460d21d7c20fd3ad684659a07d
Cr-Commit-Position: refs/heads/master@{#32477}
==========

[commit-bot: I haz the power, 2015-12-01 20:33:19]

Patchset 2 (id:??) landed as
https://crrev.com/5058f68596bbaf460d21d7c20fd3ad684659a07d
Cr-Commit-Position: refs/heads/master@{#32477}