LGTM with some comments.
https://codereview.chromium.org/201873005/diff/10001/src/runtime.cc
File src/runtime.cc (right):
https://codereview.chromium.org/201873005/diff/10001/src/runtime.cc#newcode866
src/runtime.cc:866: CHECK(start <= source_byte_length);
Why CHECK rather than RUNTIME_ASSERT? Is a crash better than throwing an
exception?
https://codereview.chromium.org/201873005/diff/10001/src/typedarray.js
File src/typedarray.js (right):
https://codereview.chromium.org/201873005/diff/10001/src/typedarray.js#newcode409
src/typedarray.js:409: ToPositiveDataViewOffset(offset),
Is this safe?
https://codereview.chromium.org/201873005/diff/10001/test/mjsunit/regress/regress-353004.js
File test/mjsunit/regress/regress-353004.js (right):
https://codereview.chromium.org/201873005/diff/10001/test/mjsunit/regress/regress-353004.js#newcode2
test/mjsunit/regress/regress-353004.js:2: // Redistribution and use in
source and binary forms, with or without
nit: you can use the new, short licence header.
https://codereview.chromium.org/201873005/diff/10001/test/mjsunit/regress/regress-353004.js#newcode30
test/mjsunit/regress/regress-353004.js:30: buffer1 = new ArrayBuffer(100
* 1024 * 1024);
Wouldn't 100K be enough for each bufferX? I fear that working with big
arrays might lead to OOM crashes on memory-constrained
devices/buildbots.
https://codereview.chromium.org/201873005/diff/10001/test/mjsunit/regress/regress-353004.js#newcode32
test/mjsunit/regress/regress-353004.js:32: array1 = new
Uint8Array(buffer1, {valueOf : function() {
nit: how about some "var" keywords, here and for every other variable in
this test?
https://codereview.chromium.org/201873005/diff/10001/test/mjsunit/regress/regress-353004.js#newcode38
test/mjsunit/regress/regress-353004.js:38: for (i = 0; i <
array1.length; i++) {
This loop is pointless; if the assertEquals right above passed, it runs
for zero iterations.
https://codereview.chromium.org/201873005/diff/10001/test/mjsunit/regress/regress-353004.js#newcode59
test/mjsunit/regress/regress-353004.js:59: for (i = 0; i <
dataView1.byteLength; i++) {
another pointless loop
https://codereview.chromium.org/201873005/diff/10001/test/mjsunit/regress/regress-353004.js#newcode91
test/mjsunit/regress/regress-353004.js:91: %ArrayBufferNeuter(buffer9);
nit: funky indentation
https://codereview.chromium.org/201873005/
--
--
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.
