parser/lexer: bump to Unicode 17, use faster unicode-ident

[Marcondiro]

Hello,

Bump the unicode version used by lexer/parser to 17.0.0 by updating:

• unicode-normalization to 0.1.25
• unicode-properties to 0.1.4
• unicode-width to 0.2.2

and by replacing unicode-xid with unicode-ident which is also 6 times faster.
I think it might be worth to run the benchmarks to double check.
(unicode-ident is already in src/tools/tidy/src/deps.rs)

Thanks!

[rustbot]

The list of allowed third-party dependencies may have been modified! You must ensure that any new dependencies have compatible licenses before merging.

cc @davidtwco, @wesleywiser

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

[rustbot]

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[rustbot]

If the Unicode version changes are intentional,
it should also be updated in the reference at
https://github.com/rust-lang/reference/blob/HEAD/src/identifiers.md.

cc @ehuss

[Kobzol]

@bors try @rust-timer queue

(Parsing could be affected too).

[rust-bors]

☀️ Try build successful (CI)
Build commit: 988451c (988451ce73b832a095adca69acf309ce27a2f54d, parent: 23c7bad921fb7163de37ea680bed317deaa03fda)

[rust-timer]

Finished benchmarking commit (988451c): comparison URL.

Overall result: ❌ regressions - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.2%	[0.1%, 0.3%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

Results (primary 2.6%, secondary 3.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	2.6%	[2.6%, 2.6%]	1
Regressions ❌ (secondary)	3.6%	[3.6%, 3.6%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	2.6%	[2.6%, 2.6%]	1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 473.971s -> 474.835s (0.18%)
Artifact size: 390.89 MiB -> 390.89 MiB (-0.00%)

[clarfonthey]

Is there a reason why the reference explicitly specifies the Unicode version in a way that makes it feel like updating that version is a nontrivial change?

i.e., is there a reason why it does not clarify that the Unicode version in the compiler is allowed to be (and should be) bumped whenever Unicode releases a new version, and to simply say something like "it is version N as of Rust 1.M"?

[joshtriplett]

I think it needs some level of review, to make sure that (for instance) the new Unicode version isn't doing anything out of the ordinary, and to make sure that some person in the project experienced with Unicode has taken at least a cursory look at the changes to XID_Start/XID_Continue and any related changes to confusables that overlap with XID_Start/XID_Continue.

I don't think that review is best done in lang. I think we should delgate that to whichever team is making sure of the above. (Is that T-compiler?) So, ideally, I'd love to see a proposal to lang requesting a delegation to take responsibility for the above.

That said, let's go ahead and sign off on this change to unblock it.

[bors]

☔ The latest upstream changes (presumably #147029) made this pull request unmergeable. Please resolve the merge conflicts.

[Mark-Simulacrum]

r? @joshtriplett

I tend to agree that FCP seems like the wrong tool to confirm implementation details (e.g., membership of XID sets). I don't know what specific review you'd like to perform so yielding to you.

[rustbot]

joshtriplett is not on the review rotation at the moment.
They may take a while to respond.

[rustbot]

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[traviscross]

cc @rust-lang/fls

[rust-rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

[ehuss]

@simulacrum @Josh Triplett This PR seems to be stuck in limbo. I'm not quite following exchange. Josh, were you specifically asking someone to do an analysis now of the codepoints that changed? Or were you saying that is something that should usually be done in the future?

@simulacrum I think Josh was saying they specifically don't want to do the review.

Are you saying that unicode-security needs to be updated to Unicode 17 before this can continue? Are Unicode updates required to keep the confusables list in sync?

If you want a diff, here it is: https://gist.github.com/ehuss/4ec7304c08bc3d179febf3fff8fdbacd
The XID changes are described in UAX31 Revision 43.
The security changes are described in UTS39 Revision 32.

[Mark-Simulacrum]

Personally, I'm happy to r+ this, but several people on lang indicated they wanted some kind of review. It's not obvious to me whether their check boxes on the FCP indicate they have done said review (it's not clear to me what would go into that, so I can't really check).

It's not clear to me what would block us from bumping the Unicode version we're using (maybe I don't know enough, though), so in light of that uncertainty I didn't want to just r+. But if we treat the FCP as having resolved it then I'm happy to effectively rubber stamp this.

@ehuss, you had some concerns over using different crates here - happy for us to not approve this for those reasons (I don't fully understand that concern but haven't dug into the details).

[clarfonthey]

Since all the relevant crates seem to export some flavour of the UNICODE_VERSION constant included in the standard library, perhaps there could be a simple test somewhere that asserts that all these constants are identical? That way manual review of the versions isn't required.

[traviscross]

From the lang side, as @scottmcm mentioned,

I would emphasize that the lang FCP here is just about bumping the unicode version in the abstract

our FCP covered only the decision to change the set of Rust programs that are accepted by virtue of changing the version of Unicode that Rust is defined to use. It did not indicate that we had done any review beyond that we felt necessary to OK this in terms of the language definition.

No concerns have been filed, and our FCP is complete, so if an r+ reviewer is happy with this change (on the understanding that nobody on lang necessarily did the detailed check that @joshtriplett mentioned), it's procedurally good to go as far as the lang team is concerned.

[Mark-Simulacrum]

Since all the relevant crates seem to export some flavour of the UNICODE_VERSION constant included in the standard library, perhaps there could be a simple test somewhere that asserts that all these constants are identical? That way manual review of the versions isn't required.

This sounds like a good idea. Can we add a static assert or similar on this?

No concerns have been filed, and our FCP is complete [...]

Given the above I think I'm OK moving forward with the asserts added. r=me with that done.

[clarfonthey]

This sounds like a good idea. Can we add a static assert or similar on this?

I was poking around on the feasibility of this and will post the relevant PRs when I get around to it. Ultimately, after looking more into the relevant stuff, it really doesn't feel like it matters that much; most of the "worst case scenarios" are just things like identifiers using just-added scripts being accepted by cargo and not by the compiler, or vice/versa, which still would result in an error regardless. Unicode has a pretty strong stability guarantee and they take care to not cause too many issues.

That said, I'll be sure to link the relevant PRs back here in case people are curious.

[joshtriplett]

Clarifying something: the request for the future, not blocking this PR, is that we have some set of folks (e.g. @Manishearth) who know Unicode well that we could delegate the "version N+1 LGTM" decision to in the future.

[Manishearth]

I'd be willing to help but I think for that to work the relevant teams would have to codify what they are looking for.

For reference, here's the difference between Unicode 16 and 17 for XID_Start:

https://util.unicode.org/UnicodeJsps/list-unicodeset.jsp?a=%5B%3AU17%3AXID_Start%3A%5D+-+%5B%3AU16%3AXID_Start%3A%5D&g=&i=idstatus .

(and here are the new XID_Continue-only characters)

Is it a question of mismatched goals between Rust and Unicode, where Unicode's bar for "valid in identifiers" is subtly different from that of Rust? Is it a question of trust in Unicode's ability to hit that bar? Or something else?

The rough definition from the spec is this:

The formal syntax provided here captures the general intent that an identifier consists of a string of characters beginning with a letter or an ideograph, and followed by any number of letters, ideographs, digits, or underscores. It provides a definition of identifiers that is guaranteed to be backward compatible with each successive release of Unicode, but also adds any appropriate new Unicode characters.

If it's about Rust having a different bar, then Rust needs to articulate what that bar is. There's a good chance that we can even write tests against Unicode data that verify some of those properties. This is what we did in the non-ascii idents RFC: Rust articulated its concerns about it, and we were able to crystallize those into lints that use other Unicode properties (ones explicitly designed for this purpose, even).

If it's about Unicode's ability to hit that bar, I can certainly give such a list a once-over and check if there aren't any mistakes. But beyond that, I am literally on the body that assigns these properties, and if I needed help to verify such a thing (no one person understands all of the things in Unicode) I'd just be asking other people on that body, so I don't think that would be any sort of meaningful exercise.

---

For instance, unicode-width got updated to Unicode 16 few months after unicode-xid got its update. Or even right now the latest unicode-xid on crates.io is still based on Unicode 16 while normalization is on 17.

Note that this is mostly because unicode_rs crates get bumped when someone wants them to be. If Rust wanted them to be bumped, they can be bumped (best way: make a PR with the bump).

[Marcondiro]

Since all the relevant crates seem to export some flavour of the UNICODE_VERSION constant included in the standard library, perhaps there could be a simple test somewhere that asserts that all these constants are identical? That way manual review of the versions isn't required.

At the moment there is this test in place, checking that rustc_lexer::UNICODE_IDENT_VERSION == rustc_parse::UNICODE_NORMALIZATION_VERSION

Would expanding this test to all the Unicode-version dependent crates used in /compiler/* be enough?

[clarfonthey]

So, to clarify, this is exactly what I was proposing in this comment: #148321 (comment)

I was looking into it, and my particular method I was going with was just adding an anonymous constant in each of the crates to make a static assertion. So, for example, in rustc_lexer, this is what I currently have right now in my (unpublished) branch:

// ensure that unicode version is same as libstd
const _: () = {
    let internal = std::char::UNICODE_VERSION;
    let properties = unicode_properties::UNICODE_VERSION;
    assert!(internal.0 as u64 == properties.0);
    assert!(internal.1 as u64 == properties.1);
    assert!(internal.2 as u64 == properties.2);
};
const _: () = {
    let internal = std::char::UNICODE_VERSION;
    let xid = unicode_xid::UNICODE_VERSION;
    assert!(internal.0 as u64 == xid.0);
    assert!(internal.1 as u64 == xid.1);
    assert!(internal.2 as u64 == xid.2);
};

(note: I'm using libstd's version as an anchor because it's the most convenient: if all of them match, then libstd should match. also, while I could use unstable features to make this simpler with assert_eq, I chose to go with something that works on stable atm just for ease of maintenance)

[clarfonthey]

Gonna preface this with: my main reason for having this discussion in the first place is, as I mostly mentioned, I think that bumping the Unicode version, since it is a routine change, should have some sort of playbook so we don't need to have these discussions again. And, so, I'm having these discussions here because I think the formation of said playbook is relevant, but if you'd rather move it somewhere else, I'm fine doing that too.

Note that this is mostly because unicode_rs crates get bumped when someone wants them to be. If Rust wanted them to be bumped, they can be bumped (best way: make a PR with the bump).

Not that this is the place to put this, but the exact reason why I submitted unicode-rs/unicode-script#25 (which you merged) is because Clippy uses unicode-script and it doesn't have a published version for Unicode 17, so, an attempt to enforce that all the Unicode versions match for clippy specifically fail. I imagine, then, if we were to enforce all of this, the procedure would then be to ping all of the authors of these crates, ensure that they're all updated to the latest version of Unicode, then perform the bump for all R-L crates? While this is also a request to do so for this particular crate, I'm also using it as an example since this would be part of what people should do going forward.

Right now, tidy only allows checking particular crates in Cargo.lock, and not explicit versions of them, so, we can't particularly enforce only versions which support the right Unicode version. This is a potential side concern of verifying the Unicode version is consistent: even though we can check the UNICODE_VERSION const for everything we use, we can't verify that dependencies also use the same version, which, even if it doesn't matter that much, is still probably better for the sake of reducing table size in binaries. So, I guess, that's also a side note here. I'm not sure exactly what the policy is for allowing dependencies in the compiler, but I assume that at least part of it is some assurance that the people who maintain them are at least active enough to merge changes if the compiler needs it.

There's also a potential consideration of just including the Unicode functionality most critical to the compiler directly in the compiler itself. Since all of the code used has a compatible license, it shouldn't be tremendously difficult to copy it in somewhere, and it would also mean that we could share some of the code that parses stuff like UnicodeData.txt between these various crates. This isn't to say it should be part of libstd, but maybe having it in the compiler to make version consistency easier is a consideration.