[es6] support `get` and `set` in shorthand properties

[es6] support `get` and `set` in shorthand properties

Add support for `get` and `set` as shorthand properties. Also
supports them for CoverInitializedName in BindingPatterns and (once implemented)
AssignmentPatterns.

BUG=v8:4412, v8:3584
LOG=N
R=adamk, aperez, wingo, rossberg

Committed: https://crrev.com/b444da41ad9ef32cea08fd24fd89eab258cd1360
Cr-Commit-Position: refs/heads/master@{#30769}

[caitp (gmail), 2015-09-07 01:43:26]

caitpotter88@gmail.com changed reviewers:
+ adamk@chromium.org, aperez@igalia.com

[caitp (gmail), 2015-09-07 01:45:25]

PTAL, quick bugfix for reported issue with shorthand properties. Super simple
stuff.

[caitp (gmail), 2015-09-07 03:14:10]

Patchset #4 (id:60001) has been deleted

[caitp (gmail), 2015-09-07 03:16:17]

https://codereview.chromium.org/1328083002/diff/80001/src/preparser.h
File src/preparser.h (right):

https://codereview.chromium.org/1328083002/diff/80001/src/preparser.h#newcode...
src/preparser.h:2614: if (!in_class && !is_generator) {
I reorganized ParsePropertyDefinition because it was getting extremely confusing
and hard to follow. I think this version is easier to understand what's
happening in, but it's just one option of several for fixing this bug, I guess

[adamk, 2015-09-08 23:17:17]

I agree that some refactoring here would be helpful for readability, but I'm not
sure the switch statement you landed on was it. So I'd suggest going with a
minimal fix and dealing with the refactoring separately.

https://codereview.chromium.org/1328083002/diff/40001/src/preparser.h
File src/preparser.h (right):

https://codereview.chromium.org/1328083002/diff/40001/src/preparser.h#newcode...
src/preparser.h:2663: goto shorthand_property;
Can't you just do without the goto?

} else if ((is_get || is_set) &&
           (in_class ||
            (peek() != Token::ASSIGN &&
             peek() != Token::COMMA &&
             peek() != Token::RBRACE)) {


that'll fall through to the next else/if.

https://codereview.chromium.org/1328083002/diff/80001/src/preparser.h
File src/preparser.h (right):

https://codereview.chromium.org/1328083002/diff/80001/src/preparser.h#newcode...
src/preparser.h:2641: break;
This break makes this whole approach not much easier to read (for me) than the
old version.

[caitp (gmail), 2015-09-08 23:35:13]

Sounds good to me. I'll have a patchset later tonight

https://codereview.chromium.org/1328083002/diff/40001/src/preparser.h
File src/preparser.h (right):

https://codereview.chromium.org/1328083002/diff/40001/src/preparser.h#newcode...
src/preparser.h:2663: goto shorthand_property;
On 2015/09/08 23:17:17, adamk wrote:
> Can't you just do without the goto?
> 
> } else if ((is_get || is_set) &&
>            (in_class ||
>             (peek() != Token::ASSIGN &&
>              peek() != Token::COMMA &&
>              peek() != Token::RBRACE)) {
> 
> 
> that'll fall through to the next else/if.

That's true. I think this is still a confusing hack which mixes up the different
productions, but it seems like a reasonable way to split off from the
refactoring. I'll do that.

https://codereview.chromium.org/1328083002/diff/80001/src/preparser.h
File src/preparser.h (right):

https://codereview.chromium.org/1328083002/diff/80001/src/preparser.h#newcode...
src/preparser.h:2641: break;
On 2015/09/08 23:17:17, adamk wrote:
> This break makes this whole approach not much easier to read (for me) than the
> old version.

It can work the other way, just makes the invention kind of bad

[caitp (gmail), 2015-09-16 15:21:49]

On 2015/09/08 23:35:13, caitp wrote:
> Sounds good to me. I'll have a patchset later tonight
> 
> https://codereview.chromium.org/1328083002/diff/40001/src/preparser.h
> File src/preparser.h (right):
> 
>
https://codereview.chromium.org/1328083002/diff/40001/src/preparser.h#newcode...
> src/preparser.h:2663: goto shorthand_property;
> On 2015/09/08 23:17:17, adamk wrote:
> > Can't you just do without the goto?
> > 
> > } else if ((is_get || is_set) &&
> >            (in_class ||
> >             (peek() != Token::ASSIGN &&
> >              peek() != Token::COMMA &&
> >              peek() != Token::RBRACE)) {
> > 
> > 
> > that'll fall through to the next else/if.
> 
> That's true. I think this is still a confusing hack which mixes up the
different
> productions, but it seems like a reasonable way to split off from the
> refactoring. I'll do that.
> 
> https://codereview.chromium.org/1328083002/diff/80001/src/preparser.h
> File src/preparser.h (right):
> 
>
https://codereview.chromium.org/1328083002/diff/80001/src/preparser.h#newcode...
> src/preparser.h:2641: break;
> On 2015/09/08 23:17:17, adamk wrote:
> > This break makes this whole approach not much easier to read (for me) than
the
> > old version.
> 
> It can work the other way, just makes the invention kind of bad

Ping, this was a really simple fix, it would be good to land it =)

[adamk, 2015-09-16 15:25:35]

lgtm

Sorry for the delay, I was expecting a ping from your end when you uploaded a
new patch.

[caitp (gmail), 2015-09-16 15:35:15]

The CQ bit was checked by caitpotter88@gmail.com

[caitp (gmail), 2015-09-16 15:35:15]

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/1328083002/#ps160001
(title: "Rebase")

[commit-bot: I haz the power, 2015-09-16 15:35:28]

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

[caitp (gmail), 2015-09-16 15:43:20]

On 2015/09/16 15:25:35, adamk wrote:
> lgtm
> 
> Sorry for the delay, I was expecting a ping from your end when you uploaded a
> new patch.

My bad, maybe I'm too used to Bugzilla reporting uploaded attachments by email
automatically, by default.

[commit-bot: I haz the power, 2015-09-16 16:01:55]

Committed patchset #8 (id:160001)

[commit-bot: I haz the power, 2015-09-16 16:02:15]

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