1021379 - Rename typed arrays' move method to copyWithin, fix up to ES6 semantics

Status: RESOLVED FIXED

(Core :: JavaScript: Standard Library, enhancement)

Description

[Jeff Walden [:Waldo]]

We implemented {{Ui,I}nt{8,16,32}Array,Float{16,32}Array}.prototype.move a bit ago and proposed it to TC39.  They added it, but possibly with different semantics from ours, and definitely with a different name -- "copyWithin".  We should fix and rename so we can enable this everywhere (it's turned off in beta/release right now because of the non-standard thing) and not have this non-standard extension to think about.

Comment 1

[Till Schneidereit [:till]]

Rishab, you said you'd like to work on this. The URL links to the relevant part of the spec. I'd recommend looking at how Array#copyWithin is implemented in bug 934423.

It's possible that this boils down to changing `memmove` to `memcpy` in fun_move_impl in TypedArrayObject.cpp and renaming all references to copyWithin. But then again the algorithm might have changed in subtle ways. To weed those differences out, it'd be best to annotate the implementation with the same "// Step n." comments as in Array#copyWithin.

Comment 2

[Rishab Arora [:spacetime]]

Attachment: Preliminary patch for TypedArray#copyWithin

Hi, I've made some changes to TypedArray#copyWithin to comply with ES6. For tests, I modified all the tests from Array#copyWithin, removing a few (this as number, this.length is called only once). I also had some questions regarding the tests:

1. The test "check that delete is strict" depends on Bug 949868 . How do I find out the correct behavior? (Test is currently commented out)
2. In the test "test if we throw in between", since we're using memcpy unlike the implementation for Array where each element is accessed, wouldn't this test never work? Does the spec require this? (Currently commented out)
3. "test that this.length is called only once"; Why do we need this one? (Removed the test)
4. "test null on third argument is handled correctly". Does the spec handle null? (Commented)
5. Test with Proxy object fails. I feel it is linked to the next test. (Currently fails)
6. Tampering the global Object fails. I'm not sure why this is happening. Is this expected?(Currently fails)

Comment 3

[André Bargull [:anba]]

(In reply to Rishab Arora [:spacetime] from comment #2)
> 1. The test "check that delete is strict" depends on Bug 949868 . How do I
> find out the correct behavior? (Test is currently commented out)

This test does not apply to typed array objects, "holes" are never present.


> 2. In the test "test if we throw in between", since we're using memcpy
> unlike the implementation for Array where each element is accessed, wouldn't
> this test never work? Does the spec require this? (Currently commented out)

This test does not apply to typed array objects, indexed elements are never accessors. 


> 3. "test that this.length is called only once"; Why do we need this one?
> (Removed the test)

Here you should instead test that "length" is never called, even if it was defined as an explicit accessor property.

---
// test that this.length is never called
arr = new constructor([0, 1, 2, 3, 5]);
Object.defineProperty(arr, "length", {
  get: function () { throw new Error("length accessor called"); }
});
arr.copyWithin(1, 3);
---

> 4. "test null on third argument is handled correctly". Does the spec handle
> null? (Commented)

Most likely this test was added to ensure the self-hosted version for Array.prototype.copyWithin correctly handles `undefined` and `null` argument values (cf. difference between `x == null` and `x === undefined`). According to the specification `null` has the same effect as `0`, because ToLength(null) == ToInteger(null) == ToNumber(null) == 0.


> 5. Test with Proxy object fails. I feel it is linked to the next test.
> (Currently fails)

This test should throw a TypeError, because %TypedArray%.prototype.copyWithin is only applicable for typed array objects.


> 6. Tampering the global Object fails. I'm not sure why this is happening. Is
> this expected?(Currently fails)

This test does not apply to typed array objects, you can leave it out.

Comment 4

[Till Schneidereit [:till]]

Comment on attachment 8452178 [details] [diff] [review]
Preliminary patch for TypedArray#copyWithin

Review of attachment 8452178 [details] [diff] [review]:
-----------------------------------------------------------------

This is on the right track, but needs a bit more work.

A couple of general comments:
I guess you manually renamed the test file? Please use `hg mv` so history is preserved. Also, with the current patch at least the old test file would still exist and cause failures.

For more tests, please look at the tests for Array.prototype.copyWithin as landed in bug 934423. You should be able to copy most of them over, although some won't make sense for typed arrays.

I see that André commented on your questions, and as always I don't have anything productive to add to his comments.

::: js/src/jit-test/tests/basic/typed-array-copywithin.js
@@ +17,5 @@
> +];
> +
> +for (constructor of constructors) {
> +
> +    assertEq(constructor.prototype.copyWithin.length, 2);    

Nit: whitespace at line end.

::: js/src/vm/TypedArrayObject.cpp
@@ +526,5 @@
>          return CallNonGenericMethod<ThisTypedArrayObject::IsThisClass,
>                                      ThisTypedArrayObject::fun_subarray_impl>(cx, args);
>      }
>  
> +    /* copyWithin(target, start[, end]) */

Please add a reference to the spec version and section: "ES6 draft rev 25, 22.2.3.5"

@@ +538,1 @@
>          JS_ASSERT(IsThisClass(args.thisv()));

Keep this assert at the very top of the function.

@@ +538,2 @@
>          JS_ASSERT(IsThisClass(args.thisv()));
> +        /* Step 1 */

This actually includes step 2: ReturnIfAbrupt is a spec-device that isn't explicitly expressed in our implementation.

General nit: Please use line comments for the steps, add a full stop at the end, and pluralize "step" for ranges. I.e., this should be "// Steps 1-2." This is the style we use in other builtin functions, and I think it makes sense to standardize on it.

@@ +540,1 @@
>          Rooted<TypedArrayObject*> tarray(cx, &args.thisv().toObject().as<TypedArrayObject>());

Even though this is an array, please rename this to "O" to align with the spec. That makes it easier to follow the implementation.

@@ +540,2 @@
>          Rooted<TypedArrayObject*> tarray(cx, &args.thisv().toObject().as<TypedArrayObject>());
> +        /* Step 2?? */

As explained above, this is part of the first step. (Actually, it happens as part of the check if the receiver is a typed array, so isn't visible in this function at all, but that's an unimportant detail.)

@@ +544,2 @@
>          uint32_t srcEnd;
> +        uint32_t target;

Rename these to match the spec and introduce them right before they're used: before step 6. They should be called "to" (for "target"), "from" (for "srcStart") and "final" (for "srcEnd").

@@ +548,1 @@
>          uint32_t originalLength = tarray->length();

Change this to "len" to match the spec.

@@ +548,4 @@
>          uint32_t originalLength = tarray->length();
> +
> +        if (!ToClampedIndex(cx, args[0], originalLength, &target) ||  /* Steps 6-8 */
> +            !ToClampedIndex(cx, args[1], originalLength, &srcStart)   /* Steps 9-11 */)

Move the steps comments above the lines they're comments for. I.e. "// Steps 6-8." above the if, "// Steps 9-11." on a line of its own after the `||`.

This applies to the comments below, too.

@@ +555,5 @@
>  
> +        if (args.get(2).isUndefined()) {  /* Step 12 */
> +          srcEnd = originalLength;
> +        }
> +        else if (!ToClampedIndex(cx, args[2], originalLength, &srcEnd)) {  /* Step 13-14 */

Nit: you wouldn't need a newline between `}` and `else`, but in this case you can just remove the braces entirely because both blocks fit on one line.

@@ +564,3 @@
>              JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_BAD_INDEX);
>              return false;
>          }

This entire block can be removed: the algorithm allows this case.

@@ +564,5 @@
>              JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_BAD_INDEX);
>              return false;
>          }
>  
> +        /* Step 15-18 */

Comment the steps individually below instead of having them all in this one comment.

@@ +569,2 @@
>          uint32_t lengthDuringMove = tarray->length(); // beware ToClampedIndex
> +        uint32_t nelts = srcEnd - srcStart;

Rename this to "count" and correctly implement step 15.

@@ +573,3 @@
>          MOZ_ASSERT(nelts <= INT32_MAX, "size limited to 2**31");
> +        if (target + nelts > lengthDuringMove) {
> +            nelts = lengthDuringMove - target;

By correctly implementing step 15 above, you can just remove this. I honestly don't understand what "lengthDuringMove" is there for: I don't see how it can ever be different from "originalLength" above.

@@ +590,5 @@
>          JS_ASSERT(byteDest + byteSize >= byteDest);
>          JS_ASSERT(byteSrc + byteSize >= byteSrc);
>  #endif
>  
> +

Nit: remove this blank line.

Comment 5

[Jeff Walden [:Waldo]]

(In reply to Till Schneidereit [:till] from comment #4)
> @@ +573,3 @@
> >          MOZ_ASSERT(nelts <= INT32_MAX, "size limited to 2**31");
> > +        if (target + nelts > lengthDuringMove) {
> > +            nelts = lengthDuringMove - target;
> 
> By correctly implementing step 15 above, you can just remove this. I
> honestly don't understand what "lengthDuringMove" is there for: I don't see
> how it can ever be different from "originalLength" above.

There are arcane reasons for this being here, that truly are necessary, that you probably don't really want to know about.  Suffice to say this *is* needed.

Please flag me for review on all the patches here when they're ready.

Comment 6

[Rishab Arora [:spacetime]]

Attachment: copywithin.patch

Thanks for the feedback everyone! I used all the test cases from Array#copyWithin's tests itself. The test file is a new file, not modified from any old file, so it doesn't need 'hg mv'. 

I've incorporated André's feedback. The only problem now is that I don't encounter a TypeError for copyWithin for proxies. Any suggestions on how to add a check for this? 

I've kept back "originalLength" but since I don't know the reason, I can't tell if this suffices. If everything else looks good, we can ask Jeff for a review.

Comment 7

[André Bargull [:anba]]

(In reply to Rishab Arora [:spacetime] from comment #6)
> I've incorporated André's feedback. The only problem now is that I don't
> encounter a TypeError for copyWithin for proxies. Any suggestions on how to
> add a check for this? 

Hmm, I forgot that SpiderMonkey auto-unwraps proxied objects, which means you can't really test the spec'ed behaviour. :-/
Whether or not the ES6 spec'ed behaviour is useful, is a different story... ;-)


> I've kept back "originalLength" but since I don't know the reason, I can't
> tell if this suffices. If everything else looks good, we can ask Jeff for a
> review.

I don't want to spoil Waldo's subtle comment, but you may need something along the lines of (untested!). :-D
---
if (final < from) {
  // count less than 0, return here.
  args.rval().set(args.thisv());
  return true;
}
uint32_t count = Min(final - from, len - to);
uint32_t lengthDuringMove = O->length(); // beware ToClampedIndex
if (from + count > lengthDuringMove || to + count > lengthDuringMove) {
  JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_TYPED_ARRAY_BAD_ARGS);
  return false;
}

// Proceed with copy...
---

Comment 8

[Sankha Narayan Guria [:sankha]]

(In reply to André Bargull from comment #7)

> Hmm, I forgot that SpiderMonkey auto-unwraps proxied objects, which means
> you can't really test the spec'ed behaviour. :-/

Can't we do something like:

if (IsProxy(args.thisv().toObject())) {
 // throw TypeError
 return false;
}

Comment 9

[Till Schneidereit [:till]]

(In reply to Sankha Narayan Guria [:sankha93] from comment #8)
> (In reply to André Bargull from comment #7)
> 
> > Hmm, I forgot that SpiderMonkey auto-unwraps proxied objects, which means
> > you can't really test the spec'ed behaviour. :-/
> 
> Can't we do something like:
> 
> if (IsProxy(args.thisv().toObject())) {
>  // throw TypeError
>  return false;
> }

Probably not: our cross-compartment wrappers are proxies, and I think we want the unwrapping behavior for those. We don't want it for content script-created proxies, however. Not sure how to go about this.

Comment 10

[Till Schneidereit [:till]]

> Probably not: our cross-compartment wrappers are proxies, and I think we
> want the unwrapping behavior for those. We don't want it for content
> script-created proxies, however. Not sure how to go about this.

Turns out this is a general problem with non-generic functions. I'll fix it in another bug, after which a test can just be:

a = new Uint8Array(10);
p = new Proxy(a, {});
p.copyWithin(1,2,3);

This will then throw "TypeError: copyWithin method called on incompatible Proxy".

Comment 11

[Rishab Arora [:spacetime]]

Attachment: Renamed typed arrays' move method to copyWithin, fixed up to ES6 semantics; Added tests for TypedArray#copyWithin

I've added the code (after testing) as suggested by André. Corrected minor whitespace issues.

Comment 12

[Till Schneidereit [:till]]

Will review later today.

Comment 13

[Till Schneidereit [:till]]

Comment on attachment 8453582 [details] [diff] [review]
Renamed typed arrays' move method to copyWithin, fixed up to ES6 semantics; Added tests for TypedArray#copyWithin

I'm afraid I wasn't able to find the time/focus to get through this review before going on PTO. :(

Waldo, this shouldn't need much more, and I think it's actually good if you take a look at it, given that it touches on the weird issues you've been dealing with.

Comment 14

[Jeff Walden [:Waldo]]

Comment on attachment 8453582 [details] [diff] [review]
Renamed typed arrays' move method to copyWithin, fixed up to ES6 semantics; Added tests for TypedArray#copyWithin

Review of attachment 8453582 [details] [diff] [review]:
-----------------------------------------------------------------

Shouldn't really be reviewing now, but I happen to be awake, so.  :-)

This is broadly on the right track, just some style nits, some actual bugs to fix, a test or two to add, and you should be good, modulo my checking up on one little spec question I have for Allen (the spec editor) to answer.

::: js/src/jit-test/tests/basic/typed-array-copywithin.js
@@ +15,5 @@
> +  Float32Array,
> +  Float64Array
> +];
> +
> +for (constructor of constructors) {

for (var constructor of constructors) {

@@ +17,5 @@
> +];
> +
> +for (constructor of constructors) {
> +
> +    assertEq(constructor.prototype.copyWithin.length, 2);

Should be 3.

@@ +51,5 @@
> +		  new constructor([1, 3, 4, 5, 5]));
> +
> +    // throws on null/undefined values
> +    assertThrowsInstanceOf(function() {
> +	constructor.prototype.copyWithin.call(null, 0, 3);

var throw42 = { valueOf: function() { throw 42; } };
constructor.prototype.copyWithin.call(null, throw42, throw42, throw42);

to ensure the TypeError throwing is right at the start and all.

@@ +56,5 @@
> +    }, TypeError, "Assert that copyWithin fails if this value is null");
> +
> +    assertThrowsInstanceOf(function() {
> +	constructor.prototype.copyWithin.call(undefined, 0, 3);
> +    }, TypeError, "Assert that copyWithin fails if this value is undefined");

Same sort of tweak as above here, too.

@@ +60,5 @@
> +    }, TypeError, "Assert that copyWithin fails if this value is undefined");
> +
> +    // test with this value as string
> +    assertThrowsInstanceOf(function() {
> +	constructor.prototype.copyWithin.call("hello world", 0, 3);

And here.

@@ +68,5 @@
> +    function f(a, b, c, d, e) {
> +	[].copyWithin.call(arguments, 1, 3);
> +	return new constructor([a, b, c, d, e]);
> +    }
> +    assertDeepEq(f(1, 2, 3, 4, 5), (new constructor([1, 4, 5, 4, 5])));

This test doesn't belong here, it's only testing Array.prototype.copyWithin which is a completely different implementation from this function.

@@ +83,5 @@
> +    var arr = new constructor(6);
> +    for (var i = 0; i < arr.length; i += 2) {
> +	arr[i] = i;
> +    }
> +    assertDeepEq(arr.copyWithin(0, 3), (new constructor([, 4, , , 4, , ])));

[].copyWithin test, remove.

@@ +157,5 @@
> +    assertDeepEq((new constructor([1, 2, 3, 4, 5])).copyWithin(0, 3, null),
> +        (new constructor([1, 2, 3, 4, 5])));
> +
> +    // test with a proxy object
> +    var proxyObj = {

Name this handler, please.

::: js/src/vm/TypedArrayObject.cpp
@@ +527,5 @@
>                                      ThisTypedArrayObject::fun_subarray_impl>(cx, args);
>      }
>  
> +    /* copyWithin(target, start[, end]) */
> +    // ES6 draft rev 25, 22.2.3.5

New step numbering in the draft that just issued last week -- I'm reviewing against those steps.  (Update: algorithm looks unchanged in new draft, but please update this comment to refer to this latest draft.)

@@ +534,3 @@
>      {
>          JS_ASSERT(IsThisClass(args.thisv()));
> +        if (args.length() < 2) {

This test and consequent block shouldn't exist.  Rather, the spec algorithm should act as if any argument referenced, but not provided, has the value |undefined|.  Please add a test for this:

var tarray = new Uint8Array(8);
try
{
  tarray.copyWithin({ valueOf: function() { throw 42; } });
  throw new Error("expected to throw");
}
catch (e)
{
  assertEq(e, 42, "should have failed converting target to index");
}

@@ +537,5 @@
>              JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_TYPED_ARRAY_BAD_ARGS);
>              return false;
>          }
> +        // Steps 1-2.
> +        Rooted<TypedArrayObject*> O(cx, &args.thisv().toObject().as<TypedArrayObject>());

Contra till, we usually use |obj| as the name for objects named |O| in the spec.  The deviation hurts comparisons against the spec, but that's well outweighed by not having to worry about confusion with literal zero.

@@ +548,5 @@
> +        uint32_t len = O->length();
> +
> +        // Steps 6-8.
> +        if (!ToClampedIndex(cx, args[0], len, &to) ||
> +        // Steps 9-11.

I'd prefer seeing from/final/to declared right next to their computations:

        // Steps 6-8.
        uint32_t to;
        if (!ToClampedIndex(cx, args.get(0), len, &to))
            return false;

        // Steps 9-11.
        uint32_t from;
        ...

(Note args.get(0) given the above comment.)

@@ +549,5 @@
> +
> +        // Steps 6-8.
> +        if (!ToClampedIndex(cx, args[0], len, &to) ||
> +        // Steps 9-11.
> +            !ToClampedIndex(cx, args[1], len, &from))

I was going to say put these both on one line, but with args.get(#) being necessary it doesn't fit.

@@ +563,1 @@
>              return false;

SpiderMonkey styling nits:

        // Steps 12-14.
        uint32_t final;
        if (args.get(2).isUndefined()) {
            final = len;
        } else {
            if (!ToClampedIndex(cx, args[2], len, &final))
                return false;
        }

@@ +563,4 @@
>              return false;
> +
> +        // Steps 15-18.
> +        if (final < from) { // count less than 0, return here.

Slight adjustments:

        // Steps 15-18.

        // If |final - from < 0|, then |count| will be less than 0, so step 18
        // never loops.  Exit early so |count| can use a non-negative type.
        // Also exit early if elements are being moved to their pre-existing
        // location.
        if (final < from || to == from) {
            args.rval().setObject(*obj);
            return true;
        }

Slight preference for returning |obj| explicitly this way, versus implicitly through its being |this| as well.

...or actually, there's a spec issue here that I need to resolve before I can say the above is exactly what you should use here.  Proceed with the above assumption for now, but the next round of review (after I do some spec-poking) I might ask you to change even from this algorithm.

@@ +569,2 @@
>          }
> +        uint32_t count = Min(final - from, len - to);

Blank line before this, to separate the conceptual pieces of the steps being implemented here.

@@ +574,5 @@
>              return false;
>          }
>  
> +        MOZ_ASSERT(to <= INT32_MAX, "size limited to 2**31");
> +        MOZ_ASSERT(count <= INT32_MAX, "size limited to 2**31");

These assertions aren't in the right place.  They're to verify that the additions above -- |from + count|, |to + count| -- don't wrap around.  Having them here doesn't help that.  Please move them back where they were, i.e. just before the above if.  (And please add the same style of assertion for |count|, too.)

@@ +578,5 @@
> +        MOZ_ASSERT(count <= INT32_MAX, "size limited to 2**31");
> +
> +        uint32_t byteDest = to * sizeof(NativeType);
> +        uint32_t byteSrc = from * sizeof(NativeType);
> +        uint32_t byteSize = count * sizeof(NativeType);

Hmm.  So these multiplications imply a second round of assertions, to verify that these multiplications can't overflow.  This is the case because typed array byte length in SpiderMonkey is capped at INT32_MAX, but we'll probably change that at some point.  So I think you need

        const size_t ElementSize = sizeof(NativeType);
        MOZ_ASSERT(to <= INT32_MAX / ElementSize, "overall byteLength capped at INT32_MAX");
        MOZ_ASSERT(from <= INT32_MAX / ElementSize, "overall byteLength capped at INT32_MAX");
        MOZ_ASSERT(count <= INT32_MAX / ElementSize, "overall byteLength capped at INT32_MAX");

@@ +593,5 @@
>          JS_ASSERT(byteSrc + byteSize >= byteSrc);
>  #endif
>  
> +        uint8_t *data = static_cast<uint8_t*>(O->viewData());
> +        memcpy(&data[byteDest], &data[byteSrc], byteSize);

Please use

PodMove(&data[byteDest], &data[byteSrc], byteSize);

so as to get non-overlapping assertions.  memcpy *requires* the ranges not overlap, but it's totally possible for them to here, so we have to use a move-ing sort of function.  And we want PodMove over memmove because of its better typing than memmove (wrt src/dest typing being not just void*).

@@ +2054,3 @@
>  #else
>  # define EXPERIMENTAL_FUNCTIONS(_t)
>  #endif

Get rid of this whole thing and just use

    JS_FN("set", _typedArray##Object::fun_copyWithin, 3, JSFUN_GENERIC_NATIVE),

where this macro was being used.  |move| was experimental, but this is in a spec and was the consequence of discussion of our experiment, so it should be okay to expose generally.

Note the 2=>3 change here -- that's because the spec method has (target, start, end = this.length) in it, and chapter 17 says length is the number of named arguments in the signature (not including a ...rest argument).  We need a test for this (or perhaps an existing test must be updated).

Comment 15

[Rishab Arora [:spacetime]]

Attachment: Renamed typed arrays' move method to copyWithin, fixed up to ES6 semantics; Added tests for TypedArray#copyWithin

Thanks for the review, Waldo. I've fixed the issues you mentioned.
If the spec changes after your discussion with Allen, I'll add that.

Comment 16

[Jeff Walden [:Waldo]]

Talked with Allen about this, but I've been too distracted by other things to properly follow up on this.  It's on the todo list for Real Soon Now.

Comment 17

[Jeff Walden [:Waldo]]

Attachment: Final patch

No change needed to follow up on Allen's comments, so this is basically good to go.  "Basically".

In making that assumption, I did a few last bits of once-overing here, partly to double-check the changes, partly because it never hurts.  A few new -- but small -- things popped up.

Re "Should be 3", I made that comment on another patch recently and was reminded that I'd forgotten to heed spec language that overrode the section-heading instructions.  So I checked here -- and it definitely says copyWithin has length 2.  I reverted to what you originally had.  Sorry for misleading!  :-(

In the throw-42 cases, I think I meant to have the new tests *in addition* to the existing ones.  I made that change in this patch.

I meant for the name to be "obj", not "Obj".  Capitalized names are for enums and classes, pretty much.  Renamed to "obj".

Interestingly, looking at this in an editor, I get syntax highlighting of |final|, because in C++ it's a contextual keyword to mark a virtual function as not overridable in base classes, or to mark a class as uninheritable.  But it's only *contextual*, so technically the highlighting is a lie.  A little screwy, I guess, but it's the right name and not forbidden.  So no name-change here.

The |to <= INT32_MAX / ElementSize| sorts of assertions should precede the multiplications they're designed to check.  Moved them.

When I tried running this, I found the proxy test was failing.  On a second look I think that might be what's supposed to happen, so I commented it out for now.  We can follow up on that and either remove it entirely or fix it in a new bug.

Also in terms of tests, js/src/tests/js1_8_5/extensions/typedarray.js needed updating for this function-renaming and signature change.  I made the necessary changes in this patch.

Last bits, on the style side of things: I converted a bunch of tab characters into spaces (they made the test unreadable in my editor in places), got rid of some noise-inducing parentheses in a few places, vertically aligned the first argument and the second arguments to assertDeepEq, and renamed the test file to copyWithin instead of copywithin.  Nothing major, just little things.

Tryservering happening now -- when this passes, I'll land it:

https://tbpl.mozilla.org/?tree=Try&rev=4ec566a275f9

Comment 18

[Jeff Walden [:Waldo]]

And one last fix beyond that, for an xray test that wanted the old method name.

https://hg.mozilla.org/integration/mozilla-inbound/rev/712980bf7962

Thanks for the patchwork, and sorry for the delay in getting on with this.  :-(

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/712980bf7962

Comment 20

[Florian Scholz (Open Web Docs)]

Release notes:
https://developer.mozilla.org/en-US/Firefox/Releases/34#JavaScript

Updated reference:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray/copyWithin
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray/move (marked as obsolete)

Also mentioned linked from constructor pages {{Ui,I}nt{8,16,32}Array,Float{16,32}Array}
E.g. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Int8Array