http://codereview.chromium.org/8820014/diff/4001/src/arm/builtins-arm.cc File src/arm/builtins-arm.cc (right):
http://codereview.chromium.org/8820014/diff/4001/src/arm/builtins-arm.cc#newcode397 src/arm/builtins-arm.cc:397: Label bailout; Unused label. (Does this even compile with -Werror?) http://codereview.chromium.org/8820014/diff/4001/src/arm/builtins-arm.cc#newcode398 src/arm/builtins-arm.cc:398: __ mov(r7, sp); Why? I don't see how sp would get overwritten. http://codereview.chromium.org/8820014/diff/4001/src/builtins.cc File src/builtins.cc (right): http://codereview.chromium.org/8820014/diff/4001/src/builtins.cc#newcode238 src/builtins.cc:238: array->set_length(Smi::FromInt(0)); A comment explaining why the length must be set to 0 first would be nice. http://codereview.chromium.org/8820014/diff/4001/src/builtins.cc#newcode255 src/builtins.cc:255: FixedArrayBase* elms = FixedArrayBase::cast(obj); Thanks to templates, you can clean this up a little by writing: if (!maybe_obj->To(&elms)) return maybe_obj; Then you wouldn't need "Object* obj;" at all. I don't mind either way, your call. http://codereview.chromium.org/8820014/diff/4001/src/ia32/builtins-ia32.cc File src/ia32/builtins-ia32.cc (right): http://codereview.chromium.org/8820014/diff/4001/src/ia32/builtins-ia32.cc#newcode1246 src/ia32/builtins-ia32.cc:1246: // esp[4] : argc nit: excess space http://codereview.chromium.org/8820014/diff/4001/src/ia32/builtins-ia32.cc#newcode1265 src/ia32/builtins-ia32.cc:1265: // esp[4] : argc same nit here http://codereview.chromium.org/8820014/diff/4001/src/ia32/builtins-ia32.cc#newcode1286 src/ia32/builtins-ia32.cc:1286: // esp[4] : argc same nit here http://codereview.chromium.org/8820014/diff/4001/src/objects-inl.h File src/objects-inl.h (right): http://codereview.chromium.org/8820014/diff/4001/src/objects-inl.h#newcode1282 src/objects-inl.h:1282: mode); nit: no need for line break http://codereview.chromium.org/8820014/diff/4001/src/objects.cc File src/objects.cc (right): http://codereview.chromium.org/8820014/diff/4001/src/objects.cc#newcode8370 src/objects.cc:8370: void JSArray::Expand(int required_size) { Instead of all this code here, we should be able to make use of SetFastElementsCapacityAndLength (passing in the original length, and required_size as capacity). http://codereview.chromium.org/8820014/diff/4001/test/mjsunit/array-construct-transition.js File test/mjsunit/array-construct-transition.js (right): http://codereview.chromium.org/8820014/diff/4001/test/mjsunit/array-construct-transition.js#newcode33 test/mjsunit/array-construct-transition.js:33: a = new Array(0, 1, 2); nit: "var a", please. Same for b and c. http://codereview.chromium.org/8820014/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
