Advance representation-aware type lowering runtime gates#5291
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 (57)
💤 Files with no reviewable changes (6)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (45)
📝 WalkthroughWalkthroughAdds JsValueBits support, refactors native fact collection into TypeFacts, wires native_facts through codegen, updates raw-f64 and numeric fast paths, changes typed-feedback array-set ABI to f64, and expands compiler-output verification plus documentation. ChangesTyped Native Specialization Pipeline
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related issues
Possibly related PRs
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 unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
crates/perry-codegen/src/expr/index.rs (1)
222-259:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUpdate the native-record consumer for the inlined raw-f64 store.
Line 259 still records
js_array_numeric_set_f64_unboxed, but the changed fast paths now canonicalize and store inline. The artifact should not claim the old runtime helper was consumed.Proposed metadata fix
- "js_array_numeric_set_f64_unboxed", + "inline_raw_f64_store",Also applies to: 307-309
🤖 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/expr/index.rs` around lines 222 - 259, The metadata recording in the ctx.record_lowered_value_with_access_mode_and_facts call still references the old runtime helper name "js_array_numeric_set_f64_unboxed", but the code has been changed to inline the canonicalization and store operations directly via canonicalize_raw_f64_numeric_store_value and blk.store instead of calling that helper. Update the metadata string passed to record_lowered_value_with_access_mode_and_facts to accurately reflect that an inline fast path is being used rather than the old helper, and apply the same fix to the similar code locations mentioned at lines 307-309.crates/perry-codegen/src/expr/index_set.rs (2)
480-489:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRecord the canonical raw-f64 value that was actually stored.
The fast path stores
numeric_value, but theNativeRep::F64record still points atval_double. For boxed int32 inputs or canonicalized NaNs, that records a different representation than the slot contains.Proposed fix
- let numeric_value = - canonicalize_raw_f64_numeric_store_value(blk, &val_double); + let numeric_value = + canonicalize_raw_f64_numeric_store_value(blk, &val_double); blk.store(DOUBLE, &numeric_value, &element_ptr); blk.br(&merge_label); } let stored = LoweredValue { semantic: SemanticKind::JsNumber, rep: NativeRep::F64, llvm_ty: DOUBLE, - value: val_double.clone(), + value: numeric_value.clone(), };🤖 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/expr/index_set.rs` around lines 480 - 489, The LoweredValue struct is being created with value pointing to val_double, but the actual stored value in the element_ptr is numeric_value (the canonicalized version from canonicalize_raw_f64_numeric_store_value). Replace the value field in the LoweredValue initialization to use numeric_value instead of val_double so that the recorded representation matches what was actually stored in memory. This ensures that for boxed int32 inputs or canonicalized NaNs, the LoweredValue accurately reflects the canonical raw-f64 value that was written to the slot.
370-418:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDerive the fallback
idx_doublefrom the active counter slot.Line 418 now sends the
f64index to the fallback, but line 370 lowers the local independently from thei32_counter_slotsvalue used by the guard/store. If the native counter slot is the current loop value, a guard failure can write the fallback at a stale boxed-local index.Proposed fix
- let arr_box = lower_expr(ctx, object)?; - let idx_double = lower_expr(ctx, index)?; - // Grab i32 slot name before mutably borrowing ctx for block(). - let i32_slot_opt = ctx.i32_counter_slots.get(idx_id).cloned(); - let idx_i32 = if let Some(ref i32_slot) = i32_slot_opt { - ctx.block().load(I32, i32_slot) - } else { - ctx.block().fptosi(DOUBLE, &idx_double, I32) - }; + let arr_box = lower_expr(ctx, object)?; + // Grab i32 slot name before mutably borrowing ctx for block(). + let i32_slot_opt = ctx.i32_counter_slots.get(idx_id).cloned(); + let (idx_i32, idx_double) = if let Some(ref i32_slot) = i32_slot_opt { + let idx_i32 = ctx.block().load(I32, i32_slot); + let idx_double = ctx.block().sitofp(I32, &idx_i32, DOUBLE); + (idx_i32, idx_double) + } else { + let idx_double = lower_expr(ctx, index)?; + let idx_i32 = ctx.block().fptosi(DOUBLE, &idx_double, I32); + (idx_i32, idx_double) + };🤖 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/expr/index_set.rs` around lines 370 - 418, The issue is that when a counter slot is available in i32_counter_slots, the idx_i32 value is derived from that slot via ctx.block().load(), but the fallback operation at line 418 uses the independently lowered idx_double value, creating a mismatch. To fix this, after determining idx_i32 (whether from the counter slot or via fptosi conversion in the else branch), convert idx_i32 back to a double using ctx.block().sitofp() and store that result. Then use this derived double value when passing to the js_typed_feedback_array_index_set_fallback_boxed call instead of the original idx_double, ensuring both the guard/store and fallback operations use the same index value.
🧹 Nitpick comments (7)
crates/perry-runtime/src/typed_feedback.rs (1)
1089-1123: 💤 Low valueConsider extracting header lookup to avoid duplicate work.
plain_array_index_set_guardcallsplain_array_index_guardwhich internally callsgc_header_for_user_addr, then calls it again at line 1098. While the cost is low (pointer arithmetic and validation), this could be avoided by refactoring the shared logic to return the header alongside the result.🤖 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/typed_feedback.rs` around lines 1089 - 1123, The function plain_array_index_set_guard duplicates work by calling plain_array_index_guard (which internally calls gc_header_for_user_addr) and then calling gc_header_for_user_addr again. Refactor by extracting the header lookup and initial validation logic into a shared helper function that returns both the guard validation result and the header object, then update plain_array_index_guard and plain_array_index_set_guard to use this helper to eliminate the redundant gc_header_for_user_addr call.TYPE_LOWERING_GUIDANCE.md (3)
842-842: 💤 Low valueOptional: Add hyphen to compound adjective.
Line 842: "The best single sentence guidance" should be "The best single-sentence guidance" (hyphenating compound adjectives modifying a noun).
This is a minor style point.
🤖 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 `@TYPE_LOWERING_GUIDANCE.md` at line 842, In TYPE_LOWERING_GUIDANCE.md, the phrase "single sentence guidance" uses a compound adjective that should be hyphenated when modifying a noun. Change "single sentence guidance" to "single-sentence guidance" to follow standard English grammar conventions for compound adjectives.
539-539: 💤 Low valueOptional: Reduce overuse of "exactly."
Line 539 uses "exactly the kinds of facts Perry can provide." The word "exactly" is used elsewhere in the document as well. For a formal technical document, consider reducing or replacing:
these are the precise/specific kinds of facts Perry can provide from its type/layout systemThis is a minor style suggestion; the current phrasing is acceptable.
🤖 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 `@TYPE_LOWERING_GUIDANCE.md` at line 539, On line 539 in TYPE_LOWERING_GUIDANCE.md, the phrase "exactly the kinds of facts Perry can provide from its type/layout system" uses "exactly" which appears multiple times throughout the document. Replace "exactly" in this instance with alternatives such as "precise" or "specific" to reduce redundancy and maintain a more formal technical writing tone. Consider reviewing other occurrences of "exactly" in the document and replacing them with more varied language.
212-212: 💤 Low valueOptional: Reduce repetition of "No" sentence starts.
Three successive sentences begin with "No": "No per-iteration NaN-box decode. No per-iteration length load. No bounds check..."
This is stylistically sub-optimal. Suggested revision:
This eliminates per-iteration NaN-box decoding, length loads, and bounds checks (if the loop proof establishes `i < arr.length`), with no runtime helper call.This is a minor style improvement; the current phrasing is clear.
🤖 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 `@TYPE_LOWERING_GUIDANCE.md` at line 212, Reduce the stylistic repetition in the guidance text by combining the three successive sentences that each begin with "No" into a single, more fluid sentence. Find the section describing optimization details about NaN-box decode, length load, bounds check, and runtime helper call, and restructure it to eliminate the repetitive "No" sentence starts while preserving all the technical information about performance optimizations and loop proofs.crates/perry-codegen/src/type_analysis.rs (1)
898-914: Consider refining PropertySet logic inexpr_may_materialize_pod_localto match actual materialization conditions.PropertySet on a POD field does not unconditionally cause materialization—it only materializes when the assigned value is incompatible with the native field type (as shown in the lowering code at pod_record.rs:140-185). The current code returns
truefor any PropertySet on a known POD field, which is overly conservative and may gate numeric fast paths unnecessarily.Additionally, line 31 (recursing on
object) is redundant whenpod_field_setistrue, sinceobjectis guaranteed to beLocalGet(target_id)which already returnstrueon line 3.Refactoring to check the value instead of returning unconditionally would align with the PropertyGet pattern (which returns
falsefor POD field reads) and enable more numeric optimizations:Expr::PropertySet { object, property, value } if matches!(object.as_ref(), Expr::LocalGet(id) if *id == target_id) && ctx.pod_records.get(&target_id).is_some_and(|local| { local.layout.fields.iter().any(|field| field.name == *property) }) => { // Known POD field set on target: only check if value materializes target expr_may_materialize_pod_local(ctx, value, target_id) }🤖 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/type_analysis.rs` around lines 898 - 914, The PropertySet branch in the expr_may_materialize_pod_local function is overly conservative by returning true for any PropertySet on a known POD field, when in reality only incompatible value types cause materialization. Refactor the PropertySet handling to remove the unconditional pod_field_set return and instead only recurse on the value parameter when assigning to a known POD field, eliminating the redundant check on object (since it is guaranteed to be LocalGet(target_id) when pod_field_set is true) and aligning the behavior with the PropertyGet pattern to enable more numeric optimizations.crates/perry-codegen/docs/native-representation.md (1)
64-87: 💤 Low valueConsider briefly noting the schema version gap.
The schema version jumps from 5 to 12 in the documentation (while the code bumps from 11 to 12). This is fine since documentation doesn't update for every schema version, but a brief note acknowledging the gap or referencing what changed in versions 6-11 would help readers understand the evolution.
This is optional and doesn't block the PR.
🤖 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/docs/native-representation.md` around lines 64 - 87, The documentation section describing schema version 12 should acknowledge the gap between schema version 5 (presumably documented earlier) and the current version 12. Add a brief note before or at the beginning of the schema version 12 section that explains the version jump, either by stating that intermediate schema versions 6-11 are not documented here, or by providing a concise summary of what evolved across those versions. This will help readers understand the schema evolution timeline and why earlier versions are skipped in the documentation.crates/perry-codegen/src/expr/property_set.rs (1)
265-326: 💤 Low valueConsider deferring
raw_f64_fieldcomputation until after slot lookup.The
raw_f64_fieldandnumeric_storechecks at lines 270-277 are computed before verifying thatmaybe_slotexists (line 279). If the slot doesn't exist, this work is wasted. While not incorrect, moving these computations inside theif let Some(slot)block would avoid unnecessary analysis when the scalar-replaced slot isn't present.🤖 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/expr/property_set.rs` around lines 265 - 326, The computation of raw_f64_field (via the call to crate::type_analysis::scalar_replaced_field_is_raw_f64) and numeric_store is performed before verifying that maybe_slot contains a value with the if let Some(slot) check. Move both the raw_f64_field calculation and the numeric_store boolean assignment inside the if let Some(slot) block to avoid performing unnecessary analysis when no scalar-replaced slot exists. Keep val_double outside since it's still needed for the early return.
🤖 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-codegen/src/expr/property_get.rs`:
- Around line 766-778: The scalar_replaced_field_is_raw_f64 function call uses
object.as_ref() (inspecting Expr::This) while
scalar_replaced_field_raw_f64_store_state is keyed by target_id, creating an
inconsistency in raw-f64 classification. Replace the object.as_ref() argument
passed to scalar_replaced_field_is_raw_f64 with a synthetic LocalGet(target_id)
expression to ensure both function calls evaluate the same identifier and
provide consistent raw-f64 classification for scalar-constructor this.x paths.
In `@crates/perry-codegen/src/stmt/loops.rs`:
- Around line 349-357: The issue is that the i32_counter_slot inserted at line
356 (via ctx.i32_counter_slots.insert) can overwrite a pre-existing entry, and
the cleanup code at lines 722-723 unconditionally removes this slot, deleting
counter specializations that should persist after the loop. Track which
i32_counter_slots entries are freshly created by this loop iteration (similar to
how the local-bound path handles freshness tracking) so that the cleanup logic
only removes the slots created by this specific loop, preserving any
pre-existing counter specializations for code after the loop.
In `@TYPE_LOWERING.md`:
- Around line 34-38: The code block containing the bit-pattern diagram (starting
with "Bit 63: Sign") uses bare triple-backticks without a language specifier,
which prevents syntax highlighting. Locate this code block in the
TYPE_LOWERING.md file and add "text" as the language identifier immediately
after the opening triple-backticks to enable proper rendering and syntax
highlighting.
- Around line 40-52: Fix the NaN-box tags table formatting in the markdown table
spanning the TAG_CONSTANT, Value, and Meaning columns. Add trailing pipes to all
table rows to properly close the table structure, and move the reference
citation [4](`#0-3`) that appears in the JS_HANDLE_TAG row outside of the table
since it creates a column-count mismatch. Restructure the table to have
consistent three-column formatting with proper delimiters, then place any
reference citations below the table as separate lines.
- Around line 1-40: The markdown link fragments in TYPE_LOWERING.md (such as
`[1](`#0-0`)` and `[2](`#0-1`)`) use numeric patterns that don't correspond to
actual Markdown-generated heading IDs. Replace all 35+ fragment references
throughout the document (appearing on lines 22, 26, 52, 63, 75, 86, 101, etc.)
with either explicit HTML anchor tags placed directly above the citation targets
or rewrite the references to use proper heading-derived anchors based on the
actual heading text. For example, change references like `[1](`#0-0`)` to either
add an explicit anchor like `<a id="0-0"></a>` before the target content, or
change the reference to point to an actual heading like
`#hir-type-representation` based on the "HIR Type Representation" heading in the
document.
- Around line 13-22: The table in the "TypeScript Type | HIR Type | Runtime
Representation" section has a column-count mismatch where the i32 (inferred) row
contains four cells including the citation references [1](`#0-0`) [2](`#0-1`), but
the header row only defines three columns. Fix this by either adding a fourth
header column to accommodate the references, or by removing the citation
references from the table cell and moving them outside the table as standalone
footnotes. Additionally, add trailing pipes (|) to the end of each row in the
table to conform to MD055 Markdown formatting standards and ensure consistent
pipe-delimited structure across all rows.
- Around line 1142-1204: Add language specifiers to code blocks in
TYPE_LOWERING.md that currently use bare triple-backticks without a language
identifier. At lines 1142, 1173, and 1204, modify any code blocks marked as "Not
supported" or containing TypeScript/JavaScript code to include appropriate
language specifiers such as `text` for plain text examples or
`typescript`/`javascript` for code snippets, ensuring all documentation code
examples follow consistent markdown formatting with explicit language
declarations.
---
Outside diff comments:
In `@crates/perry-codegen/src/expr/index_set.rs`:
- Around line 480-489: The LoweredValue struct is being created with value
pointing to val_double, but the actual stored value in the element_ptr is
numeric_value (the canonicalized version from
canonicalize_raw_f64_numeric_store_value). Replace the value field in the
LoweredValue initialization to use numeric_value instead of val_double so that
the recorded representation matches what was actually stored in memory. This
ensures that for boxed int32 inputs or canonicalized NaNs, the LoweredValue
accurately reflects the canonical raw-f64 value that was written to the slot.
- Around line 370-418: The issue is that when a counter slot is available in
i32_counter_slots, the idx_i32 value is derived from that slot via
ctx.block().load(), but the fallback operation at line 418 uses the
independently lowered idx_double value, creating a mismatch. To fix this, after
determining idx_i32 (whether from the counter slot or via fptosi conversion in
the else branch), convert idx_i32 back to a double using ctx.block().sitofp()
and store that result. Then use this derived double value when passing to the
js_typed_feedback_array_index_set_fallback_boxed call instead of the original
idx_double, ensuring both the guard/store and fallback operations use the same
index value.
In `@crates/perry-codegen/src/expr/index.rs`:
- Around line 222-259: The metadata recording in the
ctx.record_lowered_value_with_access_mode_and_facts call still references the
old runtime helper name "js_array_numeric_set_f64_unboxed", but the code has
been changed to inline the canonicalization and store operations directly via
canonicalize_raw_f64_numeric_store_value and blk.store instead of calling that
helper. Update the metadata string passed to
record_lowered_value_with_access_mode_and_facts to accurately reflect that an
inline fast path is being used rather than the old helper, and apply the same
fix to the similar code locations mentioned at lines 307-309.
---
Nitpick comments:
In `@crates/perry-codegen/docs/native-representation.md`:
- Around line 64-87: The documentation section describing schema version 12
should acknowledge the gap between schema version 5 (presumably documented
earlier) and the current version 12. Add a brief note before or at the beginning
of the schema version 12 section that explains the version jump, either by
stating that intermediate schema versions 6-11 are not documented here, or by
providing a concise summary of what evolved across those versions. This will
help readers understand the schema evolution timeline and why earlier versions
are skipped in the documentation.
In `@crates/perry-codegen/src/expr/property_set.rs`:
- Around line 265-326: The computation of raw_f64_field (via the call to
crate::type_analysis::scalar_replaced_field_is_raw_f64) and numeric_store is
performed before verifying that maybe_slot contains a value with the if let
Some(slot) check. Move both the raw_f64_field calculation and the numeric_store
boolean assignment inside the if let Some(slot) block to avoid performing
unnecessary analysis when no scalar-replaced slot exists. Keep val_double
outside since it's still needed for the early return.
In `@crates/perry-codegen/src/type_analysis.rs`:
- Around line 898-914: The PropertySet branch in the
expr_may_materialize_pod_local function is overly conservative by returning true
for any PropertySet on a known POD field, when in reality only incompatible
value types cause materialization. Refactor the PropertySet handling to remove
the unconditional pod_field_set return and instead only recurse on the value
parameter when assigning to a known POD field, eliminating the redundant check
on object (since it is guaranteed to be LocalGet(target_id) when pod_field_set
is true) and aligning the behavior with the PropertyGet pattern to enable more
numeric optimizations.
In `@crates/perry-runtime/src/typed_feedback.rs`:
- Around line 1089-1123: The function plain_array_index_set_guard duplicates
work by calling plain_array_index_guard (which internally calls
gc_header_for_user_addr) and then calling gc_header_for_user_addr again.
Refactor by extracting the header lookup and initial validation logic into a
shared helper function that returns both the guard validation result and the
header object, then update plain_array_index_guard and
plain_array_index_set_guard to use this helper to eliminate the redundant
gc_header_for_user_addr call.
In `@TYPE_LOWERING_GUIDANCE.md`:
- Line 842: In TYPE_LOWERING_GUIDANCE.md, the phrase "single sentence guidance"
uses a compound adjective that should be hyphenated when modifying a noun.
Change "single sentence guidance" to "single-sentence guidance" to follow
standard English grammar conventions for compound adjectives.
- Line 539: On line 539 in TYPE_LOWERING_GUIDANCE.md, the phrase "exactly the
kinds of facts Perry can provide from its type/layout system" uses "exactly"
which appears multiple times throughout the document. Replace "exactly" in this
instance with alternatives such as "precise" or "specific" to reduce redundancy
and maintain a more formal technical writing tone. Consider reviewing other
occurrences of "exactly" in the document and replacing them with more varied
language.
- Line 212: Reduce the stylistic repetition in the guidance text by combining
the three successive sentences that each begin with "No" into a single, more
fluid sentence. Find the section describing optimization details about NaN-box
decode, length load, bounds check, and runtime helper call, and restructure it
to eliminate the repetitive "No" sentence starts while preserving all the
technical information about performance optimizations and loop proofs.
🪄 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: d9db0e08-e3c2-4adc-ae4d-21dac48d629e
📒 Files selected for processing (56)
TYPE_LOWERING.mdTYPE_LOWERING_GUIDANCE.mdbenchmarks/compiler_output/fixtures/raw_numeric_object_fields.tsbenchmarks/compiler_output/workloads.tomlcrates/perry-codegen/docs/native-representation.mdcrates/perry-codegen/src/codegen/closure.rscrates/perry-codegen/src/codegen/entry.rscrates/perry-codegen/src/codegen/function.rscrates/perry-codegen/src/codegen/method.rscrates/perry-codegen/src/collectors/hir_facts.rscrates/perry-codegen/src/collectors/mod.rscrates/perry-codegen/src/expr/bigint_set.rscrates/perry-codegen/src/expr/binary.rscrates/perry-codegen/src/expr/buffer_access.rscrates/perry-codegen/src/expr/buffer_views.rscrates/perry-codegen/src/expr/call_spread.rscrates/perry-codegen/src/expr/calls.rscrates/perry-codegen/src/expr/compare.rscrates/perry-codegen/src/expr/i32_fast_path.rscrates/perry-codegen/src/expr/index.rscrates/perry-codegen/src/expr/index_get.rscrates/perry-codegen/src/expr/index_set.rscrates/perry-codegen/src/expr/js_runtime.rscrates/perry-codegen/src/expr/mod.rscrates/perry-codegen/src/expr/native_record.rscrates/perry-codegen/src/expr/new_dynamic.rscrates/perry-codegen/src/expr/pod_record.rscrates/perry-codegen/src/expr/property_get.rscrates/perry-codegen/src/expr/property_set.rscrates/perry-codegen/src/expr/proxy_reflect.rscrates/perry-codegen/src/expr/static_method.rscrates/perry-codegen/src/expr/unary.rscrates/perry-codegen/src/expr/write_barrier.rscrates/perry-codegen/src/lower_conditional.rscrates/perry-codegen/src/native_value/artifact.rscrates/perry-codegen/src/native_value/materialize.rscrates/perry-codegen/src/native_value/mod.rscrates/perry-codegen/src/native_value/rep.rscrates/perry-codegen/src/native_value/verify.rscrates/perry-codegen/src/runtime_decls/arrays.rscrates/perry-codegen/src/runtime_decls/objects.rscrates/perry-codegen/src/stmt/loops.rscrates/perry-codegen/src/type_analysis.rscrates/perry-codegen/tests/native_proof_buffer_views.rscrates/perry-codegen/tests/native_proof_regressions.rscrates/perry-codegen/tests/typed_feedback.rscrates/perry-codegen/tests/typed_shape_descriptors.rscrates/perry-runtime/src/array/header.rscrates/perry-runtime/src/typed_feedback.rscrates/perry-runtime/src/typed_feedback/tests.rscrates/perry-runtime/src/typed_feedback/trace.rsscripts/compiler_output_harness/capture.pyscripts/compiler_output_harness/spec.pyscripts/compiler_output_harness/verification.pytests/raw_numeric_object_fields.tstests/test_compiler_output_regression.py
| let declared_raw_f64 = | ||
| crate::type_analysis::scalar_replaced_field_is_raw_f64( | ||
| ctx, | ||
| object.as_ref(), | ||
| property, | ||
| ); | ||
| let raw_f64_field = | ||
| crate::type_analysis::scalar_replaced_field_raw_f64_store_state( | ||
| ctx, | ||
| Some(target_id), | ||
| property, | ||
| declared_raw_f64, | ||
| ); |
There was a problem hiding this comment.
Use target_id for scalar-this raw-f64 declaration checks.
This branch keys the store-state lookup by target_id, but the declaration check still inspects Expr::This. Use a synthetic LocalGet(target_id) so scalar-constructor this.x gets the same raw-f64 classification as the local scalar-replaced path.
Proposed fix
- let declared_raw_f64 =
- crate::type_analysis::scalar_replaced_field_is_raw_f64(
- ctx,
- object.as_ref(),
- property,
- );
+ let target_expr = Expr::LocalGet(target_id);
+ let declared_raw_f64 =
+ crate::type_analysis::scalar_replaced_field_is_raw_f64(
+ ctx,
+ &target_expr,
+ property,
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let declared_raw_f64 = | |
| crate::type_analysis::scalar_replaced_field_is_raw_f64( | |
| ctx, | |
| object.as_ref(), | |
| property, | |
| ); | |
| let raw_f64_field = | |
| crate::type_analysis::scalar_replaced_field_raw_f64_store_state( | |
| ctx, | |
| Some(target_id), | |
| property, | |
| declared_raw_f64, | |
| ); | |
| let target_expr = Expr::LocalGet(target_id); | |
| let declared_raw_f64 = | |
| crate::type_analysis::scalar_replaced_field_is_raw_f64( | |
| ctx, | |
| &target_expr, | |
| property, | |
| ); | |
| let raw_f64_field = | |
| crate::type_analysis::scalar_replaced_field_raw_f64_store_state( | |
| ctx, | |
| Some(target_id), | |
| property, | |
| declared_raw_f64, | |
| ); |
🤖 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/expr/property_get.rs` around lines 766 - 778, The
scalar_replaced_field_is_raw_f64 function call uses object.as_ref() (inspecting
Expr::This) while scalar_replaced_field_raw_f64_store_state is keyed by
target_id, creating an inconsistency in raw-f64 classification. Replace the
object.as_ref() argument passed to scalar_replaced_field_is_raw_f64 with a
synthetic LocalGet(target_id) expression to ensure both function calls evaluate
the same identifier and provide consistent raw-f64 classification for
scalar-constructor this.x paths.
| # Perry: Type Lowering & Native Runtime Support — Full Findings & Gaps | ||
|
|
||
| --- | ||
|
|
||
| ## 1. Type Lowering Pipeline | ||
|
|
||
| Perry's type system flows from TypeScript annotations through HIR to native code. Types are **erased** before final machine code, but they drive optimization decisions throughout the pipeline. | ||
|
|
||
| ### HIR Type Representation | ||
|
|
||
| The `LoweringContext` in `perry-hir` infers types during AST→HIR lowering via `infer_type_from_expr`: | ||
|
|
||
| | TypeScript Type | HIR Type | Runtime Representation | | ||
| |---|---|---| | ||
| | `number` | `Type::Number` | Raw `f64` (IEEE 754 double) | | ||
| | `string` | `Type::String` | Pointer to `StringHeader` (NaN-boxed `STRING_TAG 0x7FFF`) | | ||
| | `boolean` | `Type::Boolean` | `TAG_TRUE/TAG_FALSE` singletons | | ||
| | `bigint` | `Type::BigInt` | Pointer to `BigIntHeader` (`BIGINT_TAG 0x7FFA`) | | ||
| | `class T` | `Type::Named(name)` | Pointer to `ObjectHeader` with `class_id` (`POINTER_TAG 0x7FFD`) | | ||
| | `any` / `unknown` | `Type::Any` | Dynamic NaN-boxed `f64` | | ||
| | `T[]` | `Type::Array(elem)` | Pointer to `ArrayHeader` | | ||
| | `i32` (inferred) | `Type::Int32` | Parallel `i32` alloca slot | [1](#0-0) [2](#0-1) | ||
|
|
||
| ### Generics: Monomorphization | ||
|
|
||
| Perry implements generics via monomorphization — each unique type instantiation produces a specialized function/class with mangled names (e.g., `identity$number`). The `MonomorphizationContext` uses work queues to recursively specialize dependencies. [3](#0-2) | ||
|
|
||
| --- | ||
|
|
||
| ## 2. NaN-Boxing: The Universal Value Representation | ||
|
|
||
| All JS values are represented as 64-bit `f64` (`JSValue`). The top 16 bits encode the type tag; the bottom 48 bits carry the payload (pointer, integer, or SSO data). | ||
|
|
||
| ``` | ||
| Bit 63: Sign (always 0 for tagged values) | ||
| Bits 62-48: Type tag | ||
| Bits 47-0: Payload (pointer / integer / SSO bytes) | ||
| ``` | ||
|
|
||
| | Tag Constant | Value | Meaning | |
There was a problem hiding this comment.
Fix markdown link fragments to match actual heading IDs or use explicit anchors.
The citations use fragment references like [1](#0-0) and [2](#0-1), but Markdown auto-generates heading IDs from text (not numeric patterns). Either:
- Define explicit anchors before each citation target:
<a id="0-0"></a>
**File:** crates/perry-hir/src/lower_types.rs (L13-59)- Or rewrite references to use heading-text-derived anchors:
See [Type Inference section](`#hir-type-representation`) for details.This affects all 35+ link fragments throughout the document (lines 22, 26, 52, 63, 75, 86, 101, etc.).
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 22-22: Link fragments should be valid
(MD051, link-fragments)
[warning] 22-22: Link fragments should be valid
(MD051, link-fragments)
[warning] 22-22: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
[warning] 22-22: Table column count
Expected: 3; Actual: 4; Too many cells, extra data will be missing
(MD056, table-column-count)
[warning] 26-26: Link fragments should be valid
(MD051, link-fragments)
[warning] 34-34: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@TYPE_LOWERING.md` around lines 1 - 40, The markdown link fragments in
TYPE_LOWERING.md (such as `[1](`#0-0`)` and `[2](`#0-1`)`) use numeric patterns that
don't correspond to actual Markdown-generated heading IDs. Replace all 35+
fragment references throughout the document (appearing on lines 22, 26, 52, 63,
75, 86, 101, etc.) with either explicit HTML anchor tags placed directly above
the citation targets or rewrite the references to use proper heading-derived
anchors based on the actual heading text. For example, change references like
`[1](`#0-0`)` to either add an explicit anchor like `<a id="0-0"></a>` before the
target content, or change the reference to point to an actual heading like
`#hir-type-representation` based on the "HIR Type Representation" heading in the
document.
| | TypeScript Type | HIR Type | Runtime Representation | | ||
| |---|---|---| | ||
| | `number` | `Type::Number` | Raw `f64` (IEEE 754 double) | | ||
| | `string` | `Type::String` | Pointer to `StringHeader` (NaN-boxed `STRING_TAG 0x7FFF`) | | ||
| | `boolean` | `Type::Boolean` | `TAG_TRUE/TAG_FALSE` singletons | | ||
| | `bigint` | `Type::BigInt` | Pointer to `BigIntHeader` (`BIGINT_TAG 0x7FFA`) | | ||
| | `class T` | `Type::Named(name)` | Pointer to `ObjectHeader` with `class_id` (`POINTER_TAG 0x7FFD`) | | ||
| | `any` / `unknown` | `Type::Any` | Dynamic NaN-boxed `f64` | | ||
| | `T[]` | `Type::Array(elem)` | Pointer to `ArrayHeader` | | ||
| | `i32` (inferred) | `Type::Int32` | Parallel `i32` alloca slot | [1](#0-0) [2](#0-1) |
There was a problem hiding this comment.
Fix table pipe formatting and column-count mismatch.
The "TypeScript → HIR → Runtime Representation" table has a fourth cell in the last row that doesn't align with the header:
| `i32` (inferred) | `Type::Int32` | Parallel `i32` alloca slot | [1](`#0-0`) [2](`#0-1`)Should be restructured to either:
- Add a fourth header column, or
- Move citation references outside the table as footnotes:
| `i32` (inferred) | `Type::Int32` | Parallel `i32` alloca slot |
| ... | ... | ... | reference list belowAlso add trailing pipes to match MD055 style.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 22-22: Link fragments should be valid
(MD051, link-fragments)
[warning] 22-22: Link fragments should be valid
(MD051, link-fragments)
[warning] 22-22: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
[warning] 22-22: Table column count
Expected: 3; Actual: 4; Too many cells, extra data will be missing
(MD056, table-column-count)
🤖 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 `@TYPE_LOWERING.md` around lines 13 - 22, The table in the "TypeScript Type |
HIR Type | Runtime Representation" section has a column-count mismatch where the
i32 (inferred) row contains four cells including the citation references
[1](`#0-0`) [2](`#0-1`), but the header row only defines three columns. Fix this by
either adding a fourth header column to accommodate the references, or by
removing the citation references from the table cell and moving them outside the
table as standalone footnotes. Additionally, add trailing pipes (|) to the end
of each row in the table to conform to MD055 Markdown formatting standards and
ensure consistent pipe-delimited structure across all rows.
| ``` | ||
| Bit 63: Sign (always 0 for tagged values) | ||
| Bits 62-48: Type tag | ||
| Bits 47-0: Payload (pointer / integer / SSO bytes) | ||
| ``` |
There was a problem hiding this comment.
Specify language for code block.
Line 34 uses bare triple-backticks for the NaN-box bit-pattern diagram. Specify language for syntax highlighting:
```text
Bit 63: Sign (always 0 for tagged values)
...
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>
[warning] 34-34: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @TYPE_LOWERING.md around lines 34 - 38, The code block containing the
bit-pattern diagram (starting with "Bit 63: Sign") uses bare triple-backticks
without a language specifier, which prevents syntax highlighting. Locate this
code block in the TYPE_LOWERING.md file and add "text" as the language
identifier immediately after the opening triple-backticks to enable proper
rendering and syntax highlighting.
</details>
<!-- fingerprinting:phantom:triton:mongoose -->
<!-- cr-comment:v1:20dac6475433b33d9476f50a -->
<!-- This is an auto-generated comment by CodeRabbit -->
| | Tag Constant | Value | Meaning | | ||
| |---|---|---| | ||
| | `TAG_UNDEFINED` | `0x7FFC_0000_0000_0001` | `undefined` singleton | | ||
| | `TAG_NULL` | `0x7FFC_0000_0000_0002` | `null` singleton | | ||
| | `TAG_FALSE` | `0x7FFC_0000_0000_0003` | `false` | | ||
| | `TAG_TRUE` | `0x7FFC_0000_0000_0004` | `true` | | ||
| | `TAG_HOLE` | `0x7FFC_0000_0000_0010` | Sparse array sentinel | | ||
| | `POINTER_TAG` | `0x7FFD` | Object/Array/Symbol heap pointer | | ||
| | `INT32_TAG` | `0x7FFE` | 32-bit signed integer | | ||
| | `STRING_TAG` | `0x7FFF` | Heap `StringHeader` pointer | | ||
| | `SHORT_STRING_TAG` | `0x7FF9` | SSO: ≤5 bytes inline in payload | | ||
| | `BIGINT_TAG` | `0x7FFA` | Heap `BigIntHeader` pointer | | ||
| | `JS_HANDLE_TAG` | `0x7FFB` | Handle into V8/QuickJS heap | [4](#0-3) |
There was a problem hiding this comment.
Fix NaN-box tags table pipe and column-count formatting.
Same issues as the earlier table (lines 13-22): missing trailing pipes, column-count mismatch on reference cells. Restructure to separate citations from table data.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 52-52: Link fragments should be valid
(MD051, link-fragments)
[warning] 52-52: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
[warning] 52-52: Table column count
Expected: 3; Actual: 4; Too many cells, extra data will be missing
(MD056, table-column-count)
🤖 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 `@TYPE_LOWERING.md` around lines 40 - 52, Fix the NaN-box tags table formatting
in the markdown table spanning the TAG_CONSTANT, Value, and Meaning columns. Add
trailing pipes to all table rows to properly close the table structure, and move
the reference citation [4](`#0-3`) that appears in the JS_HANDLE_TAG row outside
of the table since it creates a column-count mismatch. Restructure the table to
have consistent three-column formatting with proper delimiters, then place any
reference citations below the table as separate lines.
| ``` | ||
|
|
||
| **File:** docs/src/language/limitations.md (L76-102) | ||
| ```markdown | ||
| ## Limited Prototype Manipulation | ||
|
|
||
| Perry compiles classes to fixed structures. Dynamic prototype modification is not supported: | ||
|
|
||
| <!-- intentionally-rejects: this snippet documents code Perry refuses to compile --> | ||
| ```text | ||
| // Not supported | ||
| MyClass.prototype.newMethod = function() {}; | ||
| Object.setPrototypeOf(obj, proto); | ||
| ``` | ||
|
|
||
| `Object.getPrototypeOf(...)` and `Reflect.getPrototypeOf(...)` are supported | ||
| for class/prototype inspection patterns, but `Object.setPrototypeOf(...)` / | ||
| `Reflect.setPrototypeOf(...)` do not mutate Perry's fixed class layout. | ||
|
|
||
| ## Weak References Are Not GC-Accurate | ||
|
|
||
| `WeakMap`, `WeakSet`, `WeakRef`, and `FinalizationRegistry` expose the expected | ||
| API shape, but their weak-reference semantics are pragmatic, not GC-accurate: | ||
| `WeakRef` keeps a strong reference internally, and `FinalizationRegistry` | ||
| records registrations but does not run cleanup callbacks after collection. | ||
|
|
||
| ## Limited Proxy Trapping | ||
|
|
||
| Proxy support is not a full engine-level trap layer for every possible dynamic | ||
| object access. Prefer plain objects and explicit APIs unless a package only | ||
| needs Perry's supported Proxy surface. | ||
| ``` | ||
|
|
||
| **File:** docs/src/language/limitations.md (L108-108) | ||
| ```markdown | ||
| Threads do not share mutable state — closures passed to thread primitives cannot capture mutable variables (enforced at compile time). Values are deep-copied across thread boundaries. There is no `SharedArrayBuffer` or `Atomics`. | ||
| ``` | ||
|
|
||
| **File:** docs/src/language/limitations.md (L109-116) | ||
| ```markdown | ||
|
|
||
| ## npm Package Compatibility | ||
|
|
||
| Not all npm packages work with Perry: | ||
|
|
||
| - **Natively supported**: ~50 popular packages (fastify, mysql2, redis, etc.) — these are compiled natively. See [Standard Library](../stdlib/overview.md). | ||
| - **`compilePackages`**: Pure TS/JS packages can be compiled natively via [configuration](../getting-started/project-config.md). | ||
| - **Not supported**: Packages requiring native addons (`.node` files), `eval()`, dynamic `require()`, or Node.js internals. | ||
| ``` | ||
|
|
||
| **File:** docs/src/packages/porting.md (L133-142) | ||
| ```markdown | ||
| ### Computed property keys in object literals | ||
|
|
||
| ```text | ||
| // Not supported | ||
| const obj = { [key]: value }; | ||
|
|
||
| // Rewrite | ||
| const obj: Record<string, V> = {}; | ||
| obj[key] = value; | ||
| ``` | ||
| ``` |
There was a problem hiding this comment.
Specify language for code blocks in documentation examples.
Lines 1142, 1173, and 1204 use bare triple-backticks for text examples. Add text or typescript specifier:
<!-- intentionally-rejects: ... -->
```text
// Supported
import { foo } from "./module";
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>
[warning] 1142-1142: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
---
[warning] 1173-1173: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
---
[warning] 1204-1204: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @TYPE_LOWERING.md around lines 1142 - 1204, Add language specifiers to code
blocks in TYPE_LOWERING.md that currently use bare triple-backticks without a
language identifier. At lines 1142, 1173, and 1204, modify any code blocks
marked as "Not supported" or containing TypeScript/JavaScript code to include
appropriate language specifiers such as text for plain text examples or
typescript/javascript for code snippets, ensuring all documentation code
examples follow consistent markdown formatting with explicit language
declarations.
</details>
<!-- fingerprinting:phantom:triton:mongoose -->
<!-- cr-comment:v1:94669aa3b9f6229cea503098 -->
<!-- This is an auto-generated comment by CodeRabbit -->
fe43889 to
e14208f
Compare
e14208f to
025b43e
Compare
…_raw_f64 + split oversized link/mod.rs Two independent pre-existing main breakages turning required CI red for every PR off main: 1. cargo-test: js_array_numeric_value_to_raw_f64 (#5291, representation-aware numeric array lowering) is #[no_mangle] but called only from generated code, so the linker dead-strips it from libperry_runtime.a -> cold PERRY_NO_AUTO_OPTIMIZE=1 compiles fail with 'Undefined symbols: _js_array_numeric_value_to_raw_f64' (5 functional_batch2_regressions tests). Fix: add a #[used] keepalive anchor (project_autoopt_ffi_symbol_link_break). 2. lint: link/mod.rs crossed the 2000-line file-size gate (2132) after #5400. Split the ~1770-line build_and_run_link orchestrator into a sibling link/build_and_run.rs (mod.rs -> 357, new file 1785; pure mechanical move).
…oversized link/mod.rs Three pre-existing CI fragilities on main turning required checks red for PRs: 0. cargo-test runs 'cargo test -p perry-runtime', which never builds the staticlib crate-type, so libperry_runtime.a/libperry_stdlib.a only exist from cache. A PR touching perry-runtime invalidates the cached staticlib and cargo never rebuilds it -> PERRY_NO_AUTO_OPTIMIZE=1 tests fail with 'Could not find libperry_runtime.a'. Fix: add 'cargo build -p perry-runtime -p perry-stdlib' to the cargo-test job before the integration-test loop. 1. js_array_numeric_value_to_raw_f64 (#5291) is #[no_mangle] but called only from generated code, so the linker dead-strips it from libperry_runtime.a -> undefined symbol at link. Fix: #[used] keepalive anchor. 2. link/mod.rs crossed the 2000-line file-size gate (2132) after #5400. Split the ~1770-line build_and_run_link orchestrator into link/build_and_run.rs.
…oversized link/mod.rs (#5406) Three pre-existing CI fragilities on main turning required checks red for PRs: 0. cargo-test runs 'cargo test -p perry-runtime', which never builds the staticlib crate-type, so libperry_runtime.a/libperry_stdlib.a only exist from cache. A PR touching perry-runtime invalidates the cached staticlib and cargo never rebuilds it -> PERRY_NO_AUTO_OPTIMIZE=1 tests fail with 'Could not find libperry_runtime.a'. Fix: add 'cargo build -p perry-runtime -p perry-stdlib' to the cargo-test job before the integration-test loop. 1. js_array_numeric_value_to_raw_f64 (#5291) is #[no_mangle] but called only from generated code, so the linker dead-strips it from libperry_runtime.a -> undefined symbol at link. Fix: #[used] keepalive anchor. 2. link/mod.rs crossed the 2000-line file-size gate (2132) after #5400. Split the ~1770-line build_and_run_link orchestrator into link/build_and_run.rs. Co-authored-by: Ralph Küpper <ralph2@skelpo.com>
Summary
This PR advances Perry's representation-aware type lowering work so
JSValueremains the fallback/ABI representation instead of the default internal representation in newly covered hot paths.The branch focuses on correctness-first lowering and verifier-backed gates:
JsValueBitsrepresentation records while preserving the external NaN-boxeddoubleABI boundaryTypeFactssurfaceNumber(...)coercionVerification
Passed locally:
cargo build -p perrycargo test -p perry-codegencargo test -p perry-runtime -- --test-threads=1python3 -m unittest tests.test_compiler_output_regression -vpython3 scripts/compiler_output_regression.py suite --suite native-abi-proof --out-dir target/type-lowering-gates/native-abi-fixes-suite --perry target/debug/perry --runs 1 --benchmark-mode smoke --perf-counters off --gate --print-summarypython3 scripts/compiler_output_regression.py suite --suite native-region-proof --out-dir target/type-lowering-gates/native-region-after-abi-fixes --perry target/debug/perry --runs 1 --benchmark-mode smoke --perf-counters off --gate --print-summarywidth_aware_buffer_kernelsandnative_abi_packet_controlcargo fmt --checkgit diff --checkNotes
This is the current scoped type-lowering runtime phase. It does not attempt the entire long-range architecture in one PR: broad typed SSA, full late boxing, typed function clones/trampolines, generalized packed array kinds, fixed unboxed class layouts everywhere, interned method dispatch, unified string refs, typed async state machines, Map/Set specialization, PGO clone selection, and broader LLVM metadata remain follow-up work.
Summary by CodeRabbit
f64canonicalization, and improved scalar-replacement handling).