Thanks a lot, Mads!

I've updated the companion CL as well.


http://codereview.chromium.org/141044/diff/1002/4
File include/v8.h (right):

http://codereview.chromium.org/141044/diff/1002/4#newcode1179
Line 1179: bool CloneElementAt(uint32_t index, Handle<Object>* result);
On 2009/06/23 09:23:43, Mads Ager wrote:
> Please add a comment that explains this method including the fact that
it can
> fail.

> I would remove the bool return value and just have the signature:

> Handle<Object> CloneElementAt(uint32_t index);

> Returning an empty handle will then signal that the cloning failed.

Done.

http://codereview.chromium.org/141044/diff/1002/5
File src/api.cc (right):

http://codereview.chromium.org/141044/diff/1002/5#newcode3026
Line 3026: i::Object* cloned =
i::Heap::CopyJSObject(i::JSObject::cast(paragon));
On 2009/06/23 09:23:43, Mads Ager wrote:
> Why not use the handle based version i::Copy instead and let that
handle
> allocation failures?  You can model this after the Object::Clone
method.

I started with it actually.  Heap version runs somewhat (not
dramatically faster).  Anyway, reverted to Copy

Done.

http://codereview.chromium.org/141044

--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---

Reply via email to