Thanks for the review! Addressed comments.
https://codereview.chromium.org/14581005/diff/2001/src/messages.js
File src/messages.js (right):
https://codereview.chromium.org/14581005/diff/2001/src/messages.js#newcode115
src/messages.js:115: ["Source is too long"],
On 2013/05/07 13:53:23, rossberg wrote:
"long" sounds weird, I'd say "large"
Done.
https://codereview.chromium.org/14581005/diff/2001/src/runtime.cc
File src/runtime.cc (right):
https://codereview.chromium.org/14581005/diff/2001/src/runtime.cc#newcode894
src/runtime.cc:894: RUNTIME_FUNCTION(MaybeObject*,
Runtime_TypedArraySetSpecialCases) {
On 2013/05/07 13:53:23, rossberg wrote:
Nit: I'd change the name, since it's the normal cases, right? How
about
TypedArraySetFastCases?
Done.
https://codereview.chromium.org/14581005/diff/2001/src/runtime.cc#newcode933
src/runtime.cc:933: source_base + source_byte_length > target_base) ||
On 2013/05/07 13:53:23, rossberg wrote:
Style nit: spurious space
Done.
https://codereview.chromium.org/14581005/diff/2001/src/runtime.cc#newcode983
src/runtime.cc:983: ASSERT((right_index + 1) - left_index <= 8);
On 2013/05/07 13:53:23, rossberg wrote:
I'd keep a comment similar to the one I had in d8 to explain this
assertion.
Done.
https://codereview.chromium.org/14581005/diff/2001/src/runtime.cc#newcode995
src/runtime.cc:995: } else { // Non-overlapping typed arrays
Can I please not? :))
On 2013/05/07 13:53:23, rossberg wrote:
Perhaps negate the conditional to put the common and simple case
first.
https://codereview.chromium.org/14581005/diff/2001/src/typedarray.js
File src/typedarray.js (right):
https://codereview.chromium.org/14581005/diff/2001/src/typedarray.js#newcode160
src/typedarray.js:160: for (var i = 0; i < l; i++) {
Done. No non-typed-array receivers
On 2013/05/07 13:53:23, rossberg wrote:
Is this function supposed to work with non-typed-array receivers? If
so,
wouldn't you need to check for overlap as well? If not, don't you need
a type
check here?
https://codereview.chromium.org/14581005/diff/2001/src/typedarray.js#newcode161
src/typedarray.js:161: this[intOffset+i] = obj[i];
On 2013/05/07 13:53:23, rossberg wrote:
Style nit: space around +
Done.
https://codereview.chromium.org/14581005/diff/2001/test/mjsunit/harmony/typedarrays.js
File test/mjsunit/harmony/typedarrays.js (right):
https://codereview.chromium.org/14581005/diff/2001/test/mjsunit/harmony/typedarrays.js#newcode454
test/mjsunit/harmony/typedarrays.js:454: }
On 2013/05/07 13:53:23, rossberg wrote:
Check that a.set.call throws with the wrong receivers.
Done.
https://codereview.chromium.org/14581005/
--
--
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/groups/opt_out.
