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):
https://codereview.chromium.org/236143002/diff/120001/src/bootstrapper.cc#newcode1370
src/bootstrapper.cc:1370:
ASSERT(object_function->initial_map()->inobject_properties() == 0);
On 2014/04/15 16:13:56, Michael Starzinger wrote:
nit: This assert belongs to the code creating the
"iterator_result_map" I think.
It ensures that the "Object" map can be used as a base for result
object map.
Can we move it down?
Done.
https://codereview.chromium.org/236143002/diff/120001/src/collection.js
File src/collection.js (right):
https://codereview.chromium.org/236143002/diff/120001/src/collection.js#newcode121
src/collection.js:121: var SET_ITERATOR_KIND_VALUES = 2;
On 2014/04/15 16:13:56, Michael Starzinger wrote:
nit: Can we move this constant, together with the comment into
macros.py?
I put ITERATOR_KIND_* in macros and replaced *_ITERATOR_KIND_* with
this.
https://codereview.chromium.org/236143002/diff/120001/src/collection.js#newcode130
src/collection.js:130: if (typeof f !== 'function') {
On 2014/04/15 16:13:56, Michael Starzinger wrote:
I think this should use IS_SPEC_FUNCTION for consistency with the rest
of the
code. I'll defer to Andreas' comment about this.
Done.
https://codereview.chromium.org/236143002/diff/120001/src/collection.js#newcode238
src/collection.js:238: var MAP_ITERATOR_KIND_ENTRIES = 3;
On 2014/04/15 16:13:56, Michael Starzinger wrote:
nit: Likewise.
Done.
https://codereview.chromium.org/236143002/diff/120001/src/collection.js#newcode247
src/collection.js:247: if (typeof f !== 'function') {
On 2014/04/15 16:13:56, Michael Starzinger wrote:
Likewise.
Done.
https://codereview.chromium.org/236143002/diff/120001/src/factory.cc
File src/factory.cc (right):
https://codereview.chromium.org/236143002/diff/120001/src/factory.cc#newcode2000
src/factory.cc:2000: Handle<JSObject>
Factory::CreateIteratorResultObject(Handle<Object> value,
On 2014/04/15 16:13:56, Michael Starzinger wrote:
nit: Likewise, can we move this up to the other allocation functions?
Done.
https://codereview.chromium.org/236143002/diff/120001/src/factory.h
File src/factory.h (right):
https://codereview.chromium.org/236143002/diff/120001/src/factory.h#newcode596
src/factory.h:596: Handle<JSObject>
CreateIteratorResultObject(Handle<Object> value, bool done);
On 2014/04/15 16:13:56, Michael Starzinger wrote:
nit: Can we call this "NewIteratorResultObject" instead and move it up
to where
the other allocation functions are?
Done.
https://codereview.chromium.org/236143002/diff/120001/src/objects-printer.cc
File src/objects-printer.cc (right):
https://codereview.chromium.org/236143002/diff/120001/src/objects-printer.cc#newcode775
src/objects-printer.cc:775: PrintF(out, " - count = ");
On 2014/04/15 16:13:56, Michael Starzinger wrote:
nit: I think there are couple of newline characters missing here. The
ShortPrint() will not print a trailing newline IIRC.
Done.
I also missed printing the map().
Fixed JSProxyPrint and JSFunctionProxyPrint too.
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 16:13:56, Michael Starzinger wrote:
These definitions should not be needed, the two objects behave like
normal
JSObjects and don't need any special casing in the visitor.
Done.
I'll have to re-add them when I expose the iterators to user code.
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 unsubscribe from this group and stop receiving emails from it, send an email
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
