new classes: change semantics of super(...) call and add new.target to construct stub.

new classes: change semantics of super(...) call and add new.target to construct stub.

R=arv@chromium.org,rossberg@chromium.org
BUG=v8:3834
LOG=N

Committed: https://crrev.com/22ce08ade63a9c4f28225db11a5b5d2726c5a8e7
Cr-Commit-Position: refs/heads/master@{#26227}

[Dmitry Lomov (no reviews), 2015-01-21 15:56:20]

dslomov@chromium.org changed reviewers:
+ arv@chromium.org
- arv@chromiun.org

[Dmitry Lomov (no reviews), 2015-01-21 18:23:27]

PTAL. 
Carved out something landable from my experiments.

This is a first step, there is more to come

[arv (Not doing code reviews), 2015-01-21 19:18:56]

https://codereview.chromium.org/803933008/diff/20001/src/ia32/builtins-ia32.cc
File src/ia32/builtins-ia32.cc (right):

https://codereview.chromium.org/803933008/diff/20001/src/ia32/builtins-ia32.c...
src/ia32/builtins-ia32.cc:162: // runtime call made it for us, and we shouldn't
do create count
this sentence could use some polish

https://codereview.chromium.org/803933008/diff/20001/src/ia32/full-codegen-ia...
File src/ia32/full-codegen-ia32.cc (right):

https://codereview.chromium.org/803933008/diff/20001/src/ia32/full-codegen-ia...
src/ia32/full-codegen-ia32.cc:3144: 
remove one empty line

https://codereview.chromium.org/803933008/diff/20001/src/ia32/full-codegen-ia...
src/ia32/full-codegen-ia32.cc:3163: CHECK(false);
UNREACHABLE()

https://codereview.chromium.org/803933008/diff/20001/src/ia32/full-codegen-ia...
src/ia32/full-codegen-ia32.cc:3165:
EnsureSlotContainsAllocationSite(expr->AllocationSiteFeedbackSlot());
TODO

https://codereview.chromium.org/803933008/diff/20001/src/ia32/full-codegen-ia...
src/ia32/full-codegen-ia32.cc:3174: CallConstructStub stub(isolate(),
RECORD_CONSTRUCTOR_TARGET);
It is not clear to me where NewTarget is pushed on the stack?

https://codereview.chromium.org/803933008/diff/20001/src/ia32/full-codegen-ia...
src/ia32/full-codegen-ia32.cc:3180:
EmitVariableAssignment(super_ref->this_var()->var(), Token::INIT_LET);
Or INIT_CONST?

https://codereview.chromium.org/803933008/diff/20001/src/runtime/runtime-obje...
File src/runtime/runtime-object.cc (right):

https://codereview.chromium.org/803933008/diff/20001/src/runtime/runtime-obje...
src/runtime/runtime-object.cc:1350: CHECK(*constructor ==
*original_constructor);
Why are we passing two args here?

https://codereview.chromium.org/803933008/diff/20001/test/mjsunit/harmony/cla...
File test/mjsunit/harmony/classes-experimental.js (right):

https://codereview.chromium.org/803933008/diff/20001/test/mjsunit/harmony/cla...
test/mjsunit/harmony/classes-experimental.js:21: } catch(ReferenceError) {}
This does not do what you think it does.

Change it to:

var ex;
try {
  this.prp1 = 3;
} catch (e) {
  ex = e;
}
assertTrue(ex instanceof ReferenceError);

[Dmitry Lomov (no reviews), 2015-01-22 11:59:01]

All comments addressed + more polish.

https://codereview.chromium.org/803933008/diff/20001/src/ia32/builtins-ia32.cc
File src/ia32/builtins-ia32.cc (right):

https://codereview.chromium.org/803933008/diff/20001/src/ia32/builtins-ia32.c...
src/ia32/builtins-ia32.cc:162: // runtime call made it for us, and we shouldn't
do create count
On 2015/01/21 19:18:55, arv wrote:
> this sentence could use some polish

Done, also removed code duplication.

https://codereview.chromium.org/803933008/diff/20001/src/ia32/full-codegen-ia...
File src/ia32/full-codegen-ia32.cc (right):

https://codereview.chromium.org/803933008/diff/20001/src/ia32/full-codegen-ia...
src/ia32/full-codegen-ia32.cc:3144: 
On 2015/01/21 19:18:55, arv wrote:
> remove one empty line

Done.

https://codereview.chromium.org/803933008/diff/20001/src/ia32/full-codegen-ia...
src/ia32/full-codegen-ia32.cc:3163: CHECK(false);
On 2015/01/21 19:18:56, arv wrote:
> UNREACHABLE()

Done.

https://codereview.chromium.org/803933008/diff/20001/src/ia32/full-codegen-ia...
src/ia32/full-codegen-ia32.cc:3165:
EnsureSlotContainsAllocationSite(expr->AllocationSiteFeedbackSlot());
On 2015/01/21 19:18:56, arv wrote:
> TODO

Done.

https://codereview.chromium.org/803933008/diff/20001/src/ia32/full-codegen-ia...
src/ia32/full-codegen-ia32.cc:3174: CallConstructStub stub(isolate(),
RECORD_CONSTRUCTOR_TARGET);
On 2015/01/21 19:18:55, arv wrote:
> It is not clear to me where NewTarget is pushed on the stack?

Not yet, added a todo.

https://codereview.chromium.org/803933008/diff/20001/src/ia32/full-codegen-ia...
src/ia32/full-codegen-ia32.cc:3180:
EmitVariableAssignment(super_ref->this_var()->var(), Token::INIT_LET);
On 2015/01/21 19:18:56, arv wrote:
> Or INIT_CONST?

Added a TODO to revisit this after TDZ is implemented

https://codereview.chromium.org/803933008/diff/20001/src/runtime/runtime-obje...
File src/runtime/runtime-object.cc (right):

https://codereview.chromium.org/803933008/diff/20001/src/runtime/runtime-obje...
src/runtime/runtime-object.cc:1350: CHECK(*constructor ==
*original_constructor);
On 2015/01/21 19:18:56, arv wrote:
> Why are we passing two args here?

We will need both args once prototype rewiring is implenmented.
Added comment and TODO.

https://codereview.chromium.org/803933008/diff/20001/test/mjsunit/harmony/cla...
File test/mjsunit/harmony/classes-experimental.js (right):

https://codereview.chromium.org/803933008/diff/20001/test/mjsunit/harmony/cla...
test/mjsunit/harmony/classes-experimental.js:21: } catch(ReferenceError) {}
On 2015/01/21 19:18:56, arv wrote:
> This does not do what you think it does.
> 
> Change it to:
> 
> var ex;
> try {
>   this.prp1 = 3;
> } catch (e) {
>   ex = e;
> }
> assertTrue(ex instanceof ReferenceError);

Right! Changed code, added a TODO to actually test when this is implemented

[arv (Not doing code reviews), 2015-01-22 16:20:21]

LGTM with nits

Maybe the bug number should only be v8:3834 and then make v8:3330 depend on
v8:3834 instead.

https://codereview.chromium.org/803933008/diff/80001/src/x64/full-codegen-x64.cc
File src/x64/full-codegen-x64.cc (right):

https://codereview.chromium.org/803933008/diff/80001/src/x64/full-codegen-x64...
src/x64/full-codegen-x64.cc:3164: /* TODO[dslomov]: support pretenuring.
The style guide says to format these as: `TODO(dslomov):`

https://codereview.chromium.org/803933008/diff/80001/test/mjsunit/harmony/cla...
File test/mjsunit/harmony/classes-experimental.js (right):

https://codereview.chromium.org/803933008/diff/80001/test/mjsunit/harmony/cla...
test/mjsunit/harmony/classes-experimental.js:21: } catch(e) {
} catch (e) {

space after `catch`, just like `if`, `else`, `for` etc.

[arv (Not doing code reviews), 2015-01-22 16:20:26]

LGTM with nits

Maybe the bug number should only be v8:3834 and then make v8:3330 depend on
v8:3834 instead.

https://codereview.chromium.org/803933008/diff/80001/src/x64/full-codegen-x64.cc
File src/x64/full-codegen-x64.cc (right):

https://codereview.chromium.org/803933008/diff/80001/src/x64/full-codegen-x64...
src/x64/full-codegen-x64.cc:3164: /* TODO[dslomov]: support pretenuring.
The style guide says to format these as: `TODO(dslomov):`

https://codereview.chromium.org/803933008/diff/80001/test/mjsunit/harmony/cla...
File test/mjsunit/harmony/classes-experimental.js (right):

https://codereview.chromium.org/803933008/diff/80001/test/mjsunit/harmony/cla...
test/mjsunit/harmony/classes-experimental.js:21: } catch(e) {
} catch (e) {

space after `catch`, just like `if`, `else`, `for` etc.

[Dmitry Lomov (no reviews), 2015-01-22 17:00:26]

All done, landing.

https://codereview.chromium.org/803933008/diff/80001/src/x64/full-codegen-x64.cc
File src/x64/full-codegen-x64.cc (right):

https://codereview.chromium.org/803933008/diff/80001/src/x64/full-codegen-x64...
src/x64/full-codegen-x64.cc:3164: /* TODO[dslomov]: support pretenuring.
On 2015/01/22 16:20:26, arv wrote:
> The style guide says to format these as: `TODO(dslomov):`

Done (other places too)

https://codereview.chromium.org/803933008/diff/80001/test/mjsunit/harmony/cla...
File test/mjsunit/harmony/classes-experimental.js (right):

https://codereview.chromium.org/803933008/diff/80001/test/mjsunit/harmony/cla...
test/mjsunit/harmony/classes-experimental.js:21: } catch(e) {
On 2015/01/22 16:20:26, arv wrote:
> } catch (e) {
> 
> space after `catch`, just like `if`, `else`, `for` etc.

Done.

[Dmitry Lomov (no reviews), 2015-01-22 17:00:30]

The CQ bit was checked by dslomov@chromium.org

[commit-bot: I haz the power, 2015-01-22 17:01:14]

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

[commit-bot: I haz the power, 2015-01-22 17:40:47]

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

[commit-bot: I haz the power, 2015-01-22 17:40:47]

Try jobs failed on following builders:
  v8_linux64_asan_rel on tryserver.v8
(http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel/builds/834)

[Dmitry Lomov (no reviews), 2015-01-22 17:52:50]

Fixed x64 issue: use rdx instead of r11 which can be clobbered by heap profiler

[arv (Not doing code reviews), 2015-01-22 17:56:53]

LGTM

[Dmitry Lomov (no reviews), 2015-01-22 18:01:48]

The CQ bit was checked by dslomov@chromium.org

[commit-bot: I haz the power, 2015-01-22 18:02:25]

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

[commit-bot: I haz the power, 2015-01-22 18:39:41]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-01-22 18:39:59]

Patchset 7 (id:??) landed as
https://crrev.com/22ce08ade63a9c4f28225db11a5b5d2726c5a8e7
Cr-Commit-Position: refs/heads/master@{#26227}