Skip to content

Preserve WTF-8 keys for computed class members - #12471

Merged
proggeramlug merged 1 commit into
PerryTS:mainfrom
jdalton:cloud/issue-12202-wtf8-class-keys
Oct 13, 2026
Merged

proggeramlug merged 1 commit into
PerryTS:mainfrom
jdalton:cloud/issue-12202-wtf8-class-keys

Conversation

@jdalton

@jdalton jdalton commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Preserve exact WTF-8 bytes for computed class member names containing lone surrogates through declaration metadata, registration, lookup, reflection, descriptors, deletion and recreation, function names, method calls, and instance accessor writes, without changing the runtime ABI or symbol-key behavior. Fixes #12202.

At exact head 2272f7b5 (tree 20d752b0), 59 unique focused runtime tests passed across 67 executions. A fresh matching dev trio build passed, and the compiled fixture matched pinned Node v26.5.1 exactly. Genuine auto-opt validation passed after rebuilding runtime and stdlib with the async-bridge feature; fresh-archive Perry output matched Node exactly. Product and host warning comparisons are non-green but match the base exactly: 42 product and 35 host diagnostics.

The complete full-lint driver finished before its 30-minute cap with exit 1: 105 passed, 32 failed of 137 evaluated, and 2 CI-only gates were skipped. Thirty failures reproduce on the comparison parent; two are environment/path failures (the Windows CRT fetch was blocked by proxy HTTP 403, and the API docs wrapper used a hard-coded binary path). The fatal host Clippy error also reproduces on the parent, while the separate nonfatal warning comparison found three added and one removed warning fingerprints (net +2; 1102 vs 1100). Warn-level Clippy is informational under the repository workflow; the full delta is disclosed in the evidence. Direct API Markdown generation produced 91,751 bytes while tracked reference.md is empty on both baseline and candidate; the generated d.ts matched. The docs drift check after the wrapper failure does not prove regeneration.

Validation is Linux-only. Generic computed-method default-parameter .length behavior remains a separate known limitation for ASCII and WTF-8 keys. Tests lack a dedicated forced-evacuation stress witness for the byte-aware setter and prototype-materialization paths. Exact logs, hashes, and attribution are in the final validation record, the failure attribution, and the Clippy/API-doc reconciliation.

Summary by CodeRabbit

  • Bug Fixes
    • Computed class member names containing lone surrogates now retain their exact identity across registration, lookup, reflection, method calls, and accessor writes. They no longer collide with replacement-character names.
    • Native method calls and class static-property access preserve these names, including through inherited methods and accessors.
  • Tests
    • Added coverage for unusual computed keys, descriptors, function names, key ordering, deletion and re-addition, and method behavior.

@coderabbitai

coderabbitai Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 086b4e43-c242-49f1-89c7-3897e4a4ef9a
📥 Commits

Reviewing files that changed from the base of the PR and between 283ab13 and 26e4d02.

📒 Files selected for processing (35)
  • changelog.d/12471-wtf8-class-keys.md
  • crates/perry-runtime/src/closure/dynamic_props.rs
  • crates/perry-runtime/src/closure/mod.rs
  • crates/perry-runtime/src/closure/props.rs
  • crates/perry-runtime/src/object/accessor_pair.rs
  • crates/perry-runtime/src/object/class_image.rs
  • crates/perry-runtime/src/object/class_registry.rs
  • crates/perry-runtime/src/object/class_registry/decl_accessors.rs
  • crates/perry-runtime/src/object/class_registry/decl_prototype_birth.rs
  • crates/perry-runtime/src/object/class_registry/declarations.rs
  • crates/perry-runtime/src/object/class_registry/declarations_tests.rs
  • crates/perry-runtime/src/object/class_registry/parent_static.rs
  • crates/perry-runtime/src/object/class_registry/parent_static/private_and_dynamic.rs
  • crates/perry-runtime/src/object/class_registry/parent_static/static_accessor_call.rs
  • crates/perry-runtime/src/object/class_registry/prototype_methods.rs
  • crates/perry-runtime/src/object/class_registry/registration.rs
  • crates/perry-runtime/src/object/class_registry/state.rs
  • crates/perry-runtime/src/object/class_value.rs
  • crates/perry-runtime/src/object/class_value/tests.rs
  • crates/perry-runtime/src/object/descriptor_state.rs
  • crates/perry-runtime/src/object/descriptors.rs
  • crates/perry-runtime/src/object/field_get_set/class_object_props.rs
  • crates/perry-runtime/src/object/field_get_set/class_object_template.rs
  • crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs
  • crates/perry-runtime/src/object/field_set_by_name.rs
  • crates/perry-runtime/src/object/field_set_by_name/tail.rs
  • crates/perry-runtime/src/object/mod.rs
  • crates/perry-runtime/src/object/native_call_method.rs
  • crates/perry-runtime/src/object/native_call_method/invalid_utf8_method_tests.rs
  • crates/perry-runtime/src/object/native_module/class_method_bind.rs
  • crates/perry-runtime/src/object/native_module/class_method_values.rs
  • crates/perry-runtime/src/object/native_module/class_ref_values.rs
  • scripts/string_payload_access_baseline.txt
  • scripts/string_validator_allowlist.txt
  • test-files/test_issue_12202_wtf8_class_keys.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

📝 Walkthrough

The runtime now carries computed class member names as byte keys through registration, storage, lookup, reflection, and invocation. The changes add byte-oriented APIs across class and property operations, and add regression coverage for lone-surrogate keys.

Changes: WTF-8 Class Member Keys

Layer / File(s) Summary
Byte-key contracts and property operations
crates/perry-runtime/src/closure/*, crates/perry-runtime/src/object/class_image.rs, crates/perry-runtime/src/object/class_registry/declarations.rs, crates/perry-runtime/src/object/accessor_pair.rs, crates/perry-runtime/src/object/descriptor_state.rs
Closure property operations and descriptor helpers now accept byte keys. Class member names, ordering entries, and bind-length keys use byte vectors. Declaration lookup compares names by bytes rather than converting them to UTF-8 strings.
Class registration and materialization
crates/perry-runtime/src/object/class_registry*, crates/perry-runtime/src/object/class_value.rs, crates/perry-runtime/src/object/class_value/tests.rs, crates/perry-runtime/src/object/field_get_set/class_object_*, crates/perry-runtime/src/object/native_module/class_method_*, changelog.d/12471-wtf8-class-keys.md, scripts/*
Class declaration, prototype, accessor, and static-member registration paths use byte keys. Method materialization and class-value operations also use byte-oriented lookups. Tests cover class-value caching, collection, static properties, and computed-member declaration lookup.
Property access, reflection, and invocation
crates/perry-runtime/src/object/descriptors.rs, crates/perry-runtime/src/object/field_get_set/*, crates/perry-runtime/src/object/field_set_by_name*, crates/perry-runtime/src/object/native_call_method*, crates/perry-runtime/src/object/native_module/class_ref_values.rs, test-files/test_issue_12202_wtf8_class_keys.ts
Class property reads, writes, descriptors, own-key collection, accessor dispatch, and method calls retain key bytes for non-UTF-8 names. Regression tests cover exact-key lookup, receiver binding, descriptors, inheritance, and key enumeration.

Priority: ⬇️ Low

Estimated code review effort: 4 (Complex) | ~60 minutes

Change: Bug fix · Severity of issue fixed: Low

Sequence Diagram(s)

sequenceDiagram
  participant ClassEvaluation
  participant ClassRegistry
  participant PropertyStorage
  participant PropertyLookup
  ClassEvaluation->>ClassRegistry: Register computed member name bytes
  ClassRegistry->>PropertyStorage: Install property using key bytes
  PropertyLookup->>ClassRegistry: Resolve member using key bytes
  PropertyLookup->>PropertyStorage: Read property using exact key bytes
Loading

Possibly related PRs

  • PerryTS/perry#11416: It introduces declaration-prototype and accessor-chain paths that this change updates to use byte-slice keys.

Suggested reviewers: proggeramlug

Merge Risk: ⚪ Minimal · up to 26e4d

No actionable issue remains identified for this change. It is mergeable after normal checks.

Pre-merge checks · Passed 4 · Failed 1

Failed Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 55.73% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 253 functions across 32 files. (3 skipped… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks (4 passed)
Check name Status Explanation
Linked Issues check Passed Issue #12202 requires computed class method names with lone surrogates to retain WTF-8 bytes through registration and invocation. The PR adds byte-based declaration, registration, lookup, prototype, f…
Out of Scope Changes check Passed The changes stay connected to #12202. Runtime changes extend byte-key handling across the class declaration and property paths needed for the stated behavior. The added tests exercise the issue and it…
Title check Passed The title clearly and concisely describes the main change: preserving WTF-8 keys for computed class members.
Description check Passed The description explains the implementation scope, linked issue, test results, known limitations, validation environment, and failure attribution. It does not use the template headings or checklist fo…
Full details: Docstring Coverage

Explanation

Docstring coverage is 55.73% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 253 functions across 32 files. (3 skipped: 3 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@jdalton

jdalton commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@jdalton
jdalton marked this pull request as ready for review October 13, 2026 04:20
@jdalton
jdalton force-pushed the cloud/issue-12202-wtf8-class-keys branch from 26e4d02 to 51164be Compare October 13, 2026 14:24
@jdalton
jdalton force-pushed the cloud/issue-12202-wtf8-class-keys branch from 51164be to e395d65 Compare October 13, 2026 14:26
@proggeramlug
proggeramlug merged commit 55597ff into PerryTS:main Oct 13, 2026
93 of 100 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 method with a lone-surrogate computed name (["\uD800"]() {}) is registered under an empty name

2 participants