Couple more comments (one thing I missed in my prev review is that ToUint32
can
fail leaving length uninitialized)
https://codereview.chromium.org/799853003/diff/20001/src/runtime/runtime-array.cc
File src/runtime/runtime-array.cc (right):
https://codereview.chromium.org/799853003/diff/20001/src/runtime/runtime-array.cc#newcode468
src/runtime/runtime-array.cc:468: val->ToUint32(&length);
This is not good.
If ToUInt32 returns false, `length` is uninitialized, but you proceed as
normal. No idea what the code below would do.
It is ok to not support large lengths, but fail gracefully, do not do
random things.
https://codereview.chromium.org/799853003/diff/20001/test/mjsunit/harmony/array-concat.js
File test/mjsunit/harmony/array-concat.js (right):
https://codereview.chromium.org/799853003/diff/20001/test/mjsunit/harmony/array-concat.js#newcode80
test/mjsunit/harmony/array-concat.js:80: "length": {valueOf: null,
toString: null},
Add a test where `toString` and `valueOf` throw an exception and
validate that the exception you get out of `concat` is exact same
exception they throw.
https://codereview.chromium.org/799853003/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
