Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(78)

Issue 384963002: ES6 Unscopables (Closed)

Created:
5 years, 3 months ago by arv (Not doing code reviews)
Modified:
5 years, 2 months ago
CC:
v8-dev
Project:
v8
Visibility:
Public.

Description

Patch Set 1 #

Total comments: 3

Patch Set 2 : #

Total comments: 10

Patch Set 3 : Refactor and expand tests a bit" #

Patch Set 4 : Add confusing test #

Patch Set 5 : Run all with tests with unscopables turned on #

Patch Set 6 : Changed tests to test multiple object types #

Total comments: 24

Patch Set 7 : Force use of holder as receiver for non js accessor callbacks #

Total comments: 1

Patch Set 8 : Revert changes to objects and comment out failing tests #

Patch Set 9 : #

Patch Set 10 : merge with accessors fixes in #

Patch Set 11 : Cleanup code using LookupIterator #

Total comments: 1

Patch Set 12 : Use new GetHolder #

Total comments: 6

Patch Set 13 : Call me Maybe? #

Patch Set 14 : Added cctest to test hidden prototypes #

Patch Set 15 : #

Total comments: 2

Patch Set 16 : Updated to reflect the July 2014 consensus #

Total comments: 6

Patch Set 17 : Remove unscopables.h. Add unscopables.js and set up the array unscopables #

Patch Set 18 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1011 lines, -3 lines) Patch
M BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +1 line, -0 lines 0 comments Download
M src/bootstrapper.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 2 chunks +5 lines, -0 lines 0 comments Download
M src/contexts.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 2 chunks +3 lines, -1 line 0 comments Download
M src/contexts.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 2 chunks +36 lines, -0 lines 0 comments Download
M src/flag-definitions.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +1 line, -0 lines 0 comments Download
M src/symbol.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +1 line, -1 line 0 comments Download
A src/unscopables.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +39 lines, -0 lines 0 comments Download
M test/cctest/cctest.gyp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +1 line, -0 lines 0 comments Download
A test/cctest/test-unscopables-hidden-prototype.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +105 lines, -0 lines 0 comments Download
A test/mjsunit/harmony/proxies-with-unscopables.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +153 lines, -0 lines 0 comments Download
A test/mjsunit/harmony/unscopables.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +664 lines, -0 lines 0 comments Download
M tools/generate-runtime-tests.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +1 line, -1 line 0 comments Download
M tools/gyp/v8.gyp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 45 (0 generated)
arv (Not doing code reviews)
Hi Andreas, As requested, I'm uploading this patch now that I've implemented the GetBindingValue part. ...
5 years, 3 months ago (2014-07-10 23:13:11 UTC) #1
arv (Not doing code reviews)
Hi Andreas, As requested, I'm uploading this patch now that I've implemented the GetBindingValue part. ...
5 years, 3 months ago (2014-07-10 23:13:12 UTC) #2
rossberg
Thanks Arv, looks good so far. Besides the refactoring you suggest I'd mainly like to ...
5 years, 3 months ago (2014-07-11 11:59:07 UTC) #3
arv (Not doing code reviews)
Add confusing test
5 years, 3 months ago (2014-07-11 21:21:42 UTC) #4
arv (Not doing code reviews)
Run all with tests with unscopables turned on
5 years, 3 months ago (2014-07-11 21:45:27 UTC) #5
arv (Not doing code reviews)
Updated but still not done. I'm seeing failures in test/mjsunit/with-leave.js. I'll look into that next ...
5 years, 3 months ago (2014-07-11 21:47:29 UTC) #6
arv (Not doing code reviews)
Changed tests to test multiple object types
5 years, 3 months ago (2014-07-16 22:57:02 UTC) #7
arv (Not doing code reviews)
This is now ready for review. PTAL.
5 years, 3 months ago (2014-07-16 23:01:14 UTC) #8
rossberg
https://codereview.chromium.org/384963002/diff/100001/src/unscopables.h File src/unscopables.h (right): https://codereview.chromium.org/384963002/diff/100001/src/unscopables.h#newcode20 src/unscopables.h:20: class UnscopableLookup { Hm, this class seems overkill to ...
5 years, 3 months ago (2014-07-17 15:12:14 UTC) #9
arv (Not doing code reviews)
I haven't uploaded a new snapshot yet. I got stuck on the following: var length ...
5 years, 3 months ago (2014-07-17 23:34:37 UTC) #10
rossberg
On 2014/07/17 23:34:37, arv wrote: > I haven't uploaded a new snapshot yet. I got ...
5 years, 2 months ago (2014-07-21 09:16:15 UTC) #11
arv (Not doing code reviews)
On 2014/07/21 at 09:16:15, rossberg wrote: > On 2014/07/17 23:34:37, arv wrote: > > I ...
5 years, 2 months ago (2014-07-21 15:27:29 UTC) #12
rossberg
On 2014/07/21 15:27:29, arv wrote: > On 2014/07/21 at 09:16:15, rossberg wrote: > > On ...
5 years, 2 months ago (2014-07-21 16:29:31 UTC) #13
arv (Not doing code reviews)
I refactored GetProperty to force usage of the holder as the receiver when we have ...
5 years, 2 months ago (2014-07-21 19:23:47 UTC) #14
rossberg
On 2014/07/21 19:23:47, arv wrote: > I refactored GetProperty to force usage of the holder ...
5 years, 2 months ago (2014-07-22 17:03:19 UTC) #15
Toon Verwaest
My comment below is based on a short explanation of the problem by rossberg. Let ...
5 years, 2 months ago (2014-07-23 12:08:39 UTC) #16
arv (Not doing code reviews)
Revert changes to objects and comment out failing tests
5 years, 2 months ago (2014-07-23 15:19:37 UTC) #17
arv (Not doing code reviews)
This is now blocked by https://codereview.chromium.org/410923003
5 years, 2 months ago (2014-07-23 18:13:41 UTC) #18
arv (Not doing code reviews)
merge with accessors fixes in
5 years, 2 months ago (2014-07-23 20:55:03 UTC) #19
arv (Not doing code reviews)
PTAL
5 years, 2 months ago (2014-07-23 20:58:22 UTC) #20
arv (Not doing code reviews)
Cleanup code using LookupIterator
5 years, 2 months ago (2014-07-23 23:06:20 UTC) #21
arv (Not doing code reviews)
Switching to use a LookupIterator cleaned this up a bit. https://codereview.chromium.org/384963002/diff/190001/src/unscopables.h File src/unscopables.h (right): https://codereview.chromium.org/384963002/diff/190001/src/unscopables.h#newcode24 ...
5 years, 2 months ago (2014-07-23 23:10:41 UTC) #22
arv (Not doing code reviews)
Use new GetHolder
5 years, 2 months ago (2014-07-24 21:32:16 UTC) #23
Toon Verwaest
https://codereview.chromium.org/384963002/diff/200001/src/runtime.cc File src/runtime.cc (right): https://codereview.chromium.org/384963002/diff/200001/src/runtime.cc#newcode9140 src/runtime.cc:9140: PropertyAttributes attrs = UnscopableLookup(&it); An exception could already have ...
5 years, 2 months ago (2014-07-25 10:09:41 UTC) #24
arv (Not doing code reviews)
On 2014/07/25 at 10:09:41, verwaest wrote: > https://codereview.chromium.org/384963002/diff/200001/src/runtime.cc > File src/runtime.cc (right): > > https://codereview.chromium.org/384963002/diff/200001/src/runtime.cc#newcode9140 ...
5 years, 2 months ago (2014-07-25 15:25:41 UTC) #25
arv (Not doing code reviews)
Never mind... I found a repro where the error is not propagated. https://codereview.chromium.org/384963002/diff/200001/src/contexts.cc File src/contexts.cc ...
5 years, 2 months ago (2014-07-25 15:33:26 UTC) #26
arv (Not doing code reviews)
Call me Maybe?
5 years, 2 months ago (2014-07-25 16:45:42 UTC) #27
Toon Verwaest
I just landed a CL that changes the return type to Maybe<> for those methods, ...
5 years, 2 months ago (2014-07-25 18:34:26 UTC) #28
Toon Verwaest
https://codereview.chromium.org/384963002/diff/200001/src/contexts.cc File src/contexts.cc (right): https://codereview.chromium.org/384963002/diff/200001/src/contexts.cc#newcode120 src/contexts.cc:120: if (isolate->has_pending_exception()) return Handle<Object>(); The problem is that it ...
5 years, 2 months ago (2014-07-25 18:52:11 UTC) #29
arv (Not doing code reviews)
Added cctest to test hidden prototypes
5 years, 2 months ago (2014-07-25 21:03:09 UTC) #30
arv (Not doing code reviews)
Toon, I added a cctest that tests the behavior with hidden prototypes. It seems like ...
5 years, 2 months ago (2014-07-25 21:08:33 UTC) #31
Toon Verwaest
If your sixth test are the expected semantics, then you indeed don't need to do ...
5 years, 2 months ago (2014-07-29 15:09:41 UTC) #32
Toon Verwaest
lgtm. just a last comment. https://codereview.chromium.org/384963002/diff/260001/src/unscopables.h File src/unscopables.h (right): https://codereview.chromium.org/384963002/diff/260001/src/unscopables.h#newcode25 src/unscopables.h:25: } while (false) Can ...
5 years, 2 months ago (2014-07-29 15:12:39 UTC) #33
arv (Not doing code reviews)
On 2014/07/29 at 15:12:39, verwaest wrote: > lgtm. just a last comment. > > https://codereview.chromium.org/384963002/diff/260001/src/unscopables.h ...
5 years, 2 months ago (2014-07-29 15:19:09 UTC) #34
Toon Verwaest
Ok, sounds good.
5 years, 2 months ago (2014-07-29 16:01:33 UTC) #35
arv (Not doing code reviews)
We talked about @@unscopables today at TC39 and we are going back to "Plan A" ...
5 years, 2 months ago (2014-07-29 19:26:03 UTC) #36
arv (Not doing code reviews)
https://codereview.chromium.org/384963002/diff/260001/src/unscopables.h File src/unscopables.h (right): https://codereview.chromium.org/384963002/diff/260001/src/unscopables.h#newcode25 src/unscopables.h:25: } while (false) On 2014/07/29 15:12:39, Toon Verwaest wrote: ...
5 years, 2 months ago (2014-07-30 15:39:38 UTC) #37
arv (Not doing code reviews)
This now reflects the JUly 2014 consesus. PTAL
5 years, 2 months ago (2014-08-05 16:47:44 UTC) #38
arv (Not doing code reviews)
Andreas, https://codereview.chromium.org/441943002/ removes Symbol.unscopables so once that lands this will have to add that back
5 years, 2 months ago (2014-08-05 16:52:01 UTC) #39
rossberg
https://codereview.chromium.org/384963002/diff/280001/include/v8.h File include/v8.h (right): https://codereview.chromium.org/384963002/diff/280001/include/v8.h#newcode892 include/v8.h:892: bool ToValue(T* t) { Can we not introduce this? ...
5 years, 2 months ago (2014-08-06 09:53:02 UTC) #40
arv (Not doing code reviews)
Remove unscopables.h. Add unscopables.js and set up the array unscopables
5 years, 2 months ago (2014-08-06 15:03:45 UTC) #41
arv (Not doing code reviews)
Thanks Andreas. I added a src/unscopables.js that conditionally adds the Symbol.unscopables. Would you prefer that ...
5 years, 2 months ago (2014-08-06 15:08:29 UTC) #42
rossberg
LGTM On 2014/08/06 15:08:29, arv wrote: > I added a src/unscopables.js that conditionally adds the ...
5 years, 2 months ago (2014-08-06 15:13:46 UTC) #43
rossberg
I'll land this.
5 years, 2 months ago (2014-08-06 15:15:56 UTC) #44
rossberg
5 years, 2 months ago (2014-08-06 15:50:55 UTC) #45
Message was sent while issue was closed.
Committed patchset #18 manually as 22942 (tree was closed).

Powered by Google App Engine
This is Rietveld 408576698