ArraySplice builtin handlified.

ArraySplice builtin handlified.

R=yangguo@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=20156

[Igor Sheludko, 2014-03-20 13:32:35]

PTAL

[Yang, 2014-03-20 15:21:41]

some comments.

https://codereview.chromium.org/206073007/diff/1/src/builtins.cc
File src/builtins.cc (right):

https://codereview.chromium.org/206073007/diff/1/src/builtins.cc#newcode980
src/builtins.cc:980: accessor->CopyElements(
I should have noticed this in an earlier CL. Why do we no longer check (assert)
whether CopyElements failed? Could we add an assertion in CopyElements itself?

https://codereview.chromium.org/206073007/diff/1/src/factory.h
File src/factory.h (right):

https://codereview.chromium.org/206073007/diff/1/src/factory.h#newcode367
src/factory.h:367: return NewJSArray(elements_kind, 0, capacity, pretenure);
Do we really need this wrapper?

https://codereview.chromium.org/206073007/diff/1/src/factory.h#newcode374
src/factory.h:374: PretenureFlag pretenure = NOT_TENURED);
Same here.

[Igor Sheludko, 2014-03-21 09:49:39]

https://codereview.chromium.org/206073007/diff/1/src/builtins.cc
File src/builtins.cc (right):

https://codereview.chromium.org/206073007/diff/1/src/builtins.cc#newcode980
src/builtins.cc:980: accessor->CopyElements(
On 2014/03/20 15:21:41, Yang wrote:
> I should have noticed this in an earlier CL. Why do we no longer check
(assert)
> whether CopyElements failed? Could we add an assertion in CopyElements itself?

Done.

https://codereview.chromium.org/206073007/diff/1/src/factory.h
File src/factory.h (right):

https://codereview.chromium.org/206073007/diff/1/src/factory.h#newcode367
src/factory.h:367: return NewJSArray(elements_kind, 0, capacity, pretenure);
On 2014/03/20 15:21:41, Yang wrote:
> Do we really need this wrapper?

The NewJSArray() without length was here for some time and already has a number
of usages. But in my case I needed a more general one having both length and
capacity parameters. So I added it (see above) and defined the old one as a
special case of a new one.

https://codereview.chromium.org/206073007/diff/1/src/factory.h#newcode374
src/factory.h:374: PretenureFlag pretenure = NOT_TENURED);
On 2014/03/20 15:21:41, Yang wrote:
> Same here.

Same here for NewJSArrayWithElements(). To make this relationship more explicit
I'll move the implementation of the specific case to the header.

[Yang, 2014-03-21 10:21:15]

On 2014/03/21 09:49:39, Igor Sheludko wrote:
> https://codereview.chromium.org/206073007/diff/1/src/builtins.cc
> File src/builtins.cc (right):
> 
> https://codereview.chromium.org/206073007/diff/1/src/builtins.cc#newcode980
> src/builtins.cc:980: accessor->CopyElements(
> On 2014/03/20 15:21:41, Yang wrote:
> > I should have noticed this in an earlier CL. Why do we no longer check
> (assert)
> > whether CopyElements failed? Could we add an assertion in CopyElements
itself?
> 
> Done.
> 
> https://codereview.chromium.org/206073007/diff/1/src/factory.h
> File src/factory.h (right):
> 
> https://codereview.chromium.org/206073007/diff/1/src/factory.h#newcode367
> src/factory.h:367: return NewJSArray(elements_kind, 0, capacity, pretenure);
> On 2014/03/20 15:21:41, Yang wrote:
> > Do we really need this wrapper?
> 
> The NewJSArray() without length was here for some time and already has a
number
> of usages. But in my case I needed a more general one having both length and
> capacity parameters. So I added it (see above) and defined the old one as a
> special case of a new one.
> 
> https://codereview.chromium.org/206073007/diff/1/src/factory.h#newcode374
> src/factory.h:374: PretenureFlag pretenure = NOT_TENURED);
> On 2014/03/20 15:21:41, Yang wrote:
> > Same here.
> 
> Same here for NewJSArrayWithElements(). To make this relationship more
explicit
> I'll move the implementation of the specific case to the header.

LGTM.

[Igor Sheludko, 2014-03-21 11:22:22]

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