Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1472)

Issue 1203813002: [es6] Make new.target work in functions (Closed)

Created:
5 years, 6 months ago by arv (Not doing code reviews)
Modified:
5 years, 5 months ago
CC:
v8-dev, v8-mips-ports_googlegroups.com
Base URL:
https://chromium.googlesource.com/v8/v8.git@master
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

[es6] Make new.target work in functions This makes new.target work in [[Call]] and [[Construct]] of ordinary functions. We achieve this by introducing a new construct stub for functions that uses the new.target variable. The construct stub pushes the original constructor just above the receiver in the construct frame. BUG=v8:3887 LOG=N R=adamk@chromium.org, dslomov@chromium.org Committed: https://crrev.com/7a63bf77eb7610afdc1a968f7660781e5160ba8d Cr-Commit-Position: refs/heads/master@{#29358}

Patch Set 1 #

Patch Set 2 : Add arrow tests #

Patch Set 3 : Trying to get arm and mips working #

Patch Set 4 : Fix push/pop issue on mips #

Patch Set 5 : Also adjust the create memento branch #

Total comments: 5

Patch Set 6 : All working #

Patch Set 7 : Cleanup #

Total comments: 1

Patch Set 8 : Fix issue with ia32 #

Patch Set 9 : Add test again. It got lost in last patchset #

Unified diffs Side-by-side diffs Delta from patch set Stats (+620 lines, -116 lines) Patch
M src/arm/builtins-arm.cc View 1 2 3 4 5 6 9 chunks +44 lines, -22 lines 0 comments Download
M src/arm64/builtins-arm64.cc View 1 2 3 4 5 6 8 chunks +43 lines, -18 lines 0 comments Download
M src/arm64/full-codegen-arm64.cc View 1 2 3 4 5 6 7 8 1 chunk +1 line, -1 line 0 comments Download
M src/builtins.h View 2 chunks +2 lines, -0 lines 0 comments Download
M src/compiler.cc View 1 2 3 4 5 6 7 8 1 chunk +5 lines, -0 lines 0 comments Download
M src/ia32/builtins-ia32.cc View 1 2 3 4 5 6 7 7 chunks +32 lines, -11 lines 0 comments Download
M src/mips/builtins-mips.cc View 1 2 3 4 5 6 9 chunks +41 lines, -20 lines 0 comments Download
M src/mips64/builtins-mips64.cc View 1 2 3 4 5 6 9 chunks +40 lines, -22 lines 0 comments Download
M src/scopes.cc View 1 2 3 4 5 6 7 8 1 chunk +8 lines, -12 lines 0 comments Download
M src/x64/builtins-x64.cc View 1 2 3 4 5 6 7 chunks +31 lines, -10 lines 0 comments Download
A test/mjsunit/harmony/new-target.js View 1 2 3 8 1 chunk +373 lines, -0 lines 0 comments Download

Messages

Total messages: 37 (8 generated)
arv (Not doing code reviews)
Add arrow tests
5 years, 6 months ago (2015-06-23 21:11:18 UTC) #1
arv (Not doing code reviews)
PTAL Any other edge cases that I should try at this point?
5 years, 6 months ago (2015-06-23 21:12:31 UTC) #2
adamk
I don't see any obvious things missing from your test cases, this looks ready to ...
5 years, 6 months ago (2015-06-23 21:40:17 UTC) #3
Dmitry Lomov (no reviews)
On 2015/06/23 21:40:17, adamk wrote: > I don't see any obvious things missing from your ...
5 years, 6 months ago (2015-06-24 09:50:34 UTC) #4
arv (Not doing code reviews)
Trying to get arm and mips working
5 years, 6 months ago (2015-06-25 17:08:52 UTC) #5
arv (Not doing code reviews)
Mips porters, I'm having some issues with both mips and mips64. It seems like this ...
5 years, 6 months ago (2015-06-25 17:12:43 UTC) #6
arv (Not doing code reviews)
5 years, 6 months ago (2015-06-25 17:14:29 UTC) #8
paul.l...
The stack layout is mixed up somewhere. I've not got a real fix, but this ...
5 years, 6 months ago (2015-06-25 18:48:19 UTC) #9
arv (Not doing code reviews)
On 2015/06/25 18:48:19, paul.l... wrote: > The stack layout is mixed up somewhere. I've not ...
5 years, 6 months ago (2015-06-25 18:58:32 UTC) #10
arv (Not doing code reviews)
Fix push/pop issue on mips
5 years, 6 months ago (2015-06-25 19:21:15 UTC) #11
arv (Not doing code reviews)
Also adjust the create memento branch
5 years, 6 months ago (2015-06-25 19:58:08 UTC) #12
arv (Not doing code reviews)
On 2015/06/25 18:58:32, arv wrote: > On 2015/06/25 18:48:19, paul.l... wrote: > > The stack ...
5 years, 6 months ago (2015-06-25 19:59:52 UTC) #13
paul.l...
On 2015/06/25 19:59:52, arv wrote: > I fixed the push/pop discrepancy and now both mips ...
5 years, 6 months ago (2015-06-25 21:56:17 UTC) #14
paul.l...
Simple issue, just took me a while to spot it. The arguments copy loop is ...
5 years, 6 months ago (2015-06-26 04:02:24 UTC) #16
arv (Not doing code reviews)
All working
5 years, 6 months ago (2015-06-26 16:23:42 UTC) #17
arv (Not doing code reviews)
All done. PTAL Paul, thank you so much for helping me sort out the bugs.
5 years, 6 months ago (2015-06-26 17:45:41 UTC) #18
adamk
lgtm
5 years, 6 months ago (2015-06-26 18:44:53 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1203813002/120001
5 years, 6 months ago (2015-06-26 18:45:43 UTC) #21
commit-bot: I haz the power
Exceeded global retry quota
5 years, 6 months ago (2015-06-26 18:48:58 UTC) #23
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1203813002/120001
5 years, 6 months ago (2015-06-26 18:49:45 UTC) #25
commit-bot: I haz the power
Exceeded global retry quota
5 years, 6 months ago (2015-06-26 18:50:11 UTC) #27
adamk
Looks like mksnapshot might be failing?
5 years, 6 months ago (2015-06-26 18:52:46 UTC) #28
arv (Not doing code reviews)
On 2015/06/26 18:52:46, adamk wrote: > Looks like mksnapshot might be failing? Yeah. I probable ...
5 years, 6 months ago (2015-06-26 18:54:27 UTC) #29
arv (Not doing code reviews)
Fixing the same issue throughout all the changes. https://codereview.chromium.org/1203813002/diff/120001/src/ia32/builtins-ia32.cc File src/ia32/builtins-ia32.cc (right): https://codereview.chromium.org/1203813002/diff/120001/src/ia32/builtins-ia32.cc#newcode414 src/ia32/builtins-ia32.cc:414: __ ...
5 years, 6 months ago (2015-06-26 19:02:38 UTC) #30
arv (Not doing code reviews)
Fix issue with ia32
5 years, 6 months ago (2015-06-26 19:06:48 UTC) #31
adamk
lgtm
5 years, 6 months ago (2015-06-26 19:16:26 UTC) #32
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1203813002/160001
5 years, 5 months ago (2015-06-29 18:27:58 UTC) #35
commit-bot: I haz the power
Committed patchset #9 (id:160001)
5 years, 5 months ago (2015-06-29 18:29:26 UTC) #36
commit-bot: I haz the power
5 years, 5 months ago (2015-06-29 18:29:44 UTC) #37
Message was sent while issue was closed.
Patchset 9 (id:??) landed as
https://crrev.com/7a63bf77eb7610afdc1a968f7660781e5160ba8d
Cr-Commit-Position: refs/heads/master@{#29358}

Powered by Google App Engine
This is Rietveld 408576698