fix(hir): run static field initializers + static blocks for in-function classes#5110
Merged
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 (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
ChangesNested class static field and block initialization
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/perry-hir/src/lower_decl/body_stmt.rs (1)
315-345: ⚡ Quick winConsider extracting shared static-init emission logic.
The static field and block emission sequence (lines 315-345) duplicates the top-level path in
crates/perry-hir/src/lower/stmt.rslines 1060-1135. Both iteratestatic_fields, substitute lexicalthis, emitClassStaticSymbolSet/StaticFieldSet, then call__perry_static_init_*methods.Extracting this into a shared helper (e.g.,
emit_class_static_initializers(class: &Class, stmts: &mut Vec<Stmt>)) would reduce the ~70-line duplication and ensure both paths stay synchronized if the emission logic changes.🤖 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-hir/src/lower_decl/body_stmt.rs` around lines 315 - 345, Extract the static initialization emission logic into a shared helper function to eliminate the ~70-line duplication between the two static-init emission paths. Create a helper function (e.g., emit_class_static_initializers) that accepts a Class reference and a mutable Vec<Stmt>, then performs the sequence of iterating static_fields, calling substitute_lexical_this_in_expr, emitting ClassStaticSymbolSet or StaticFieldSet expressions based on whether key_expr exists, and iterating static_methods to emit StaticMethodCall expressions for methods matching the __perry_static_init_ pattern. Replace the duplicated block in the current location with a call to this helper, and apply the same refactoring to the duplicate logic in the other file to ensure both code paths remain synchronized.
🤖 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.
Inline comments:
In `@crates/perry-runtime/src/value/dynamic_arith.rs`:
- Around line 105-123: The code casts ptr to ObjectHeader and immediately calls
try_read_as_search_params on it without validating that ptr is actually a valid
object pointer. Small pointers (value < 0x100000) are widget handles, not object
pointers, and dereferencing them causes memory safety issues. Before the if
block that calls crate::url::try_read_as_search_params(obj).is_some(), add a
guard to check that ptr is not a small pointer (ensure ptr >= 0x100000) to
prevent invalid memory reads when non-object pointer kinds reach this code path
during + coercion.
---
Nitpick comments:
In `@crates/perry-hir/src/lower_decl/body_stmt.rs`:
- Around line 315-345: Extract the static initialization emission logic into a
shared helper function to eliminate the ~70-line duplication between the two
static-init emission paths. Create a helper function (e.g.,
emit_class_static_initializers) that accepts a Class reference and a mutable
Vec<Stmt>, then performs the sequence of iterating static_fields, calling
substitute_lexical_this_in_expr, emitting ClassStaticSymbolSet or StaticFieldSet
expressions based on whether key_expr exists, and iterating static_methods to
emit StaticMethodCall expressions for methods matching the __perry_static_init_
pattern. Replace the duplicated block in the current location with a call to
this helper, and apply the same refactoring to the duplicate logic in the other
file to ensure both code paths remain synchronized.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a0a8174f-f460-4b8b-95e5-5e1cc4ad7be0
📒 Files selected for processing (5)
crates/perry-codegen/src/linker.rscrates/perry-hir/src/lower_decl/body_stmt.rscrates/perry-runtime/src/value/dynamic_arith.rscrates/perry-runtime/src/value/to_string.rstest-files/test_class_static_in_function.ts
Comment on lines
+105
to
+123
| // WHATWG `URL` / `URLSearchParams` have native `toString`s (`href` / the | ||
| // query string) that OrdinaryToPrimitive can't see — it would resolve the | ||
| // inherited `Object.prototype.toString` and yield "[object Object]". Like | ||
| // the Date special-case above, pre-empt with the real string so | ||
| // `"" + url` / `` `${url}` `` match explicit `url.toString()` (#URL coercion). | ||
| { | ||
| let boxed = | ||
| f64::from_bits(crate::value::POINTER_TAG | ((ptr as u64) & crate::value::POINTER_MASK)); | ||
| let href = crate::url::url_class::js_url_href_if_url(boxed); | ||
| if href.to_bits() != crate::value::TAG_UNDEFINED { | ||
| let s = js_jsvalue_to_string(href); | ||
| return crate::value::js_nanbox_string(s as i64); | ||
| } | ||
| let obj = ptr as *mut crate::object::ObjectHeader; | ||
| if crate::url::try_read_as_search_params(obj).is_some() { | ||
| let s = crate::url::search_params::js_url_search_params_to_string(obj); | ||
| return crate::value::js_nanbox_string(s as i64); | ||
| } | ||
| } |
There was a problem hiding this comment.
Guard URLSearchParams probing behind an object-pointer validity check
Line 118 casts ptr to ObjectHeader and immediately probes try_read_as_search_params, but that helper dereferences object fields. In this path, non-object pointer kinds can still reach this block, so this can become an invalid memory read/crash during + coercion.
💡 Suggested fix
{
let boxed =
f64::from_bits(crate::value::POINTER_TAG | ((ptr as u64) & crate::value::POINTER_MASK));
let href = crate::url::url_class::js_url_href_if_url(boxed);
if href.to_bits() != crate::value::TAG_UNDEFINED {
let s = js_jsvalue_to_string(href);
return crate::value::js_nanbox_string(s as i64);
}
- let obj = ptr as *mut crate::object::ObjectHeader;
- if crate::url::try_read_as_search_params(obj).is_some() {
- let s = crate::url::search_params::js_url_search_params_to_string(obj);
- return crate::value::js_nanbox_string(s as i64);
+ if crate::object::is_valid_obj_ptr(ptr as *const u8) {
+ let obj = ptr as *mut crate::object::ObjectHeader;
+ if crate::url::try_read_as_search_params(obj).is_some() {
+ let s = crate::url::search_params::js_url_search_params_to_string(obj);
+ return crate::value::js_nanbox_string(s as i64);
+ }
}
}As per coding guidelines, "Detect small pointers (value < 0x100000) as widget handles in the NaN-boxed value representation".
🤖 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-runtime/src/value/dynamic_arith.rs` around lines 105 - 123, The
code casts ptr to ObjectHeader and immediately calls try_read_as_search_params
on it without validating that ptr is actually a valid object pointer. Small
pointers (value < 0x100000) are widget handles, not object pointers, and
dereferencing them causes memory safety issues. Before the if block that calls
crate::url::try_read_as_search_params(obj).is_some(), add a guard to check that
ptr is not a small pointer (ensure ptr >= 0x100000) to prevent invalid memory
reads when non-object pointer kinds reach this code path during + coercion.
Source: Coding guidelines
proggeramlug
force-pushed
the
fix/class-static-init-in-function
branch
2 times, most recently
from
f1357f8 to
1fa77db
Compare
…on classes
A class declared inside a function body lost ALL static initialization:
`static x = …` fields and `static { … }` blocks silently stayed at their
zero default (`D.a,D.b,D.c` → `0,0,0` instead of `7,12,19`). Only
top-level classes initialized.
Root cause: the in-function class-decl lowering path
(`lower_decl/body_stmt.rs`) emitted the class plus its parent /
computed-member / capture registrations, but — unlike the module-level
path in `lower/stmt.rs` — never emitted the static-field-init
(`StaticFieldSet` / `ClassStaticSymbolSet`) or static-block-call
(`StaticMethodCall __perry_static_init_N`) statements into the function
body. So the synthetic static-init methods were never invoked and field
initializers never ran.
Fix: mirror the top-level emission in `body_stmt.rs` — after the class is
set up, push the static-field initializers (with lexical `this` in the
initializer bound to the class ref) then the static-block calls into the
function body, in source order per ClassDefinitionEvaluation.
Verified vs Node v26.3.0: in-function and arrow-IIFE classes now produce
`7,12,19` / `99` (was `0,0,0` / `0`); top-level classes unchanged. New
`test-files/test_class_static_in_function.ts` matches Node byte-for-byte;
perry-hir unit tests and existing class test-files pass.
proggeramlug
force-pushed
the
fix/class-static-init-in-function
branch
from
1fa77db to
c3bd051
Compare
proggeramlug
pushed a commit
that referenced
this pull request
Jun 14, 2026
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.
Problem
A class declared inside a function body lost all static initialization —
static x = …fields andstatic { … }blocks silently stayed at their zero default. Only top-level classes initialized.(Reproduces for both
functionbodies and arrow IIFEs.)Root cause
The in-function class-decl lowering path (
lower_decl/body_stmt.rs) emitted the class plus its parent / computed-member / capture registrations, but — unlike the module-level path inlower/stmt.rs— never emitted the static-field-init (StaticFieldSet/ClassStaticSymbolSet) or static-block-call (StaticMethodCall __perry_static_init_N) statements. So the synthetic static-init methods were never invoked and field initializers never ran.Fix
Mirror the top-level emission in
body_stmt.rs: after the class is set up, push the static-field initializers (with lexicalthisin the initializer bound to the class ref) then the static-block calls into the function body, in source order per ClassDefinitionEvaluation.Verification (vs Node v26.3.0)
7,12,19(was0,0,0); arrow-IIFE static block:99(was0); top-level unchanged (7,12,19).test-files/test_class_static_in_function.tsmatches Node byte-for-byte.cargo test -p perry-hirpasses; existing class test-files (test_class_static_iife,test_class_static_symbol,test_edge_class_advanced,test_gap_*_static_helpers) all compile + run clean.Found via a
node --experimental-strip-typesdifferential sweep. No version bump / changelog per maintainer instruction.Summary by CodeRabbit
staticblocks run correctly during declaration evaluation, including computed and symbol-keyed fields.