Improve tests for CreateByteDataBlock

[dewren99]

Please see the discussion on tc39/ecma262#3052

[ptomato]

Thanks for your contribution! This looks correct to me.

Personally I didn't mind having the 2⁵³-1 case in the test corpus as a test of "If it is impossible to create such a Data Block, throw a RangeError exception", but I can also see the point of view that it is not a 100% reliable test.

[ptomato]

I'm not sure what the [NotSupported] ArrayBuffer output means in esmeta. @jhnaldo is this an expected failure?

[anba]

Please revert the changes to this file, because tc39/ecma262#3052 can't be tested using the ArrayBuffer constructor:

The ArrayBuffer constructor applies ToIndex on its input parameter. ToIndex throws a RangeError when the input number is >2⁵³-1. So the observed RangeError after modifying the file isn't thrown from CreateByteDataBlock, but instead it's thrown from ToIndex.

[michaelficarra]

because tc39/ecma262#3052 can't be tested using the ArrayBuffer constructor:

@anba That's true, but we're also not going to advance an iterator 2⁵³ times to hit that guard. These tests are still valid, though, and an improvement. How do you suggest we test tc39/ecma262#3052?

[anba]

Yes, they're valid, but they test a different code path. If we want to keep them, they should be placed into a new file.

How do you suggest we test tc39/ecma262#3052?

TypedArray constructors should hit the new range check in CreateByteDataBlock:
TypedArray
→ AllocateTypedArray
→ AllocateTypedArrayBuffer
→ AllocateArrayBuffer
→ CreateByteDataBlock

AllocateTypedArrayBuffer performs elementSize × length, where length is a user-input coerced through ToIndex and elementSize is an integer in [1, 2, 4, 8]. So new Int16Array(2 ** 52) should hit the new range check, right?

[dewren99]

Hey @michaelficarra @ptomato, just letting you know since I had your approval for the PR, if @anba's suggestion sounds good I will start working on creating the tests under TypedArray constructor(s).

Do you still want to merge these improvements to CreateByteDataBlock tests under a different PR or should I keep this PR open?

[michaelficarra]

@anba You're right, I wasn't aware of the multiplication in AllocateTypedArrayBuffer step 3. So there's a way to trigger it other than iterating. Thanks.

@dewren99 I still like these changes as-is, but they don't exercise tc39/ecma262#3052. It's up to @ptomato and @tc39/test262-maintainers whether we should include those tests in this PR or make a separate one. If you want to get started right away, it'd probably be safest to just open a separate PR with those tests.

[ptomato]

It's fine to include them in this PR as far as I'm concerned. Are you planning to adopt @anba's suggestion of placing these tests in a different file, as well?

[dewren99]

@ptomato I haven't had the chance to look if there are already tests for TypedArray constructor(s), but if there are, I was thinking about adding the new tests there. Otherwise I was going to create a new file for the tests.

Or do you have recommendation on where I should add them? If it fine I can just update this PR to include new tests too.

[ptomato]

I'm not too familiar with the TypedArray constructors tests, but @jugglinmike might be able to answer that question.