fix(runtime): preserve declared receiver length semantics - #7862
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe change adds property-semantic ChangesLength property semantics
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
proggeramlug
marked this pull request as ready for review
August 11, 2026 15:33
proggeramlug
pushed a commit
that referenced
this pull request
Aug 11, 2026
proggeramlug
added a commit
that referenced
this pull request
Aug 11, 2026
…et a declared array type reach the guarded element read and the inline .length (#7890) * perf(codegen): interp 0.780 -> 0.675 s, iso_miss 1.061 -> 0.967 s — let a declared array type reach the guarded element read and the inline .length Two halves of one mechanism: what a program may do with an array type that came from an annotation rather than from an initializer that proved an array. A. `e.vals[i]` / `p.toks[p.pos]`. #7854 recovered a receiver's declared property type for a LOCAL (`const names = e.names`), never for the read used directly as a receiver — the HIR types a PropertyGet off a UNION receiver as `Any`, so `index_get.rs` routed those to `js_dyn_index_get`. The tier this unlocks, `lower_guarded_array_index_get`, re-checks GC_TYPE_ARRAY, forwarding, descriptors, the prototype latch and the bounds on the receiver itself, so a violated claim costs a branch and returns the same answer. B. `.length` no longer refuses a declared-only array local. #7854 refused them because the arm's fallback was `js_value_length_f64`, which answered 0 where JS answers `undefined` and did not throw on a nullish receiver (#7853). #7862 replaced that fallback with `js_value_length_property_f64` and left the refusal standing. `declared_only_array_locals` and `refined_array_type_is_declared_only` are deleted with it; `declared_only_numeric_locals` (#7773) is untouched. Quiet M1 mini, best-of-5, exit-checked: interp 0.7796 -> 0.6748 (-13.4%), iso_miss 1.0607 -> 0.9670 (-8.8%). 17 of the 19 corpus programs compile byte-identically and the two that differ are exactly the two with a type-alias over an array; noise floor from those 17 is +-1.0%. Claude-Session: https://claude.ai/code/session_012B8z92S82sCfqCrVqrFgS2 * test(gap): cover a declared-array PropertyGet used directly as an element-read receiver Claude-Session: https://claude.ai/code/session_012B8z92S82sCfqCrVqrFgS2 --------- Co-authored-by: Ralph Küpper <ralph3@skelpo.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #7853.
What changed
js_value_length_property_f64sibling for guarded source-level.lengthreadsundefinedand non-numericlengthvalues instead of collapsing them to zerojs_value_length_f64behavior unchangedCoverage
The parity regression passes real arrays, numbers, strings, numeric/string-valued array-like objects, functions, typed arrays, null, and undefined through an explicitly declared
string[]receiver. Its output is byte-identical to Node.Scope and performance
The inline Array/String layout path is unchanged; only its existing guarded slow arm calls the new helper. A quiet-M1-mini A/B used two compilers against one fixed runtime, seven alternating runs per cell: the honest-array
length +loop was 0.21s vs 0.21s andlength *was 0.22s vs 0.22s.I also tested broadening the declared-number proof policy for every arithmetic use of
.length; that changed the hot path and regressed those microbenchmarks substantially, so it is deliberately not part of this property-read fix. #7854 can remove itsdeclared_only_array_localsexclusion once this fallback is available.Tests
test_gap_7853_declared_array_length_runtime_value.tsRUST_TEST_THREADS=1 cargo test -p perry-runtime --lib length_handle_band_tests(3 passed)cargo test -p perry-codegen --lib(877 passed)cargo fmt --all -- --checkbash scripts/check_file_size.shSummary by CodeRabbit
Bug Fixes
.lengthproperty reads to follow standard JavaScript behavior.lengthproperties now remainundefinedor retain their original value instead of becoming0..lengthonnullorundefinednow throws a catchableTypeError.Tests