I think the second issue might be related to changes in GetProperty API. It
used to be a global function and now it is a static of Object (with
different type signature).
I've created a new CL and I'm running it on the try servers now.
On Tue, Apr 15, 2014 at 9:06 PM, ad...@chromium.org wrote:
One high-level comment: Is there a particular reason why SetIterator and
MapIterator are JSObjects (with mutable properties and elements)? Will they
ever
be exposed to user-land JavaScript? If not then I would vote for making them
internal objects (ideally a subclass of v8::internal::Struct).
https://codereview.chromium.org/236143002/diff/1/src/collection.js
File src/collection.js (right):
https://codereview.chromium.org/236143002/diff/1/src/collection.js#newcode130
src/collection.js:130: if (typeof f !== 'function') {
On 2014/04/14 19:51:49, arv wrote:
On 2014/04/14 19:15:42,
On 2014/04/15 10:47:23, Michael Starzinger wrote:
One high-level comment: Is there a particular reason why SetIterator and
MapIterator are JSObjects (with mutable properties and elements)? Will
they
ever
be exposed to user-land JavaScript? If not then I would vote for making
them
internal
On 2014/04/15 10:49:29, rossberg wrote:
https://codereview.chromium.org/236143002/diff/1/src/collection.js
File src/collection.js (right):
https://codereview.chromium.org/236143002/diff/1/src/collection.js#newcode130
src/collection.js:130: if (typeof f !== 'function') {
On 2014/04/14
On 2014/04/15 13:05:23, arv wrote:
How are the callables in DOM represented? We use
ObjectTemplate::SetCallAsFunctionHandler for these
https://code.google.com/p/chromium/codesearch#search/q=LegacyCallAsFunction%20file:%5C.idlsq=package:chromiumtype=cs
On Tue, Apr 15, 2014 at 9:21 AM, rossb...@chromium.org wrote:
On 2014/04/15 13:05:23, arv wrote:
How are the callables in DOM represented? We use
ObjectTemplate::SetCallAsFunctionHandler for these
https://code.google.com/p/chromium/codesearch#search/q=
Looking good. One round of comments, mostly nits.
https://codereview.chromium.org/236143002/diff/120001/src/bootstrapper.cc
File src/bootstrapper.cc (right):
https://codereview.chromium.org/236143002/diff/120001/src/bootstrapper.cc#newcode1370
src/bootstrapper.cc:1370:
One more comment.
https://codereview.chromium.org/236143002/diff/140001/test/mjsunit/harmony/collections.js
File test/mjsunit/harmony/collections.js (right):
https://codereview.chromium.org/236143002/diff/140001/test/mjsunit/harmony/collections.js#newcode820
PTAL
Michael, I'm not sure what your concern about GC is? At this point the
references are all strong refs.
I added a test but I'm not sure what you had in mind?
https://codereview.chromium.org/236143002/diff/120001/src/bootstrapper.cc
File src/bootstrapper.cc (right):
LGTM.
https://codereview.chromium.org/236143002/diff/120001/src/objects-visiting.h
File src/objects-visiting.h (right):
https://codereview.chromium.org/236143002/diff/120001/src/objects-visiting.h#newcode101
src/objects-visiting.h:101: V(JSMapIterator)\
On 2014/04/15 17:07:23, arv
On 2014/04/15 22:02:30, Michael Starzinger wrote:
LGTM.
Thanks for the review. Can you land this for me?
https://codereview.chromium.org/236143002/diff/120001/src/objects-visiting.h
File src/objects-visiting.h (right):
Committed patchset #11 manually as r20781 (presubmit successful).
https://codereview.chromium.org/236143002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups v8-dev group.
To
On 2014/04/16 00:40:18, adamk wrote:
Committed patchset #11 manually as r20781 (presubmit successful).
This got reverted in https://code.google.com/p/v8/source/detail?r=20782 for
breaking the Win32 build due to some unresolved symbols:
Mostly looking good. I think there might be an edge case with the final
element
in an iteration. And you've got some more handlification work. Other than
that,
I like the look of it!
https://codereview.chromium.org/236143002/diff/1/src/collection.js
File src/collection.js (right):
I'll look into changing things to use static next
https://codereview.chromium.org/236143002/diff/1/src/collection.js
File src/collection.js (right):
https://codereview.chromium.org/236143002/diff/1/src/collection.js#newcode130
src/collection.js:130: if (typeof f !== 'function') {
On 2014/04/14
Needs moar Handles.
Basically, in a function that does allocation, it's a Bad Idea to not use
Handles, because any of your raw Object* stored on the stack might get
relocated.
That's why I asked you add DisallowHeapAllocation to the places where you
loop
over the linked list. Another option
https://codereview.chromium.org/236143002/diff/60001/src/factory.cc
File src/factory.cc (right):
https://codereview.chromium.org/236143002/diff/60001/src/factory.cc#newcode2003
src/factory.cc:2003: JSObject::SetProperty(result, value_string(),
value, NONE, SLOPPY).Assert();
I think you can skip
https://codereview.chromium.org/236143002/diff/1/src/objects.cc
File src/objects.cc (right):
https://codereview.chromium.org/236143002/diff/1/src/objects.cc#newcode16090
src/objects.cc:16090: return factory-CreateIteratorResultObject(value,
false);
On 2014/04/14 19:51:49, arv wrote:
On
PTAL
https://codereview.chromium.org/236143002/diff/60001/src/objects.cc
File src/objects.cc (right):
https://codereview.chromium.org/236143002/diff/60001/src/objects.cc#newcode15918
src/objects.cc:15918: Object* undefined =
table-GetHeap()-undefined_value();
On 2014/04/14 21:25:54, adamk
Michael, Andreas,
I've been working with Adam on this over the last week. I think it is ready
for
review now.
Thanks.
https://codereview.chromium.org/236143002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because
https://codereview.chromium.org/236143002/diff/90001/src/bootstrapper.cc
File src/bootstrapper.cc (right):
https://codereview.chromium.org/236143002/diff/90001/src/bootstrapper.cc#newcode1295
src/bootstrapper.cc:1295: if (FLAG_harmony_collections) {
Can you move those map constructions up to
lgtm from my end, but of course I'm no v8 OWNER; looking forward to what
Michael
and/or Andreas have to say.
https://codereview.chromium.org/236143002/diff/90001/src/objects.cc
File src/objects.cc (right):
https://codereview.chromium.org/236143002/diff/90001/src/objects.cc#newcode16122
23 matches
Mail list logo
