Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.
Sign upTypedArray ArrayBuffer constructor allowing negative length? #468
Comments
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
domenic
Mar 10, 2016
Member
I don't have any specific input on this issue but I wanted to share a strategy that we often use for similar things to the typed array situation, where we're investigating how well a new-ish spec matches a long legacy of not-so-robust implementations. What we do is construct a quick test case in jsbin or live dom viewer, then create a Markdown table and fill in what we can with the test results and ask others to fill on cells for browsers we don't have on our computer (like Safari or Edge).
You can see some examples at whatwg/html#823 (comment) and whatwg/html#775 (comment).
This might be helpful for finding out if V8's check is a minority among browsers and the spec matches the others, or if all browsers agree and the spec mismatches reality, or what.
|
I don't have any specific input on this issue but I wanted to share a strategy that we often use for similar things to the typed array situation, where we're investigating how well a new-ish spec matches a long legacy of not-so-robust implementations. What we do is construct a quick test case in jsbin or live dom viewer, then create a Markdown table and fill in what we can with the test results and ask others to fill on cells for browsers we don't have on our computer (like Safari or Edge). You can see some examples at whatwg/html#823 (comment) and whatwg/html#775 (comment). This might be helpful for finding out if V8's check is a minority among browsers and the spec matches the others, or if all browsers agree and the spec mismatches reality, or what. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
allenwb
Mar 10, 2016
Member
This appears to be a bug, that took a couple to steps to introduce.
Prior to ES6 Rev17 ToNumber rather than ToLength was applied to the length argument and a RangeError was thrown if the result number was < 0. See 15.13.6.1.4 of Rev 17.
Bug 2171 pointed out that the test was unnecessary because ToLength never returns a value < 0. So the check was deleted in Rev 21 22.2.1.4
The original bug was the simple replacement of the ToNumber with the ToLength. The length argument should probably be handled similarly to what is currently done in 22.2.4.2
|
This appears to be a bug, that took a couple to steps to introduce. Prior to ES6 Rev17 ToNumber rather than ToLength was applied to the length argument and a RangeError was thrown if the result number was < 0. See 15.13.6.1.4 of Rev 17. Bug 2171 pointed out that the test was unnecessary because ToLength never returns a value < 0. So the check was deleted in Rev 21 22.2.1.4 The original bug was the simple replacement of the ToNumber with the ToLength. The length argument should probably be handled similarly to what is currently done in 22.2.4.2 |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
littledan
Mar 10, 2016
Member
@allenwb Based on the rest of the TypedArray specification, that sounds entirely reasonable. However, I'm still not sure about the web compatibility of the general strategy of throwing an exception on non-integer arguments to various TypedArray/DataView/ArrayBuffer-related functions, which is under discussion at #265 . (I believe @zloirock has found some compatibility issues when he tried to ship these semantics in core.js.) For now, I'll hold off on allowing negative lengths in V8, and I'd be in favor of the spec change that @allenwb is suggesting.
|
@allenwb Based on the rest of the TypedArray specification, that sounds entirely reasonable. However, I'm still not sure about the web compatibility of the general strategy of throwing an exception on non-integer arguments to various TypedArray/DataView/ArrayBuffer-related functions, which is under discussion at #265 . (I believe @zloirock has found some compatibility issues when he tried to ship these semantics in core.js.) For now, I'll hold off on allowing negative lengths in V8, and I'd be in favor of the spec change that @allenwb is suggesting. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
littledan
Mar 10, 2016
Member
The compat issue: karma-runner/karma#1768
@leobalter Would you be interested in writing up the spec fix that @allenwb is suggesting?
|
The compat issue: karma-runner/karma#1768 @leobalter Would you be interested in writing up the spec fix that @allenwb is suggesting? |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
|
Sure, I'll check it tomorrow. |
leobalter
referenced this issue
Mar 11, 2016
Merged
Normalize TypedArray, ArrayBuffer and DataView constructors #410
bterlson
added
the
discussion
label
Mar 18, 2016
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
bakkot
Aug 17, 2016
Contributor
I believe this is fixed after #410: length is now passed through ToIndex, which throws on negative inputs.
|
I believe this is fixed after #410: |
littledan commentedMar 10, 2016
Was this intended? In https://tc39.github.io/ecma262/#sec-typedarray-buffer-byteoffset-length , a number of the arguments are checked to be greater than or equal to 0, but the length simply has ToLength applied to it, without this check. V8 currently checks that the argument is >= 0. Unlike other places where the ES2017 draft specification asks us to be more strict than we currently are, this asks us to be more lenient. I'd be fine to implement it, but before weakening the restrictions and releasing this to the web, I have to ask, was this intentional? When it is web-compatible, I like the idea of making/keeping interfaces strict in their input validation. The negative length is checked to be possible by the test262 test
built-ins/TypedArrays/buffer-arg-defined-negative-length@allenwb