Skip to content

fix(runtime): coerce class refs through ToPrimitive - #9125

Merged
proggeramlug merged 3 commits into
PerryTS:mainfrom
proggeramlug:fix/9101-class-ref-coercion
Aug 30, 2026
Merged

fix(runtime): coerce class refs through ToPrimitive#9125
proggeramlug merged 3 commits into
PerryTS:mainfrom
proggeramlug:fix/9101-class-ref-coercion

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Fixes #9101

Summary

  • preserve static Symbol.toPrimitive methods during class lowering and expose them as bound constructor methods
  • route INT32-tagged class refs through class-aware ToPrimitive for default, number, and string hints
  • cover looped string +=, numeric operators, direct String(), hook precedence, and named/class-expression forms

The separate class-source-text retention note in #9101 remains out of scope; this fixes the three coercion defects named in the issue title. No version bump is included.

Testing

Run on root@perrymaster.skelpo.net:

  • cargo fmt --all -- --check
  • cargo test -p perry --test issue_9101_class_ref_coercion -- --nocapture
  • cargo test -p perry --test issue_9087_class_ref_add --test class_expr_well_known_symbol_methods -- --nocapture
  • cargo test -p perry-runtime dynamic_arith --lib -- --nocapture

Summary by CodeRabbit

  • Bug Fixes
    • Fixed type coercion for class constructors used in arithmetic, string concatenation, unary operations, and String().
    • Class constructors now correctly honor Symbol.toPrimitive, valueOf, and toString hooks.
    • Improved handling of numeric and string conversion hints.
    • Static and instance Symbol.toPrimitive methods are now resolved and invoked correctly.
  • Tests
    • Added regression coverage for constructor coercion and custom conversion behavior.

@coderabbitai

coderabbitai Bot commented Aug 30, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 400a74d1-4778-4d51-92a4-383152a5637e

📥 Commits

Reviewing files that changed from the base of the PR and between 497f735 and 75779ed.

📒 Files selected for processing (5)
  • crates/perry-codegen/src/expr/object_literal.rs
  • crates/perry-hir/src/lower/expr_object.rs
  • crates/perry-runtime/src/value/mod.rs
  • crates/perry-runtime/src/value/to_string.rs
  • crates/perry-runtime/src/value/to_string_class_ref.rs

📝 Walkthrough

Walkthrough

Changes

The compiler now registers static and instance Symbol.toPrimitive methods. Runtime ClassRef coercion now applies Symbol.toPrimitive, valueOf, and toString across string, numeric, arithmetic, and unary conversion paths. Integration tests cover these hooks and conversion hints.

ClassRef method registration

Layer / File(s) Summary
Well-known symbol registration
crates/perry-hir/src/lower_decl/helpers.rs, crates/perry-hir/src/lower/expr_object.rs
Static and instance toPrimitive methods are registered under the synthetic well-known-symbol name. Related object-key formatting is unchanged.
ClassRef primitive resolution
crates/perry-runtime/src/value/..., crates/perry-runtime/src/symbol/...
ClassRef conversion accepts number, string, and default hints. It resolves and binds static Symbol.toPrimitive methods before ordinary conversion.
Coercion paths and regression coverage
crates/perry-runtime/src/builtins/numbers.rs, crates/perry-runtime/src/value/dynamic_arith.rs, crates/perry-runtime/src/value/to_string.rs, crates/perry/tests/issue_9101_class_ref_coercion.rs
Numeric, string, arithmetic, unary, and String() coercion paths use ClassRef primitive conversion. Tests verify hook precedence and hint routing.
Test and formatting support
crates/perry-codegen/src/expr/object_literal.rs
Codegen test organization and formatting changed without changing test behavior.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: ⚪ Minimal · up to 497f7

The PR enables class references to use defined primitive-conversion hooks during implicit coercion while preserving validation and fallback behavior. No actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: thehypnoo

Sequence Diagram(s)

sequenceDiagram
  participant JavaScript
  participant RuntimeCoercion
  participant ClassRefPrimitive
  participant SymbolToPrimitive
  JavaScript->>RuntimeCoercion: coerce constructor ClassRef
  RuntimeCoercion->>ClassRefPrimitive: apply conversion hint
  ClassRefPrimitive->>SymbolToPrimitive: resolve and call static method
  SymbolToPrimitive-->>ClassRefPrimitive: return primitive
  ClassRefPrimitive-->>RuntimeCoercion: provide converted value
  RuntimeCoercion-->>JavaScript: complete operation
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 7 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the primary change: routing class references through ToPrimitive during runtime coercion.
Description check ✅ Passed The description covers the purpose, main changes, linked issue, scope boundary, and test commands. It omits the template's separate Changes and Checklist sections, but the required information is most…
Linked Issues check ✅ Passed The changes address the three coding defects in issue #9101: looped string concatenation, numeric coercion for * and -, and static Symbol.toPrimitive precedence. The separate class source-text retenti…
Out of Scope Changes check ✅ Passed The code and regression tests are related to class-reference coercion and the linked issue objectives. No unrelated implementation or version metadata changes are identified.
Full details: Description check

Explanation

The description covers the purpose, main changes, linked issue, scope boundary, and test commands. It omits the template's separate Changes and Checklist sections, but the required information is mostly present.

Full details: Linked Issues check

Explanation

The changes address the three coding defects in issue #9101: looped string concatenation, numeric coercion for * and -, and static Symbol.toPrimitive precedence. The separate class source-text retention concern is explicitly excluded, as permitted by the issue scope.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Ralph Küpper added 3 commits August 30, 2026 09:28
PerryTS#9125 pushed to_string.rs to 2003 lines, over the 2000-line cap
(scripts/check_file_size.sh). Extracted the ClassRef ToPrimitive group
into to_string_class_ref.rs and re-exported class_ref_to_primitive from
to_string so existing crate::value::to_string:: call paths still resolve.
@proggeramlug
proggeramlug force-pushed the fix/9101-class-ref-coercion branch from 497f735 to 75779ed Compare August 30, 2026 07:37
@proggeramlug

Copy link
Copy Markdown
Contributor Author

Merged. Closes #9101, which I filed after auditing #9097 — and it fixes the substantial half of it.

Measured on my #9097 probe (27 class-coercion shapes) against node v26.5.1:

rows wrong
main before #9097 22/27
after #9097 13/27
after this PR 8/27

So this closes five more rows: static valueOf/toString through * and -, static Symbol.toPrimitive (which correctly takes precedence over both), and the looped string += case that #9097's numeric fix had left behind. Routing ClassRefs through a real ToPrimitive with hint-mandated ordering, rather than extending the +-specific path again, is why it picks up all three at once.

The remaining 8 are the class source text gap — "" + Plain gives function Plain() { [native code] } where node gives class Plain { } — which is a lowering-level problem (the source span isn't retained), not a coercion one. That's item 4 in #9101 and wants its own change.

I added one commit. The PR pushed to_string.rs to 2003 lines, three over the 2000-line cap, so check_file_size.sh was red. I extracted the ClassRef ToPrimitive group into a sibling to_string_class_ref.rs and re-exported class_ref_to_primitive from to_string, so the existing crate::value::to_string::class_ref_to_primitive call sites in builtins/numbers.rs and symbol/iterator.rs still resolve unchanged.

Worth flagging what that split then tripped, since it will catch the next person who splits a runtime file: a new module starts with a raw-handle ceiling of zero, so relocating one pre-existing get_raw_const_ptr into it made raw_handle_debt.py red even though no new code was written. I converted it to a genuine with_const_ptr scope (the tag-combine inside allocates nothing) rather than adding a ceiling.

This landed as part of a six-PR merge train, and had to go after #9122 — the two conflict in expr_object.rs, so a bare cherry-pick onto main fails. Worth knowing if either gets reverted.

Validation on the final rebased tree: runtime 2822 passed (exit 0, 0 abort markers), fmt clean, check_file_size.sh PASS, run_lint_gates.sh all 60 gates passed; 2 CI-only skipped.

@proggeramlug
proggeramlug merged commit 4994c77 into PerryTS:main Aug 30, 2026
16 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Class refs bypass valueOf/toString in looped string +=, in * and -, and ignore Symbol.toPrimitive

1 participant