ArrayBuffer() & ArrayBuffer("some non-number") doesn't match implementations

[bterlson]

@anba contributed these tests and highlighted an issue. See also this v8 issue. No browser as far as I can tell implements the required ToNumber semantics. Considering we already have interoperable implementation here I doubt this will change. Seems like we should just do ToInteger rather than the ToNumber/ToLength incantation presently in 22.2.1.2. Thoughts?

[littledan]

Interestingly, as far as I can tell, ToLength was added exactly to support large TypedArrays. I think we should remain compatible with web usage and what implementations do, but I wonder if we can do it in a way which generally preserves some kind of consistency with ToLength.

As a strawman, what if we just replaced lines 3-6 with a simple ToLength call? That would handle coercing all sorts of things to 0, while guaranteeing that we're left with a positive integer. It would be a minor change vs current browser behavior, but as far as I can tell, it would only be in the direction of not throwing on negative numbers (where V8, for one, currently throws). An alternative would be

1. ToInteger
2. Check if it's < 0, if so throw
3. ToLength of that

which would leave us with that nice, higher level of checking, and verifying that the result is within (2**53)* range, while still allowing undefined etc to be coerced in a web-compatible way.

* To be unambiguous, I put parens around all exponentiation

[littledan]

Another place where this comes up is DataView setting/getting indices. Across all four browsers (at least for the last Safari release, and more recent versions of all other browsers), if I try to set something at the 'NaN' index, no exception is thrown. However, a new set of tests in the same patch by @anba highlighted that this violates the spec.

[zloirock]

Currently, I use this inconsistency as an ultimative feature detection for wrapping ArrayBuffer for validation in the next version of core-js, so, please, resolve this issue asap :)

[anba]

First of all I think it's hard to believe web-compatibility is affected when new ArrayBuffer("I don't even know what I'm doing") throws an error. It may even help to catch misuses like this one 😛 .

But more importantly argument input validation should be consistent across ArrayBuffer, DataView and TypedArray, because all three are used together [0]. So if it gets decided that new ArrayBuffer(1.4) should not throw an error, we may also consider to remove the error when calling new Int8Array(1.4). And IIRC input validation for these constructors has been discussed multiple times in the past, it's probably useful to check for es-discuss and meeting notes to find out why the validation steps were added in the first place.

Current situation for ArrayBuffer(length):
Edge, JSC and SM use ToInt32/ToUint32 for new ArrayBuffer(1.4), V8 calls ToInteger and performs target platform specific checks [1].

Current situation for TypedArray(length):
JSC and SM only allow integer numbers (here and here), Edge only allows numbers and calls ToInt32, V8 calls ToInteger (here [2]) and performs a range check for SMIs.

[0] It's already not quite consistent when you compare the length parameter checks in 22.2.1.2 and 22.2.1.5.
[1] I wonder if that check is actually valid for 64-bit when value is near size_t-max, e.g. new ArrayBuffer(0x10000000000000000).byteLength returns 0.
[2] And there is a missing check for symbol values in this line. Symbol.prototype[Symbol.iterator] = function*(){ print("hi") }; new Int8Array(Symbol()) shouldn't print "hi" on the console. 😉

[zloirock]

Ok. The first day of core-js 2.0 and looks like the first problem with this inconsistency (%TypedArray% constructor, but with the same change).

[zloirock]

@anba

[2] And there is a missing check for symbol values

Also, for example, null Uncaught TypeError: Cannot read property 'Symbol(Symbol.iterator)' of null

[zloirock]

So... With this change for %TypedArray% constructor in Node.js in some cases will not work ws module -> socket.io -> dependent modules because passes to Buffer -> Uint8Array floats.

[domenic]

Do we consider this fixed with #410 ?

[leobalter]

yes, this can be closed