fix: harden dynamic imports and accelerate numeric/JSON fast paths#6810
Merged
proggeramlug merged 3 commits intoJul 23, 2026
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 (1)
📝 WalkthroughWalkthroughThe PR fixes single-candidate dynamic import matching, adds transactional numeric range-add loop fast paths, refines packed-loop guards, reduces JSON validation allocation, and strengthens runtime GC-header and numeric-array checks. ChangesDynamic import resolution
Numeric range-add optimization
Runtime validation and parsing
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ForLoopLowering
participant ArrayRangeHelper
participant ArrayStorage
participant GenericLoop
ForLoopLowering->>ArrayRangeHelper: call numeric range-add helper
ArrayRangeHelper->>ArrayStorage: validate range and numeric slots
ArrayRangeHelper->>ArrayStorage: apply delta transactionally
ArrayRangeHelper-->>ForLoopLowering: return end or -1
ForLoopLowering->>GenericLoop: lower original loop on failure
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/perry-codegen/src/stmt/loops.rs (1)
304-438: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStore-shape destructuring here duplicates
supported_packed_numeric_loop_store_kind(Lines 2729-2749).Lines 354-374 and Lines 2729-2749 both destructure the loop body into
(object/target, index/key, value)with the identicalIndexSet | PutValueSet { receiver == target }accept rule. These two matchers must stay in lockstep (one drives the optimization, the other the fallback shape); a shared helper such asmatch_indexed_store_shape(store) -> Option<(&Expr, &Expr, &Expr)>removes the drift risk.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-codegen/src/stmt/loops.rs` around lines 304 - 438, Extract the duplicated IndexSet/PutValueSet destructuring and receiver-target validation from match_numeric_range_add_loop and supported_packed_numeric_loop_store_kind into a shared match_indexed_store_shape helper returning object, index, and value expressions. Replace both inline matchers with this helper while preserving their existing acceptance rules and downstream behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@crates/perry-codegen/src/stmt/loops.rs`:
- Around line 304-438: Extract the duplicated IndexSet/PutValueSet destructuring
and receiver-target validation from match_numeric_range_add_loop and
supported_packed_numeric_loop_store_kind into a shared match_indexed_store_shape
helper returning object, index, and value expressions. Replace both inline
matchers with this helper while preserving their existing acceptance rules and
downstream behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 01a176e5-1644-4522-b49e-ec8737aaefd7
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
crates/perry-codegen/src/expr/dyn_extern_i18n.rscrates/perry-codegen/src/runtime_decls/arrays.rscrates/perry-codegen/src/stmt/loops.rscrates/perry-codegen/tests/native_proof_regressions/invalidation.rscrates/perry-runtime/Cargo.tomlcrates/perry-runtime/src/array/indexing.rscrates/perry-runtime/src/array/mod.rscrates/perry-runtime/src/array/tests.rscrates/perry-runtime/src/json/mod.rscrates/perry-runtime/src/json/parse_api.rscrates/perry-runtime/src/object/field_get_set/ic_miss.rscrates/perry-runtime/src/object/prototype_chain.rscrates/perry-runtime/src/typed_feedback.rscrates/perry/tests/issue_6660_closure_dynamic_import.rsscripts/addr_class_ratchet_baseline.txt
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
1 participant
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.
Summary
Hardens runtime validation around AOT-specialized paths and removes several high-impact performance cliffs found during an audit of dynamic imports, address classification, numeric arrays, and JSON parsing.
The changes preserve ordinary JS lowering as the fallback whenever a runtime specialization cannot prove its preconditions.
Changes
ERR_MODULE_NOT_FOUNDrather than silently loading the sole compiled candidate.arr[i] = arr[i] + constantloops. It validates the complete window before mutation and falls back without partial effects for holes, non-numbers, frozen arrays, descriptors, or unsupported receivers.serde::de::IgnoredAnyinstead of constructing and immediately dropping a second fullserde_json::Valuetree before Perry builds its runtime representation.Benchmark results
Measured on the same machine/build configuration; checksums and the large-pipeline output hash were unchanged.
number[]loopany[]loopThe matrix baseline was noisy due to concurrent local work; the after range is included rather than claiming precision beyond the measurements.
AOT boundary
This fixes the single-candidate silent-wrong-module case and honors the post-hook specifier. Perry remains closed-world for dynamic imports: a hook redirect to a module absent from the compiled candidate set rejects rather than loading arbitrary new source at runtime.
Related issue
n/a — standalone audit findings.
Test plan
cargo check -p perry-codegen -p perry-runtimecargo build --release -p perry -p perry-runtime-static -p perry-stdlib-staticcargo test -p perry-codegen --test native_proof_regressions— 245 passedcargo test -p perry --test issue_6660_closure_dynamic_import -- --test-threads=1— 6 passedcargo test -p perry-runtime typed_feedback -- --test-threads=1— 49 passedcargo test -p perry-runtime numeric_range_add -- --test-threads=1Focused JSON direct-parse and malformed/trailing-input tests
python3 scripts/addr_class_inventory.py— 800 files scanned, 265 allowlisted, 567 known sites held by the ratchetcargo fmt --allgit diff --checkAffected release targets build clean
Full non-UI workspace test suite was not run locally
Added regression tests in the affected compiler/runtime crates
Documentation not required: the new runtime ABI helpers are internal codegen support, not public APIs
Screenshots / output
Not applicable.
Checklist
feat:/fix:/docs:/chore:prefix convention used in the logSummary by CodeRabbit
end-from-length support.import()now validates the runtime module specifier correctly after loader/resolver hook redirects, including single-candidate resolution, preventing silent misloads.