[es6] Throw TypeError for computed static prototype property name

[es6] Throw TypeError for computed static prototype property name

The prototype of a class constructor function is read only. When we set
computed property names we were ignoring this and we were overriding the
property.

Since the prototype is the only possible own read only property on the
constructor function object we special case this so we do not have to
check this for every property in the class literal.

BUG=v8:3945
LOG=N
R=mstarzinger@chromium.org, dslomov@chromium.org

Committed: https://crrev.com/8d946b9c3f6ea42dd5232c0529be4d47798b06aa
Cr-Commit-Position: refs/heads/master@{#27106}

[arv (Not doing code reviews), 2015-03-06 11:50:11]

Cleanup comment

[arv (Not doing code reviews), 2015-03-06 11:51:29]

PTAL

Other ports will follow if this seems like a good approach.

https://codereview.chromium.org/985643003/diff/20001/src/compiler/ast-graph-b...
File src/compiler/ast-graph-builder.cc (right):

https://codereview.chromium.org/985643003/diff/20001/src/compiler/ast-graph-b...
src/compiler/ast-graph-builder.cc:1394:
javascript()->CallRuntime(Runtime::kThrowIfStaticPrototype, 1);
I'm tempted to do the `if (name === 'prototype') { ... }` here instead of doing
that in the runtime function.

[Michael Starzinger, 2015-03-06 13:05:19]

https://codereview.chromium.org/985643003/diff/20001/src/compiler/ast-graph-b...
File src/compiler/ast-graph-builder.cc (right):

https://codereview.chromium.org/985643003/diff/20001/src/compiler/ast-graph-b...
src/compiler/ast-graph-builder.cc:1394:
javascript()->CallRuntime(Runtime::kThrowIfStaticPrototype, 1);
On 2015/03/06 11:51:29, arv wrote:
> I'm tempted to do the `if (name === 'prototype') { ... }` here instead of
doing
> that in the runtime function.

I would be fine with having the comparison inline if you like to go for it. You
could use JSStrictEqual and the IfBuilder to construct that. Check out
AstGraphBuilder::BuildStackCheck for inspiration on how this might look like.

[caitp (gmail), 2015-03-06 13:52:13]

The error is that the `prototype` property is set up with writable=false,
configurable=false properties --- so the expected error is either
`strict_cannot_assign` or `strict_read_only_property`, and shouldn't affect
sloppy mode once the construct is available in sloppy mode.

I think this could be fixed just by making sure that when defining class
properties, property attributes are not ignored.

[arv (Not doing code reviews), 2015-03-06 16:10:04]

On 2015/03/06 13:52:13, caitp wrote:
> The error is that the `prototype` property is set up with writable=false,
> configurable=false properties --- so the expected error is either
> `strict_cannot_assign` or `strict_read_only_property`, and shouldn't affect
> sloppy mode once the construct is available in sloppy mode.
> 
> I think this could be fixed just by making sure that when defining class
> properties, property attributes are not ignored.

Yes. That was my first thought too. But since the only non configurable own
property is the prototype property there is no need to check the common cases on
non computed, non static properties.

[arv (Not doing code reviews), 2015-03-09 14:28:13]

I see I didn't reply to this part

On 2015/03/06 13:52:13, caitp wrote:
> and shouldn't affect
> sloppy mode once the construct is available in sloppy mode.

The entire class is always strict (extends expressions, computed property name
expressions, methods ...).

[arv (Not doing code reviews), 2015-03-09 14:28:30]

Dmitry, Any comments?

[caitp (gmail), 2015-03-09 14:31:00]

On 2015/03/09 14:28:13, arv wrote:
> I see I didn't reply to this part
> 
> On 2015/03/06 13:52:13, caitp wrote:
> > and shouldn't affect
> > sloppy mode once the construct is available in sloppy mode.
> 
> The entire class is always strict (extends expressions, computed property name
> expressions, methods ...).

Yeah you're right, I guess 10.2.1 spells that out pretty clearly

[Dmitry Lomov (no reviews), 2015-03-09 14:46:04]

https://codereview.chromium.org/985643003/diff/20001/src/compiler/ast-graph-b...
File src/compiler/ast-graph-builder.cc (right):

https://codereview.chromium.org/985643003/diff/20001/src/compiler/ast-graph-b...
src/compiler/ast-graph-builder.cc:1394:
javascript()->CallRuntime(Runtime::kThrowIfStaticPrototype, 1);
On 2015/03/06 13:05:18, Michael Starzinger wrote:
> On 2015/03/06 11:51:29, arv wrote:
> > I'm tempted to do the `if (name === 'prototype') { ... }` here instead of
> doing
> > that in the runtime function.
> 
> I would be fine with having the comparison inline if you like to go for it.
You
> could use JSStrictEqual and the IfBuilder to construct that. Check out
> AstGraphBuilder::BuildStackCheck for inspiration on how this might look like.

I think inline comparison is better overall since it is one runtime call less on
happy path. But ok either way.

[arv (Not doing code reviews), 2015-03-09 16:18:05]

Handle branching in TF

[arv (Not doing code reviews), 2015-03-09 16:29:39]

Ports done

[arv (Not doing code reviews), 2015-03-09 16:34:28]

Inline BuildThrowStaticPrototype

[arv (Not doing code reviews), 2015-03-09 16:37:03]

PTAL

I added the other ports and implemented the branching in TurboFan.

[Michael Starzinger, 2015-03-09 20:54:06]

LGTM on the TurboFan port, didn't look at the rest.

[Dmitry Lomov (no reviews), 2015-03-10 10:35:48]

lgtm

[arv (Not doing code reviews), 2015-03-10 13:57:05]

The CQ bit was checked by arv@chromium.org

[commit-bot: I haz the power, 2015-03-10 13:57:51]

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

[commit-bot: I haz the power, 2015-03-10 14:14:37]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-03-10 14:14:49]

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