Remove list copy constructor (for which there was no corresponding...

Remove list copy constructor (for which there was no corresponding
assignment operator) and add an AddAll method to lists instead.

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

[Kasper Lund, 2009-05-22 11:07:25]

LGTM, but it would be nice to simplify AddAll and make sure that repeated access
to stuff like other.length_ is fast. Maybe you should extend test-list.cc to
make sure AddAll works as intended?

http://codereview.chromium.org/115705/diff/1003/1006
File src/list-inl.h (right):

http://codereview.chromium.org/115705/diff/1003/1006#newcode49
Line 49: int result_length = length_ + other.length_;
There's a mixture of other.length() and other.length_ access in this function.

http://codereview.chromium.org/115705/diff/1003/1007
File src/virtual-frame.cc (right):

http://codereview.chromium.org/115705/diff/1003/1007#newcode40
Line 40: : elements_(original->elements_.capacity()),
Maybe length would be enough here?

[Kevin Millikin (Chromium), 2009-05-22 11:33:06]

As we discussed offline, it's unexpected that the assignment operator for the
elements is used for small lists and not for large ones.

We should always add via the for loop.

With that change, LGTM.

[Mads Ager (chromium), 2009-05-22 11:51:41]

On 2009/05/22 11:07:25, Kasper Lund wrote:
> LGTM, but it would be nice to simplify AddAll and make sure that repeated
access
> to stuff like other.length_ is fast. Maybe you should extend test-list.cc to
> make sure AddAll works as intended?

AddAll has been simplified and the memcpy part has been removed.  It seems to
make no performance difference.  Simple test case added to test-list.

> http://codereview.chromium.org/115705/diff/1003/1006
> File src/list-inl.h (right):
> 
> http://codereview.chromium.org/115705/diff/1003/1006#newcode49
> Line 49: int result_length = length_ + other.length_;
> There's a mixture of other.length() and other.length_ access in this function.

Fixed.

> http://codereview.chromium.org/115705/diff/1003/1007
> File src/virtual-frame.cc (right):
> 
> http://codereview.chromium.org/115705/diff/1003/1007#newcode40
> Line 40: : elements_(original->elements_.capacity()),
> Maybe length would be enough here?

Fixed.

[Kasper Lund, 2009-05-22 11:55:52]

LGTM.