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

Issue 960343002: ES6: Make function name configurable (Closed)

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

Description

ES6: Make function name configurable This is partially based on r21609 but that CL was incomplete. Function name is still non writable so one has to use defineProperty to change the actual value. BUG=v8:3333 LOG=N R=adamk, mstarzinger@chromium.org Committed: https://crrev.com/f7790f7670c8d859455a98fcb90ff1b66af1eca7 Cr-Commit-Position: refs/heads/master@{#26924}

Patch Set 1 #

Patch Set 2 : Make sure we test both sloppy and strict #

Patch Set 3 : whitespace #

Total comments: 2

Patch Set 4 : git rebase #

Patch Set 5 : Relax assert #

Total comments: 4

Patch Set 6 : Handle O.o #

Patch Set 7 : cleanup #

Total comments: 3

Patch Set 8 : Fix O.o test #

Patch Set 9 : Return directly from EnqueueChangeRecord #

Patch Set 10 : Use reconfigure instead #

Unified diffs Side-by-side diffs Delta from patch set Stats (+199 lines, -21 lines) Patch
M src/accessors.cc View 1 2 3 4 5 6 7 8 9 1 chunk +34 lines, -2 lines 0 comments Download
M src/bootstrapper.cc View 3 chunks +19 lines, -15 lines 0 comments Download
A test/mjsunit/es6/function-name-configurable.js View 1 2 3 4 5 1 chunk +115 lines, -0 lines 0 comments Download
M test/mjsunit/es7/object-observe.js View 1 2 3 4 5 6 7 2 chunks +27 lines, -1 line 0 comments Download
M test/mjsunit/regress/regress-1530.js View 1 2 3 4 1 chunk +3 lines, -2 lines 0 comments Download
M test/mjsunit/regress/regress-270142.js View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 36 (8 generated)
arv (Not doing code reviews)
more tests
5 years, 10 months ago (2015-02-26 20:44:13 UTC) #1
arv (Not doing code reviews)
Make sure we test both sloppy and strict
5 years, 10 months ago (2015-02-26 21:20:58 UTC) #2
arv (Not doing code reviews)
remove debug prints
5 years, 10 months ago (2015-02-26 21:24:05 UTC) #3
arv (Not doing code reviews)
whitespace
5 years, 10 months ago (2015-02-26 21:25:12 UTC) #4
arv (Not doing code reviews)
PTAL I'll also apply this to Blink to see what LayoutTest this needs updating. https://codereview.chromium.org/960343002/diff/80001/test/mjsunit/es7/object-observe.js ...
5 years, 10 months ago (2015-02-26 21:26:24 UTC) #7
arv (Not doing code reviews)
git rebase
5 years, 10 months ago (2015-02-26 21:39:00 UTC) #8
arv (Not doing code reviews)
Relax assert
5 years, 10 months ago (2015-02-26 22:27:35 UTC) #9
arv (Not doing code reviews)
https://codereview.chromium.org/960343002/diff/120001/src/lookup.cc File src/lookup.cc (right): https://codereview.chromium.org/960343002/diff/120001/src/lookup.cc#newcode316 src/lookup.cc:316: DCHECK(DATA == state_ || ACCESSOR == state_); Toon, this ...
5 years, 10 months ago (2015-02-26 22:30:37 UTC) #11
arv (Not doing code reviews)
On 2015/02/26 21:26:24, arv wrote: > PTAL > > I'll also apply this to Blink ...
5 years, 10 months ago (2015-02-26 23:22:04 UTC) #12
adamk
Answered the Object.observe part of this. Basically we treat these accessor-backed data properties just the ...
5 years, 10 months ago (2015-02-26 23:31:51 UTC) #13
Toon Verwaest
https://codereview.chromium.org/960343002/diff/120001/src/accessors.cc File src/accessors.cc (right): https://codereview.chromium.org/960343002/diff/120001/src/accessors.cc#newcode1151 src/accessors.cc:1151: } On 2015/02/26 23:31:51, adamk wrote: > You're going ...
5 years, 9 months ago (2015-02-27 12:44:11 UTC) #14
Toon Verwaest
(I'm not 100% sure about the O.o thing, I guess adamk knows what he's talking ...
5 years, 9 months ago (2015-02-27 12:45:39 UTC) #15
arv (Not doing code reviews)
Handle O.o
5 years, 9 months ago (2015-02-27 15:31:06 UTC) #16
arv (Not doing code reviews)
cleanup
5 years, 9 months ago (2015-02-27 15:37:33 UTC) #17
arv (Not doing code reviews)
https://codereview.chromium.org/960343002/diff/160001/src/accessors.cc File src/accessors.cc (right): https://codereview.chromium.org/960343002/diff/160001/src/accessors.cc#newcode1147 src/accessors.cc:1147: value = it.WriteDataValue(value); Turns out that I could not ...
5 years, 9 months ago (2015-02-27 15:44:33 UTC) #18
arv (Not doing code reviews)
https://codereview.chromium.org/960343002/diff/160001/test/mjsunit/es7/object-observe.js File test/mjsunit/es7/object-observe.js (right): https://codereview.chromium.org/960343002/diff/160001/test/mjsunit/es7/object-observe.js#newcode1001 test/mjsunit/es7/object-observe.js:1001: assertTrue(Object.getOwnPropertyDescriptor(obj, prop).configurable); On 2015/02/27 15:44:33, arv wrote: > This ...
5 years, 9 months ago (2015-02-27 16:23:15 UTC) #19
arv (Not doing code reviews)
Fix O.o test
5 years, 9 months ago (2015-02-27 16:50:21 UTC) #20
arv (Not doing code reviews)
PTAL This is now ready to go. The Blink surpression has also landed.
5 years, 9 months ago (2015-02-27 16:51:41 UTC) #21
adamk
lgtm
5 years, 9 months ago (2015-02-27 17:27:51 UTC) #22
Toon Verwaest
lgtm
5 years, 9 months ago (2015-02-27 17:49:36 UTC) #23
arv (Not doing code reviews)
Return directly from EnqueueChangeRecord
5 years, 9 months ago (2015-02-27 17:53:49 UTC) #24
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/960343002/200001
5 years, 9 months ago (2015-02-27 17:55:19 UTC) #27
commit-bot: I haz the power
Try jobs failed on following builders: v8_linux64_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel/builds/3418)
5 years, 9 months ago (2015-02-27 18:13:50 UTC) #29
arv (Not doing code reviews)
Use reconfigure instead
5 years, 9 months ago (2015-02-27 18:55:48 UTC) #30
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/960343002/220001
5 years, 9 months ago (2015-02-27 19:18:28 UTC) #33
commit-bot: I haz the power
Committed patchset #10 (id:220001)
5 years, 9 months ago (2015-02-27 19:29:01 UTC) #34
commit-bot: I haz the power
Patchset 10 (id:??) landed as https://crrev.com/f7790f7670c8d859455a98fcb90ff1b66af1eca7 Cr-Commit-Position: refs/heads/master@{#26924}
5 years, 9 months ago (2015-02-27 19:29:11 UTC) #35
arv (Not doing code reviews)
5 years, 9 months ago (2015-03-02 00:18:36 UTC) #36
Message was sent while issue was closed.
A revert of this CL (patchset #10 id:220001) has been created in
https://codereview.chromium.org/969683002/ by arv@chromium.org.

The reason for reverting is: Breaks Chrome browser test that checks Object.name

[16509:16509:0228/030150:INFO:CONSOLE(43)] "Uncaught Error: Clobbered
Object.name getter", source: http://www.chromium.org:33611/assertions.js (43)

http://build.chromium.org/p/client.v8/builders/Linux%20Tests%20%28dbg%29%281%....

Powered by Google App Engine
This is Rietveld 408576698