Move object literal checking into checker classes

Move object literal checking into checker classes

This removes the duplicate property check from object literals.

Instead we repurpose the ObjectLiteralChecker into two cases, implemented
by two subclasses to ObjectLiteralCheckerBase called ObjectLiteralChecker
and ClassLiteralChecker.

The object literal checker now only checks for duplicate __proto__ fields in
object literals.

The class literal checker checks for duplicate constructors, non constructor
fields named constructor as well as static properties named prototype.

BUG=v8:3819
LOG=Y
R=adamk, dslomov@chromium.org

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

[arv (Not doing code reviews), 2015-01-23 23:41:08]

Duplicate __proto__ is no longer allowed so no need to special case those in
CalculateEmitStore

[arv (Not doing code reviews), 2015-01-23 23:56:43]

Remove branch from codegens now that PROTOTYPE is always emit_store

[arv (Not doing code reviews), 2015-01-24 00:03:15]

Use variables

[arv (Not doing code reviews), 2015-01-24 00:03:56]

PTAL

Now that duplicate __proto__ is a syntax error it allows some simplifications in
the codegen.

[adamk, 2015-01-24 00:18:07]

https://codereview.chromium.org/873823003/diff/60001/src/preparser.h
File src/preparser.h (right):

https://codereview.chromium.org/873823003/diff/60001/src/preparser.h#newcode3003
src/preparser.h:3003: if (in_class_) {
Did you consider having two separate checkers, one for classes and one for
objects? Very little of the logic seems to be shared...

[arv (Not doing code reviews), 2015-01-27 00:36:03]

Split the impl of the checker into 2 classes

[adamk, 2015-01-27 20:05:48]

Looking good. Please update the CL description to match the new implementation
(with two subclasses).

What's the state of duplicate property checking across engines? It's hard to
imagine actual code that depends on this behavior, but the number of test
changes involved does worry me a little bit about compatibility.

https://codereview.chromium.org/873823003/diff/80001/src/preparser.h
File src/preparser.h (left):

https://codereview.chromium.org/873823003/diff/80001/src/preparser.h#oldcode3056
src/preparser.h:3056: parser()->ReportMessage("strict_duplicate_property");
If this message is now unused, you ought to be able to remove it (same goes
below).

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

https://codereview.chromium.org/873823003/diff/80001/src/preparser.h#newcode546
src/preparser.h:546: kMethodProperty = 3
Given that these are no longer bit patterns, you should be able to leave off the
values and let the compiler assign them.

[arv (Not doing code reviews), 2015-01-27 21:45:59]

Remove unused error messages

[arv (Not doing code reviews), 2015-01-28 02:07:47]

git rebase

[arv (Not doing code reviews), 2015-01-28 02:36:53]

Fix assert

[arv (Not doing code reviews), 2015-01-28 02:37:15]

PTAL

https://codereview.chromium.org/873823003/diff/80001/src/preparser.h
File src/preparser.h (left):

https://codereview.chromium.org/873823003/diff/80001/src/preparser.h#oldcode3056
src/preparser.h:3056: parser()->ReportMessage("strict_duplicate_property");
On 2015/01/27 20:05:48, adamk wrote:
> If this message is now unused, you ought to be able to remove it (same goes
> below).

Done.

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

https://codereview.chromium.org/873823003/diff/80001/src/preparser.h#newcode546
src/preparser.h:546: kMethodProperty = 3
On 2015/01/27 20:05:48, adamk wrote:
> Given that these are no longer bit patterns, you should be able to leave off
the
> values and let the compiler assign them.

Done.

[adamk, 2015-01-28 17:57:26]

Code looks good to me, but can you address my question up-thread about
compatibility before landing?

[arv (Not doing code reviews), 2015-01-28 18:18:45]

On 2015/01/28 17:57:26, adamk wrote:
> Code looks good to me, but can you address my question up-thread about
> compatibility before landing?

Firefox removed their duplicate property check a while ago. They also enforce
that they do not have duplicate __proto__ properties.

I talked to MS and they have removed the duplicate property check but they still
do not throw an error for duplicate __proto__.

JSC is still using the ES5 semantics.

[adamk, 2015-01-28 18:19:30]

On 2015/01/28 18:18:45, arv wrote:
> On 2015/01/28 17:57:26, adamk wrote:
> > Code looks good to me, but can you address my question up-thread about
> > compatibility before landing?
> 
> Firefox removed their duplicate property check a while ago. They also enforce
> that they do not have duplicate __proto__ properties.
> 
> I talked to MS and they have removed the duplicate property check but they
still
> do not throw an error for duplicate __proto__.
> 
> JSC is still using the ES5 semantics.

lgtm

Sounds like we're in the middle of the pack, so I feel comfortable with this.

[arv (Not doing code reviews), 2015-01-29 17:39:33]

On 2015/01/28 18:19:30, adamk wrote:
> On 2015/01/28 18:18:45, arv wrote:
> > On 2015/01/28 17:57:26, adamk wrote:
> > > Code looks good to me, but can you address my question up-thread about
> > > compatibility before landing?
> > 
> > Firefox removed their duplicate property check a while ago. They also
enforce
> > that they do not have duplicate __proto__ properties.
> > 
> > I talked to MS and they have removed the duplicate property check but they
> still
> > do not throw an error for duplicate __proto__.
> > 
> > JSC is still using the ES5 semantics.
> 
> lgtm
> 
> Sounds like we're in the middle of the pack, so I feel comfortable with this.

I'm not clear if I reenabled the Blink layout tests when this was reverted a
couple of months ago. I'll try to if the new try bot support will allow me to
test this on Blink.

[arv (Not doing code reviews), 2015-01-29 23:10:27]

The CQ bit was checked by arv@chromium.org

[commit-bot: I haz the power, 2015-01-29 23:10:43]

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

[commit-bot: I haz the power, 2015-01-29 23:12:36]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2015-01-29 23:12:56]

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