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):
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 wrote:
> 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.
These visitor ids are the main reason I asked for a GC test case. Because
with
the old version I reviewed (i.e. patch set #8 and #9) the iterator
objects got
separate visitor ids assigned but none of the visitors actually
registered any
callbacks for these ids. So although I didn't try to run it, I was under
the
impression that e.g. the static marking visitor would not have been able
to
handle those new visitor ids. I am still pretty convinced that running
the new
test against patch set #9 will crash, because it will dispatch to a
non-registered callback.
You are right. I just checked out the old version with the gc test and it
did
crash.
https://codereview.chromium.org/236143002/
--
--
v8-dev mailing list
[email protected]
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 [email protected].
For more options, visit https://groups.google.com/d/optout.
