Fix array boilerplate object transitioning.

Fix array boilerplate object transitioning.

Array literal boilerplate objects can be transitioned while existing
un-transitioned clones are still being populated. This adds a check that
prevents us from performing the same transition twice.

R=danno@chromium.org
BUG=v8:2055
TEST=mjsunit/regress/regress-2055

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

[danno, 2012-04-03 15:47:46]

Almost....

https://chromiumcodereview.appspot.com/9950095/diff/1/src/runtime.cc
File src/runtime.cc (right):

https://chromiumcodereview.appspot.com/9950095/diff/1/src/runtime.cc#newcode4684
src/runtime.cc:4684: ASSERT(elements_kind == boilerplate->GetElementsKind());
The boilerplate could also be FAST_ELEMENTS at this point. You need to make sure
that the boilerplate has transitioned to at least the ElementsKind that's
needed, but beyond is OK, too. You might want to use
IsMoreGeneralElementsKindTransition for this test.

https://chromiumcodereview.appspot.com/9950095/diff/1/src/runtime.cc#newcode4698
src/runtime.cc:4698: }
Same here.

https://chromiumcodereview.appspot.com/9950095/diff/1/test/mjsunit/regress/re...
File test/mjsunit/regress/regress-2055.js (right):

https://chromiumcodereview.appspot.com/9950095/diff/1/test/mjsunit/regress/re...
test/mjsunit/regress/regress-2055.js:39: assertEquals([0,[0,[]]], foo(2));
Test the crazy case that I describe in the .cc file.

[Michael Starzinger, 2012-04-03 16:31:52]

Added new patch set. PTAL.

https://chromiumcodereview.appspot.com/9950095/diff/1/src/runtime.cc
File src/runtime.cc (right):

https://chromiumcodereview.appspot.com/9950095/diff/1/src/runtime.cc#newcode4684
src/runtime.cc:4684: ASSERT(elements_kind == boilerplate->GetElementsKind());
On 2012/04/03 15:47:46, danno wrote:
> The boilerplate could also be FAST_ELEMENTS at this point. You need to make
sure
> that the boilerplate has transitioned to at least the ElementsKind that's
> needed, but beyond is OK, too. You might want to use
> IsMoreGeneralElementsKindTransition for this test.

Done.

https://chromiumcodereview.appspot.com/9950095/diff/1/src/runtime.cc#newcode4698
src/runtime.cc:4698: }
On 2012/04/03 15:47:46, danno wrote:
> Same here.

Done.

https://chromiumcodereview.appspot.com/9950095/diff/1/test/mjsunit/regress/re...
File test/mjsunit/regress/regress-2055.js (right):

https://chromiumcodereview.appspot.com/9950095/diff/1/test/mjsunit/regress/re...
test/mjsunit/regress/regress-2055.js:39: assertEquals([0,[0,[]]], foo(2));
On 2012/04/03 15:47:46, danno wrote:
> Test the crazy case that I describe in the .cc file.

Done.

[danno, 2012-04-03 16:38:10]

LGTM with comments

https://chromiumcodereview.appspot.com/9950095/diff/4/src/runtime.cc
File src/runtime.cc (right):

https://chromiumcodereview.appspot.com/9950095/diff/4/src/runtime.cc#newcode4685
src/runtime.cc:4685: ASSERT(elements_kind == boilerplate->GetElementsKind());
This assertion isn't needed and misleading now (similar to below).

https://chromiumcodereview.appspot.com/9950095/diff/4/src/runtime.cc#newcode4698
src/runtime.cc:4698: ASSERT(elements_kind == boilerplate->GetElementsKind());
This will assert when elements_kind is FAST_SMI_ONLY_ELEMENTS but
boilerplate->GetElementsKind() is FAST_DOUBLE_ELEMENTS, but that's actually a
valid case.

[Michael Starzinger, 2012-04-03 16:55:03]

Added new patch set. Landed.

https://chromiumcodereview.appspot.com/9950095/diff/4/src/runtime.cc
File src/runtime.cc (right):

https://chromiumcodereview.appspot.com/9950095/diff/4/src/runtime.cc#newcode4685
src/runtime.cc:4685: ASSERT(elements_kind == boilerplate->GetElementsKind());
On 2012/04/03 16:38:10, danno wrote:
> This assertion isn't needed and misleading now (similar to below).

Done.

https://chromiumcodereview.appspot.com/9950095/diff/4/src/runtime.cc#newcode4698
src/runtime.cc:4698: ASSERT(elements_kind == boilerplate->GetElementsKind());
On 2012/04/03 16:38:10, danno wrote:
> This will assert when elements_kind is FAST_SMI_ONLY_ELEMENTS but
> boilerplate->GetElementsKind() is FAST_DOUBLE_ELEMENTS, but that's actually a
> valid case.

Done.