Handlify JSObject::DeepCopy method.

Handlify JSObject::DeepCopy method.

R=verwaest@chromium.org

Committed: http://code.google.com/p/v8/source/detail?r=16708

[Toon Verwaest, 2013-08-20 09:37:13]

Added comments. Looking good, minus the missing empty-handle-check.

https://codereview.chromium.org/22934006/diff/1/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/22934006/diff/1/src/objects.cc#newcode5710
src/objects.cc:5710: (IsFastObjectElementsKind(copy->GetElementsKind())));
We can only have JSObject values if we are in the case
IsFastObjectElementsKind(copy->GetElementsKind()). This ASSERT at least
superfluous, but also indicates that we are doing too much work in the case that
we are FAST_(HOLEY_)SMI; trying to copy JSObjects that can't be there by
construction.

https://codereview.chromium.org/22934006/diff/1/src/runtime.cc
File src/runtime.cc (right):

https://codereview.chromium.org/22934006/diff/1/src/runtime.cc#newcode503
src/runtime.cc:503: return
*JSObject::DeepCopy(Handle<JSObject>::cast(boilerplate));
As discussed, here and everywhere else, we need an empty-handle check; since the
stack check in DeepCopy can return Handle<Object>::null().

[Michael Starzinger, 2013-08-29 19:18:27]

Addressed comments. PTAL. Also not that the two call-site in hydrogen.cc have
disappeared on bleeding_edge (after I will rebase), so I didn't add an
empty-handle check.

https://codereview.chromium.org/22934006/diff/1/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/22934006/diff/1/src/objects.cc#newcode5710
src/objects.cc:5710: (IsFastObjectElementsKind(copy->GetElementsKind())));
On 2013/08/20 09:37:13, Toon Verwaest wrote:
> We can only have JSObject values if we are in the case
> IsFastObjectElementsKind(copy->GetElementsKind()). This ASSERT at least
> superfluous, but also indicates that we are doing too much work in the case
that
> we are FAST_(HOLEY_)SMI; trying to copy JSObjects that can't be there by
> construction. 

I see your point. I can add special treatment for FAST_SMI_ELEMENTS and
FAST_HOLEY_SMI_ELEMENTS kinds either in this CL or in a separate one. I would
prefer a separate one and keep this a pure refactoring. Let me know what you
prefer.

https://codereview.chromium.org/22934006/diff/1/src/runtime.cc
File src/runtime.cc (right):

https://codereview.chromium.org/22934006/diff/1/src/runtime.cc#newcode503
src/runtime.cc:503: return
*JSObject::DeepCopy(Handle<JSObject>::cast(boilerplate));
On 2013/08/20 09:37:13, Toon Verwaest wrote:
> As discussed, here and everywhere else, we need an empty-handle check; since
the
> stack check in DeepCopy can return Handle<Object>::null().

Done.

[Toon Verwaest, 2013-08-29 20:54:51]

I agree we should do the refactoring in a different CL. This one LGTM.

[Michael Starzinger, 2013-09-13 09:51:24]

Committed patchset #2 manually as r16708 (presubmit successful).