Skip to content

Match namereplace private name records to CPython UCD tables - #9827

Open
youknowdot wants to merge 1 commit into
RustPython:mainfrom
youknowdot:fix/unicode-name-capi-20261113
Open

youknowdot wants to merge 1 commit into
RustPython:mainfrom
youknowdot:fix/unicode-name-capi-20261113

Conversation

@youknowdot

@youknowdot youknowdot commented Nov 13, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Port CPython 3.15's UCD-file-order private name records used by namereplace.

CPython's Tools/unicode/makeunicodedata.py places name aliases at U+F0000 and named sequences at U+F0200. Its private getname callback includes these records for namereplace, while public unicodedata.name excludes them. Generate equivalent ordered tables from the already vendored UCD files, keeping existing public lookups unchanged.

For example U+F0000 now produces \N{NULL} and U+F0200 produces \N{KEYCAP NUMBER SIGN}, rather than numeric escapes.

Validation

  • Compared every record against CPython 3.15.2: 481 aliases and 461 named sequences, including public name(..., None) and out-of-range boundaries.
  • cargo test -p rustpython-unicode --all-features: 5 passed.
  • Unicode crate strict all-target/all-feature Clippy passed.
  • cargo check -p rustpython-common passed.
  • Commit hooks passed; added Python snippet regression assertions.
  • Independent review found no blocking issue.
  • Full VM runtime tests not yet run on this patch due limited disk; draft pending CI.

Scope

This does not fix the two remaining test_unicodedata expected failures concerning lazy module import/reload and compiler name escapes. Those markers remain unchanged. Proper shared provider lifecycle and Ruff parser callback need separate work, without accepting unsafe foreign pointers or introducing a fake capsule ABI. The casing/checksum fix is separate in #9824; no casing logic is duplicated here.

Summary by CodeRabbit

  • New Features
    • Unicode name replacement now recognizes designated code points for aliases and named sequences, producing their corresponding names in encoded output.
    • Other valid Unicode characters continue to use their standard character names, while invalid code points retain the existing numeric escape behavior.
  • Tests
    • Added coverage for alias and named-sequence replacements, range boundaries, and fallback behavior.

@coderabbitai

coderabbitai Bot commented Nov 13, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

Name replacement now resolves designated private-use code points through ordered alias and named-sequence tables. Other valid Unicode scalar values use their Unicode character names, and numeric escape fallbacks remain unchanged.

Changes: Unicode name replacement

Layer / File(s) Summary
Generate and resolve replacement names
crates/unicode/build.rs, crates/unicode/src/data.rs
The generator emits alias and named-sequence names in input order and asserts that the alias count is below 0x200. name_for_namereplace resolves the corresponding private-use ranges and falls back to Unicode character names for other valid scalar values. Tests cover table ordering, range boundaries, and fallback behavior.
Use replacement-name lookup in encoding
crates/common/src/encodings.rs, extra_tests/snippets/stdlib_unicode_shared.py
NameReplace::handle_encode_error uses name_for_namereplace. Tests check the replacement output for U+F0000 and U+F0200 and confirm that unicodedata.name returns None for both.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: joshuamegnauth54, youknowone

Merge Risk: Minimal · up to 255aa

This change makes namereplace produce the CPython-compatible names for the two designated private-use code points. Public Unicode name lookups are unchanged, and no actionable merge-blocking risk remains beyond CI confirmation.

Pre-merge checks: Passed 4

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title names the main change: matching namereplace private-use name records to the CPython UCD table order. The raw summary shows the encodings.rs lookup switch, the build.rs ordered tables, the ne…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title names the main change: matching namereplace private-use name records to the CPython UCD table order. The raw summary shows the encodings.rs lookup switch, the build.rs ordered tables, the new name_for_namereplace function, and the added tests all serve that goal. The title is specific and does not list files or use vague terms. It is 60 characters, within a reasonable length.

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

Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between cc6fb57 and 255aa03.

Files selected for processing: 4

  • crates/common/src/encodings.rs
  • crates/unicode/build.rs
  • crates/unicode/src/data.rs
  • extra_tests/snippets/stdlib_unicode_shared.py

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

@codspeed

codspeed Bot commented Nov 13, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 62 untouched benchmarks
⏩ 4 skipped benchmarks1


Comparing youknowdot:fix/unicode-name-capi-20261113 (9304be8) with main (e68658a)

Open in CodSpeed

Footnotes

  1. 4 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@youknowone

Copy link
Copy Markdown
Member

@joshuamegnauth54 could you review this patch?

@youknowdot
youknowdot force-pushed the fix/unicode-name-capi-20261113 branch from 9304be8 to 255aa03 Compare November 13, 2026 06:59
@youknowone
youknowone marked this pull request as ready for review November 13, 2026 08:16
@chatgpt-codex-connector

Copy link
Copy Markdown
Codex Review Summary: ❌ Failed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ❌ Failed 2026-11-13T08:16:45.512292Z
We couldn’t complete this review. Comment @codex review to try again.
255aa03 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@joshuamegnauth54

Copy link
Copy Markdown
Contributor

LGTM. The patch is referencing this code: https://github.com/python/cpython/blob/11eed559c09ecfaddd87dbf9c10fbe742f0f57de/Tools/unicode/makeunicodedata.py#L1136-L1172

I think it may help fix some of the EAW discrepancies that I couldn't figure out as well.

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.

3 participants