runtime: migrate array-like lengths to Int64#461
Conversation
|
Warning Review limit reached
More reviews will be available in 8 minutes and 52 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (22)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 628e621550
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let _ = interp.define_own_property( | ||
| target, | ||
| index.to_string(), | ||
| value, | ||
| String_(index.to_string()), | ||
| PartialDescriptor::data_default(value), | ||
| @token.Loc::default(), | ||
| strict=true, | ||
| ) |
There was a problem hiding this comment.
For custom species methods such as an Array subclass whose Symbol.species constructor returns Object.preventExtensions({}), CreateDataPropertyOrThrow must throw if defining the result index returns false. This branch now ignores the Boolean from define_own_property, so map/slice/concat can silently return a non-extensible result without the copied element (and then set length), whereas the old strict set_property path raised.
Useful? React with 👍 / 👎.
| if new_len > ARRAY_MAX_SAFE_LENGTH_I64 { | ||
| raise @errors.TypeError(message="Invalid array length") |
There was a problem hiding this comment.
When len + insertCount - actualDeleteCount exceeds the max safe integer, the spec requires the TypeError before ArraySpeciesCreate, but this new check runs only after creating and populating the deleted array. With a custom constructor[Symbol.species], splice will call user code and may copy deleted values before throwing, so overflow cases have observable side effects that should not occur.
Useful? React with 👍 / 👎.
Benchmark ResultsRun: https://github.com/dowdiness/js_engine/actions/runs/28181056771
Stage summary
Focused bytecode base-vs-head comparison
Base-vs-head comparison
Mean-time chart (log scale)
Closure-conversion comparison
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a243dc57cd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } else if n.is_inf() { | ||
| if n < 0.0 { | ||
| -ARRAY_MAX_SAFE_LENGTH_I64 |
There was a problem hiding this comment.
When n is -Infinity, saturating it to -ARRAY_MAX_SAFE_LENGTH_I64 makes callers that add the index to a max-safe length treat it as index 0. For example, Array.prototype.at.call({0: 'x', length: 9007199254740991}, -Infinity) would return 'x', and lastIndexOf(..., -Infinity) can search index 0, but ToIntegerOrInfinity should keep this out of range. Preserve an infinity sentinel or special-case these relative-index callers instead of clamping negative infinity.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c6106304f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let o = to_object(source, interp) | ||
| let len = @runtime.to_array_like_length_interp(o, interp) | ||
| for i = 0; i < len; i = i + 1 { | ||
| for i = 0L; i < len; i = i + 1L { |
There was a problem hiding this comment.
When Array.from is used on a non-iterable array-like whose length is now representable as an Int64 but is too large for an Array result, this loop reads index properties before the result constructor can reject the length. For example, Array.from({ length: 2 ** 32, get 0() { throw err; } }) should throw the Array constructor's RangeError before accessing 0, but this path invokes the getter first (or loops billions of times if there is no early throw). Add the ArrayCreate/constructor length check before entering the array-like element loop.
Useful? React with 👍 / 👎.
| let array_like = to_object(source, interp) | ||
| let len = @runtime.to_array_like_length_interp(array_like, interp) | ||
| for i = 0; i < len; i = i + 1 { | ||
| for i = 0L; i < len; i = i + 1L { |
There was a problem hiding this comment.
The array-like branch of TypedArray.from now accepts an Int64 length but still collects every source element before constructing the typed-array result. For sources with a length the constructor would reject in this runtime, such as Int8Array.from({ length: 2147483648, get 0() { throw err; } }), the getter runs or the loop hangs instead of throwing the constructor/allocation RangeError before element access. Validate/construct from len before this collection loop.
Useful? React with 👍 / 👎.
1 participant
Summary
InttoInt64DefineOwnProperty, own-key ordering, and species creationIntint64-array-index (#307)Test262 skip entries now covered by the migrationRefs #392, #307.
Testing
moon info && moon fmtmoon checkmoon test(2223 passed)32/32passed,0skippedmake test262-validate-skipsgit diff --checkKnown baseline issue
make architecture-auditstill fails on pre-existing representation allowlist/stale entries inbuiltins_object,builtins_map_set, andbuiltins_promise; this was present before this branch and is unrelated to Array index width: migrate ToLength/ToIndex from Int to Int64 for spec conformance #307.