Changing behavior of Array#copyWithin when used on array-like with negative length

# C. Scott Ananian (10 years ago)

For reference: ecmascript#2546

Array#copyWithin has a (non-normative) signature of (target, start, end = this.length). However, this is slightly misleading because the spec actually calls ToLength on this.length and then uses that for the default value of end. This changes end's effective value when this.length is negative.

In the bug we discuss changing the non-normative descriptive text to be less misleading.

But I'd like to invite broader discussion on a different approach: can we change the spec so that the end = this.length default was actually correct? This would only possibly change behavior on array-likes with negative length, and probably wouldn't even change observable behavior in that case (since length is treated as 0). Basically we'd be just calling ToInteger on the default value of end rather than ToLength. But it would be an end-run around confusing language in the spec.

# Allen Wirfs-Brock (10 years ago)

ToLength is used a number of places within the ES6 spec. where formerly ToUint32 was used. It allows indices to be larger 2^32-2 and avoids weird wrap behavior for indices in that range. I doubt that we could compatibly get away with replacing those legacy ToUnit32 calls with a ToLength that preserved negative values. Even if we could we would have to review all of the array (and string) algorithms that use ToLength to make sure they still work reasonably with negative length values. I really don't see what benefit we would get from that work.

# André Bargull (10 years ago)

I think Scott is requesting this change: gist.github.com/anba/6c75c34c72d4ffaa8de7

# Claude Pache (10 years ago)

Le 14 févr. 2014 à 21:46, "C. Scott Ananian" <ecmascript at cscott.net> a écrit :

array-likes with negative length

"Array-likes with negative length" doesn't make sense, or at least it isn't a useful concept as far as ECMAScript is concerned – as it doesn't make sense to consider arraylikes of fractional length, or of length equal to the string "LOL".

If the informal signature of Array.prototype.copyWithin is misleading, it could be rewritten as:

Array.prototype.copyWithin (target, start, end = length of this)

where "length of obj" is to be interpreted as ToLength(obj.length) – at least, it is what the algorithm uses in each and every step where the "length" is expected (the len variable in steps 8, 11, 12, 14 and 15).

# C. Scott Ananian (10 years ago)

On Fri, Feb 14, 2014 at 11:50 AM, André Bargull <andre.bargull at udo.edu> wrote:

I think Scott is requesting this change: gist.github.com/anba/6c75c34c72d4ffaa8de7

Yes, although my proposed diff (in the linked bug) was the shorter, "12. If end is undefined, let relativeEnd be ToInteger(lenVal); else let relativeEnd be ToInteger(end)." Same effect, though.

Claude Pache wrote:

"Array-likes with negative length" doesn't make sense.

Array.prototype.copyWithin.call({ length: -1 }, ... );

Call it whatever you like, although I'm always interested in learning new phrases of ECMAspeak (if there's an appropriate name for this).

# Claude Pache (10 years ago)

Le 14 févr. 2014 à 23:40, "C. Scott Ananian" <ecmascript at cscott.net> a écrit :

Call it whatever you like, although I'm always interested in learning new phrases of ECMAspeak (if there's an appropriate name for this).

Since you are interested: "The abstract operation ToLength converts its argument to an integer suitable for use as the length of an array-like object." 1

It doesn't really matter whether ToLength could extract a reasonable value for an unreasonable object, or not. But consistency is important: note that this operation is used whenever the length of an arraylike is expected, not only for default arguments.

# C. Scott Ananian (10 years ago)

Would "an array-like with a negative length field" read better in ecmaspeak? I'm talking about the value before ToLength is invoked. Array.prototype.slice.call(3) and Array.prototype.slice.call({length:-1}) both complete successfully (and return the same value), due to the soothing effect of ToLength. My informal definition for "array-like" is "something which is not actually an Array but which will not throw an error when used as the receiver in an invocation of a generic method of Array", so both 3 and {length:-1} are "array-like" under my definition, and then I need a way to describe the difference between them. I believe you are saying they are both "array-likes with zero length", since that's the result of ToLength. But perhaps you wouldn't call them "array-like" at all?

# C. Scott Ananian (10 years ago)

On Fri, Feb 14, 2014 at 12:40 PM, C. Scott Ananian <ecmascript at cscott.net> wrote:

Yes, although my proposed diff (in the linked bug) was the shorter, "12. If end is undefined, let relativeEnd be ToInteger(lenVal); else let relativeEnd be ToInteger(end)." Same effect, though.

No objections from implementors? Has anyone actually implemented copyWithin (other than in es6-shim)?

# Claude Pache (10 years ago)

Le 14 févr. 2014 à 23:40, C. Scott Ananian <ecmascript at cscott.net> a écrit :

Yes, although my proposed diff (in the linked bug) was the shorter, "12. If end is undefined, let relativeEnd be ToInteger(lenVal); else let relativeEnd be ToInteger(end)." Same effect, though.

Just a last note. Beyond the philosophical aspect whether arraylikes of negative length make any sense at all, there is a strong technical issue you have probably overlooked: For array methods in general, and for the optional argument of Array.prototype.copyWithin in particular (see step 14 of the algorithm), a negative index is not understood as an absolute position, but rather as a position relative to the end of the array. For instance, for an array (or arraylike) of length n, if -3 is passed, it indicates position n-3. In your case, it is certainly not the semantics you want. (In fact, everything will happily coerce to 0 at the end of the journey, but it's just happenstance.)

# C. Scott Ananian (10 years ago)

I have not overlooked it. The fact that negative end is significant is exactly the reason why using ToLength on it seems wrong. Even thought end defaults to this.length, it should still be normalized with ToInteger since negative values are semantically valid.

But as you point out, I don't think there's any actual behavior change, since everything washes out to 0 at the end. It's just a matter of writing a clearer more consistent spec.

# Brendan Eich (10 years ago)

C. Scott Ananian wrote:

But as you point out, I don't think there's any actual behavior change, since everything washes out to 0 at the end. It's just a matter of writing a clearer more consistent spec.

Yet in that light you still have a point, I think. Allen?

# André Bargull (10 years ago)

On 2/14/2014 11:40 PM, C. Scott Ananian wrote:

Yes, although my proposed diff (in the linked bug) was the shorter, "12. If end is undefined, let relativeEnd be ToInteger(lenVal); else let relativeEnd be ToInteger(end)." Same effect, though.

It's not the same effect, because lenVal could be an object with valueOf/toString/@toPrimitive side-effects.

# Allen Wirfs-Brock (10 years ago)

On Feb 17, 2014, at 12:25 PM, Brendan Eich wrote:

Yet in that light you still have a point, I think. Allen?

The ticket is still open, but it really is a style issue and if a change is made here consistency probably requires changes to other methods as well. Right now I have bigger fish to finish frying but at some point I'll consider it.

# C. Scott Ananian (10 years ago)

It's not the same effect, because lenVal could be an object with valueOf/toString/@toPrimitive side-effects.

Point taken. (Although I'm fine with invoking the side effects twice if you're using this.length as a default value, since that would be 'unsurprising' if you are looking at the method signature.)

Allen: I can volunteer to offload some of the work of auditing for similar cases with default arguments. From a quick read-through, only Array#fill seems to have the same issue. Array#lastIndexOf is written in the ES5 style, where we look at arguments.length instead of using default parameters in the signature. But it does use ToLength(this.length) - 1 as a default value; I think that could be changed to ToInteger(this.length) - 1 for consistency without affecting actual behavior.

The copyWithin, fill, and lastIndexOf, subarray methods of %TypedArray%.prototype are spec'ed with default arguments equal to this.length, but there's no way length can be negative there, I think (offtopic: ...although I'd really be much happier if the generic Array methods could be written to special-case %TypedArray% in the proper way so that we could observe DRY instead of cloning most of the algorithm descriptions).

# Claude Pache (10 years ago)

Le 17 févr. 2014 à 21:25, Brendan Eich <brendan at mozilla.com> a écrit :

Yet in that light you still have a point, I think. Allen?

Different perspectives... My point of view is precisely that, with the change proposed, the algorithm is less clear, and less consistent, for the reasons stated in this thread. The benefit of the change is that the signature Array.prototype.copyWithin (target, start, end = this.length) doesn't lie for silly values of this. Is it worth the added obscurity and inconsistency of the algorithm itself for these same cases?

But I think that clarity can be improved by avoiding to obscure the intent by computation details. For my personal polyfill, I have found useful to abstract out the following steps, used (with slight variations) thrice in Array.prototype.copyWithin and twice in Array.prototype.fill, (and which can be used for various other methods of Array.prototype):

IndexFromRelativeIndex(k, len, default)

  1. Assert: len is an integer between 0 and 2^53-1.
  2. Assert: default is an integer between 0 and len.
  3. If k is undefined, return default
  4. Let relative be ToInteger(k). ReturnIfAbrupt(relative).
  5. If relative >= 0, return min(relative, len).
  6. Else, if relative < 0, return max(len + relative, 0).

I think that the intent of this algorithm is clear despite its Cobol-like language. Actually, for step 3, the spec uses the equivalent of: "If k is undefined, let relative be default and go to step 5"; moreover, that step is omitted when default is 0. The places where Array.prototype.copyWithin 1 uses it, are (where len = ToLength(this.length) per step 4):

steps 6 to 8. Let to be IndexFromRelativeIndex(target, len, 0). ReturnIfAbrupt(to).
steps 9 to 11: Let from be IndexFromRelativeIndex(start, len, 0). ReturnIfAbrupt(from).
steps 12 to 14: Let final be IndexFromRelativeIndex(end, len, len). ReturnIfAbrupt(final).

Note in particular that the default value is always one of the two ends of the range of possible positions, either 0 or len.

# Claude Pache (10 years ago)

Le 17 févr. 2014 à 22:32, C. Scott Ananian <ecmascript at cscott.net> a écrit :

Allen: I can volunteer to offload some of the work of auditing for similar cases with default arguments. From a quick read-through, only Array#fill seems to have the same issue. Array#lastIndexOf is written in the ES5 style, where we look at arguments.length instead of using default parameters in the signature. But it does use ToLength(this.length) - 1 as a default value; I think that could be changed to ToInteger(this.length) - 1 for consistency without affecting actual behavior.

There is also Array#slice, which uses ToLength(this.length) as default for its second argument (even if its length is 2).

# C. Scott Ananian (10 years ago)

I like this refactoring. This doesn't change the spec's behavior; this would be the first solution proposed in ecmascript#2546 which was to rewrite the spec's non-normative text to make it clear that end=this.length in the signature was describing the behavior in the common case only. The description of Array#slice for example, does not use a default parameter in the signature and is accurate (assuming you read "the length of the array" as ToLength(this.length)).

On Tue, Feb 18, 2014 at 10:32 AM, Claude Pache <claude.pache at gmail.com> wrote:

I think that the intent of this algorithm is clear despite its Cobol-like language. Actually, for step 3, the spec uses the equivalent of: "If k is undefined, let relative be default and go to step 5"; moreover, that step is omitted when default is 0.

This difference seem to have no effect, given that step 2 asserts that default is an integer between 0 and len.

steps 6 to 8. Let to be IndexFromRelativeIndex(target, len, 0). ReturnIfAbrupt(to). steps 9 to 11: Let from be IndexFromRelativeIndex(start, len, 0). ReturnIfAbrupt(from).

For greater precision, you might want to specify +0 as the default here, to match the result specified for ToInteger(undefined).

# Brendan Eich (10 years ago)

Claude Pache wrote:

But I think that clarity can be improved by avoiding to obscure the intent by computation details. For my personal polyfill, I have found useful to abstract out the following steps, used (with slight variations) thrice in Array.prototype.copyWithin and twice in Array.prototype.fill, (and which can be used for various other methods of Array.prototype):

IndexFromRelativeIndex(k, len, default)

+1

Nit: How about just FromRelativeIndex for the name?