Implement TypedArray.set function.

Implement TypedArray.set function.

R=rossberg@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=14576

[Dmitry Lomov (no reviews), 2013-05-07 10:36:21]

PTAL.
This is actually based mostly on your implementation from d8.cc

[Dmitry Lomov (no reviews), 2013-05-07 11:19:57]

Fixed a bug.

[rossberg, 2013-05-07 13:53:22]

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"],
"long" sounds weird, I'd say "large"

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) {
Nit: I'd change the name, since it's the normal cases, right? How about
TypedArraySetFastCases?

https://codereview.chromium.org/14581005/diff/2001/src/runtime.cc#newcode933
src/runtime.cc:933: source_base + source_byte_length >  target_base) ||
Style nit: spurious space

https://codereview.chromium.org/14581005/diff/2001/src/runtime.cc#newcode983
src/runtime.cc:983: ASSERT((right_index + 1) - left_index <= 8);
I'd keep a comment similar to the one I had in d8 to explain this assertion.

https://codereview.chromium.org/14581005/diff/2001/src/runtime.cc#newcode995
src/runtime.cc:995: } else {  // Non-overlapping typed arrays
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++) {
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];
Style nit: space around +

https://codereview.chromium.org/14581005/diff/2001/test/mjsunit/harmony/typed...
File test/mjsunit/harmony/typedarrays.js (right):

https://codereview.chromium.org/14581005/diff/2001/test/mjsunit/harmony/typed...
test/mjsunit/harmony/typedarrays.js:454: }
Check that a.set.call throws with the wrong receivers.

[Dmitry Lomov (no reviews), 2013-05-07 14:20:00]

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/typed...
File test/mjsunit/harmony/typedarrays.js (right):

https://codereview.chromium.org/14581005/diff/2001/test/mjsunit/harmony/typed...
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.

[rossberg, 2013-05-07 14:31:45]

LGTM, with one comment

https://codereview.chromium.org/14581005/diff/9001/test/mjsunit/harmony/typed...
File test/mjsunit/harmony/typedarrays.js (right):

https://codereview.chromium.org/14581005/diff/9001/test/mjsunit/harmony/typed...
test/mjsunit/harmony/typedarrays.js:455: assertThrows(function() { a.set.call(0)
}, TypeError);
More interestingly, maybe add a case for []

[Dmitry Lomov (no reviews), 2013-05-07 14:42:26]

Committed patchset #3 manually as r14576 (presubmit successful).