Introduce builtin for Array.shift function.

Introduce builtin for Array.shift function.

[antonm, 2010-02-12 13:18:17]

Mads,

may you have a look?

If it's fine, I'd port other function as well.

[Mads Ager (chromium), 2010-02-12 13:27:51]

I don't think this works correctly for arrays with holes?

http://codereview.chromium.org/604033/diff/1/3
File src/builtins.cc (right):

http://codereview.chromium.org/604033/diff/1/3#newcode341
src/builtins.cc:341: Object* undefined = Heap::undefined_value();
Get rid of the local variable and just put in Heap::undefined_value at the only
use site?

http://codereview.chromium.org/604033/diff/1/3#newcode358
src/builtins.cc:358: if (first->IsTheHole())
Please use braces around the body.

http://codereview.chromium.org/604033/diff/1/3#newcode363
src/builtins.cc:363: if (e->IsTheHole())
Please use braces around the body.

http://codereview.chromium.org/604033/diff/1/3#newcode365
src/builtins.cc:365: elms->set(i, e);
Will this preserve array holes?  Won't GetElement return undefined for holes in
the array?  Then set will introduce an element that wasn't there before with an
undefined value?

[antonm, 2010-02-12 14:05:55]

http://codereview.chromium.org/604033/diff/1/3
File src/builtins.cc (right):

http://codereview.chromium.org/604033/diff/1/3#newcode341
src/builtins.cc:341: Object* undefined = Heap::undefined_value();
On 2010/02/12 13:27:51, Mads Ager wrote:
> Get rid of the local variable and just put in Heap::undefined_value at the
only
> use site?

Done.

http://codereview.chromium.org/604033/diff/1/3#newcode358
src/builtins.cc:358: if (first->IsTheHole())
On 2010/02/12 13:27:51, Mads Ager wrote:
> Please use braces around the body.

Done.

http://codereview.chromium.org/604033/diff/1/3#newcode363
src/builtins.cc:363: if (e->IsTheHole())
On 2010/02/12 13:27:51, Mads Ager wrote:
> Please use braces around the body.

Done.

http://codereview.chromium.org/604033/diff/1/3#newcode365
src/builtins.cc:365: elms->set(i, e);
On 2010/02/12 13:27:51, Mads Ager wrote:
> Will this preserve array holes?  Won't GetElement return undefined for holes
in
> the array?  Then set will introduce an element that wasn't there before with
an
> undefined value?

Nice catch, thanks a lot.  Regression test added.  Fixed in not most optimal way
for sparsed arrays (doing HasElement call).  If it'd prove to be a bottleneck,
I'd adjust.

The problem is I didn't find an API to get element which would distinguish
between hole and undefined.  It's easy to add (if indeed missing).

[antonm, 2010-02-12 14:08:21]

Sorry, messed up with git cl.  Moving to 
http://codereview.chromium.org/606017


On 2010/02/12 14:05:55, antonm wrote:
> http://codereview.chromium.org/604033/diff/1/3
> File src/builtins.cc (right):
> 
> http://codereview.chromium.org/604033/diff/1/3#newcode341
> src/builtins.cc:341: Object* undefined = Heap::undefined_value();
> On 2010/02/12 13:27:51, Mads Ager wrote:
> > Get rid of the local variable and just put in Heap::undefined_value at the
> only
> > use site?
> 
> Done.
> 
> http://codereview.chromium.org/604033/diff/1/3#newcode358
> src/builtins.cc:358: if (first->IsTheHole())
> On 2010/02/12 13:27:51, Mads Ager wrote:
> > Please use braces around the body.
> 
> Done.
> 
> http://codereview.chromium.org/604033/diff/1/3#newcode363
> src/builtins.cc:363: if (e->IsTheHole())
> On 2010/02/12 13:27:51, Mads Ager wrote:
> > Please use braces around the body.
> 
> Done.
> 
> http://codereview.chromium.org/604033/diff/1/3#newcode365
> src/builtins.cc:365: elms->set(i, e);
> On 2010/02/12 13:27:51, Mads Ager wrote:
> > Will this preserve array holes?  Won't GetElement return undefined for holes
> in
> > the array?  Then set will introduce an element that wasn't there before with
> an
> > undefined value?
> 
> Nice catch, thanks a lot.  Regression test added.  Fixed in not most optimal
way
> for sparsed arrays (doing HasElement call).  If it'd prove to be a bottleneck,
> I'd adjust.
> 
> The problem is I didn't find an API to get element which would distinguish
> between hole and undefined.  It's easy to add (if indeed missing).