Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Normative: Array.from should support arrays of 2**53-1 elements, rather than just 2**53-2 #2578

Open
nicolo-ribaudo opened this issue Nov 15, 2021 · 2 comments

Comments

@nicolo-ribaudo
Copy link
Member

nicolo-ribaudo commented Nov 15, 2021

Description:

All the array methods throw an error when they create an array longer than 2**53-1 (thus, when you exceed the index 2**53-2). However, Array.from(iterable) throws one element earlier:

https://tc39.es/ecma262/#sec-array.from

e. Repeat,
    i. If k ≥ 2**53 - 1, then
        1. Let error be ThrowCompletion(a newly created TypeError object).
        2. Return ? IteratorClose(iteratorRecord, error).
   ii. Let Pk be ! ToString(𝔽(k)).
  iii. Let next be ? IteratorStep(iteratorRecord).
   iv. If next is false, then
        1. Perform ? Set(A, "length", 𝔽(k), true).
        2. Return A.
    v. Let nextValue be ? IteratorValue(next).
   ... other steps to add a new element with nextValue

This code always checks if we reached the length 2**53-1, but it throws before making sure that we are actually exceeding it. Looking at #352, it seems like the intention was to throw when exceeding 2**53-1.

I think step e.i should be moved after step e.iv.

Additionally, I think we should introduce a similar check in the Array.from(arrayLike) case (Array.from({ length: 2**53 }) should throw), but that's a bigger normative change.

eshost Output:

$ eshost -e "Array.from(function*() { for (let i = 0; i < 2**53-1; i++) yield i;}())" -t

┌────────────────┬───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐
│ ChakraCore     │                                                                                                                                                                                       │
├────────────────┼───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Hermes         │                                                                                                                                                                                       │
│                │ HermesGC: OOM: [] reason = Max heap size was exceeded (1 from category: vm_allocate_category), numCollections = 3968, heapSize = 1073741824, allocated = 1051421752, va = 1073741824. │
├────────────────┼───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ JavaScriptCore │                                                                                                                                                                                       │
├────────────────┼───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Moddable XS    │                                                                                                                                                                                       │
├────────────────┼───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ SpiderMonkey   │                                                                                                                                                                                       │
├────────────────┼───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ V8             │                                                                                                                                                                                       │
│                │ RangeError: Invalid array length                                                                                                                                                      │
└────────────────┴───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘

I don't understand these eshost results, and in Node.js it segfaults.

@gibson042
Copy link
Contributor

Probably related to #2502.

@bathos
Copy link
Contributor

bathos commented Jan 3, 2022

@nicolo-ribaudo if that’s ordinary Array, wouldn’t a RangeError be the expected outcome?

  • The max array index is (the canonical numeric string of) 2 ** 32 - 2
  • Defining properties with keys whose numeric value is beyond this range works — but they’re just ordinary properties (AEO [[DefineOwnProperty]] would exit via step 3)
  • But: setting a length greater than 2 ** 32 - 1 would lead to throwing a RangeError (ArraySetLength steps 3-5)
  • Array.from would try to set such a length when it finishes consuming the iterator (step 5.e.iv.1)

(I’d guess the reason from permits 2 ** 53 - 1 is because it’s meant to be “generic” rather than because Array itself is supposed to also permit it, though I dunno the history.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants