https://codereview.chromium.org/753553002/diff/280002/include/v8.h
File include/v8.h (right):

https://codereview.chromium.org/753553002/diff/280002/include/v8.h#newcode477
include/v8.h:477: T* internal_field1_;
I'm a little late weighing in here, but I have a few points of concern.
The first is that these two fields can be heap objects, which is crazily
dangerous for about 6 reasons.  We should set nullptr for any values
that are not aligned according to SetAlignedPointerInInternalField,
which won't catch all smis (probably we should box any smis set as
internal fields).

The second problem is that in many cases in blink we want both the
internal fields and the parameter, so splitting into two callbacks types
is a pretty bad idea here.  I think setting a bit saying you need up to
the first 2 internal fields if they are there should be okay, since an
embedder should be able to rearrange things such that those fields are
in the first two slots.  There should be no additional memory required
at that point for global handle nodes, just additional memory for the
phantom callback queue, which should be fine.

Lastly, these field are set with void*, so I think getting them with
void* should be okay, avoiding extra template parameters, but i'm pretty
indifferent to this.

https://codereview.chromium.org/753553002/

--
--
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.

Reply via email to