Repository navigation
Vendor unicode_names2 - #28784
Vendor unicode_names2#28784
unicode_names2#28784Conversation
Typing conformance resultsNo changes detected ✅Current numbersThe percentage of diagnostics emitted that were expected errors held steady at 98.15%. The percentage of expected errors that received a diagnostic held steady at 98.07%. The number of fully passing files held steady at 133/146. |
Memory usage reportMemory usage unchanged ✅ |
|
|
d9c1bd6 to
1321f68
Compare
| //! ```rust,ignore | ||
| //! #![feature(proc_macro_hygiene)] | ||
| //! | ||
| //! #[macro_use] | ||
| //! extern crate unicode_names2_macros; | ||
| //! |
There was a problem hiding this comment.
I didn't vendor this crate since we don't use it, so we had to skip compiling this example.
MichaReiser
left a comment
There was a problem hiding this comment.
I assume the reason why you chose to stick to the alias based implementation is to reduce divergence from upstream (and make it easier to move onto upstream in the future?)
Given that the upstream change is uncertain and there might also not be that much benefit of us to move back to it, I'm inclined to change our crate to always do strict matching, which allows us to remove the somewhat hard to understand matching in Ruff's parser.
There was a problem hiding this comment.
Thanks for writing this. I'm a bit worried that we'll forge tto bump the unicode version (I also don't know how to do that). Should we add a Update Python Version section to Contributing.md, and mention that we need to update the unicode standard and how to do this?
There was a problem hiding this comment.
Shouldn't commit 2 contain the new added alias method? At least, that's what your commit history says and commit 2 already uses the method.
I'd be inclined to only change ruff_unicode_names2 in this commit and to make the parser changes in a separate commit (makes it much easier to see what changes we made).
Given that this is a single PR, I think it could also make sense to rebase merge this PR, so that we retain this history
There was a problem hiding this comment.
Yeah, sorry. I noticed that they were out of order and thought about switching them but didn't think it mattered since I wasn't expecting to rebase-merge. I can tidy this up some more.
Although from your top-level comment, it sounds like I should just revert the loose-matching PR, and then I think we won't need any changes on our side. We'll just keep using the crate as before. But yes, I kept my upstream PR's approach with the hope that it would land and we would un-vendor. That's also why I didn't think much about future Unicode updates, but I'll write something about that too.
b4fa551 to
b2f7b7d
Compare
Merging this PR will improve performance by 7.25%
Performance Changes
Tip Curious why performance improved? Comment Comparing Footnotes
|
b2f7b7d to
31e1e8b
Compare
|
I think this is ready for another look. I switched to the strict-matching-only approach, restructured the commits, and updated the description. I also documented the Unicode update process, as you suggested. |
|
|
||
| ## Updating the Python version | ||
|
|
||
| When adding support for a new Python version, update Ruff's Unicode data to match |
There was a problem hiding this comment.
Technically not in the scope of this PR, so I'm fine deferring, but we could mention the PythonVersion changes here.
There was a problem hiding this comment.
I temporarily pushed this as a separate commit: 19b6f9a
Now I'll start some rebasing to see if we can rebase-merge the PR.
Copy the upstream runtime and generator before the alias-lookup API changes. Source: https://github.com/progval/unicode_names2/tree/63eda5de2bd0dd1e6243c9797efe5a84919a756b Adapt vendored Unicode crates to the Ruff workspace Wire the crates into the workspace, disable nightly-only targets, and configure lint and formatting checks to preserve the vendored source. Replace the relocated generator's broken license symlinks with copies of the licenses.
Restore registered alias spellings as lookup keys before removing loose matching. This reverts progval/unicode_names2#61. Reproduced in an upstream checkout at 63eda5de2bd0dd1e6243c9797efe5a84919a756b: git revert --no-commit 63eda5de2bd0dd1e6243c9797efe5a84919a756b Copy generator/src/lib.rs and src/lib.rs back into the corresponding vendored crates. The revert applies without conflicts.
Restore case-insensitive lookup with exact separator matching by reverting progval/unicode_names2#46. Keep Unicode 17 data and the later name-decoding performance improvements. After reverting alias normalization in the upstream checkout: git revert --no-commit d6b2923654a8c65a0155419d206e24b839ccd315 Resolve the sole conflict in src/lib.rs by deleting both the loose-matching documentation and the obsolete Cargo-enabled section, which upstream had already removed. Copy src/lib.rs, generator/src/lib.rs, and generator/src/phf.rs back into the corresponding vendored crates.
Use the vendored runtime and generator instead of the registry dependency, providing Unicode 17 names with Python-compatible matching. Update workspace dependencies and tool configuration, add parser regressions, and document how to update Unicode data when supporting a new Python version.
19b6f9a to
1a9d89e
Compare
Summary
This PR adds
ruff_unicode_names2andruff_unicode_names2_generatorcrates, vendored from theupstream
unicode_names2crate with the changes described below. To help with review, I organizedthis as 4 commits:
vendor the upstream crate from its master branch
Initially this diff command passed, until I started adapting the vendored crate to our workspace by updating the crate metadata, copying over some symlinked licenses, and configuring clippy and rustfmt to avoid changing anything.
revert my bug fix PR for an issue introduced by the loose matching PR
Equivalent to
git revert --no-commit 63eda5de2bd0dd1e6243c9797efe5a84919a756bin the upstreamrepo. I verified these by running the revert commands and repeating the diff above.
revert the loose matching PR, restoring the strict matching present in v1.3.0, which we
currently use
Equivalent to
git revert --no-commit d6b2923654a8c65a0155419d206e24b839ccd315in the upstreamrepo. There was one merge conflict in some docs, resolved by deleting both sides.
integrate the vendored crates into ruff
This updates our parser to use the vendored version instead of upstream, adds a few tests to
demonstrate Unicode 17 support and the lack of loose matching, and adds docs for future Unicode
version updates.
The motivations for vendoring are that 1) we don't want loose matching because CPython doesn't do loose matching for its Unicode character name escapes (e.g.
"\N{snowman}"), and 2) we do want Unicode 17 support. The former meant we couldn't upgrade from v1.3.0 to v2.0.0 (or v3 or v4), while the latter meant that we needed to upgrade. With the reverted commits in our vendored copy, we're closer to a fork of v1.3.0, as the loose matching is the largest change since then, but our version also includes the performance improvements from progval/unicode_names2#54.See progval/unicode_names2@v1.3.0...v4.0.0 for the full list of changes.
Test Plan
Vendored tests, plus a few new parser tests that exercise strict vs loose matching.