https://chromiumcodereview.appspot.com/11299004/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):

https://chromiumcodereview.appspot.com/11299004/diff/1/src/hydrogen.cc#newcode6669
src/hydrogen.cc:6669: fast_mode = false;
Why do you always set this to false? You could do the same trick as for
is_array: initialize to true, and inside the loop set to false if
necessary. You can even keep the existing "break;", just add the
following after the if-block:

fast_mode = fast_mode &&
IsFastElementsKind(current_map->elements_kind());

https://chromiumcodereview.appspot.com/11299004/diff/1/src/ic.cc
File src/ic.cc (right):

https://chromiumcodereview.appspot.com/11299004/diff/1/src/ic.cc#newcode987
src/ic.cc:987: // we have IC support only for getting the array length
property.
nit: capital W

https://chromiumcodereview.appspot.com/11299004/diff/1/src/ic.cc#newcode988
src/ic.cc:988: if (holder->IsJSArray() &&
nit: You can avoid one level of indentation here by moving this up into
a new "else if" block, and keeping the original "else { ASSERT(...);
return; }" after that.

https://chromiumcodereview.appspot.com/11299004/diff/1/src/objects.h
File src/objects.h (right):

https://chromiumcodereview.appspot.com/11299004/diff/1/src/objects.h#newcode8356
src/objects.h:8356: class PropertyIndex;
nit: two empty lines between top-level definitions (i.e. two before
"class PropertyIndex" and two after).

https://chromiumcodereview.appspot.com/11299004/diff/1/test/mjsunit/array-length.js
File test/mjsunit/array-length.js (right):

https://chromiumcodereview.appspot.com/11299004/diff/1/test/mjsunit/array-length.js#newcode132
test/mjsunit/array-length.js:132: var getLength = function(a) { return
a.length; }
What's the point of doing this? Are you trying to get a fresh copy of
the function with uninitialized ICs? That won't work, because closures
created from the same literal will share the same code (it's why we have
SharedFunctionInfo).

Otherwise, you could just declare these functions globally and avoid
passing them around all the time.

https://chromiumcodereview.appspot.com/11299004/diff/1/test/mjsunit/array-length.js#newcode147
test/mjsunit/array-length.js:147: length = 4;
unused variable?

https://chromiumcodereview.appspot.com/11299004/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to