NFC normalize lifetime identifiers

[gmorenz]

Fixes #126759

[rustbot]

r? @madsmtm

rustbot has assigned @madsmtm.
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

[jieyouxu]

This is a observable language change in that previously we didn't accept

struct Foo<'a>(&'a ());
fn foo<'K>(_x: Foo<'K>, _y: Foo<'K>) {}
//      ^- latin K               ^- kelvin K

error[E0261]: use of undeclared lifetime name `'K`
 --> src/lib.rs:3:33
  |
3 | fn foo<'K>(_x: Foo<'K>, _y: Foo<'K>) {}
  |                                 ^^ undeclared lifetime
  |
help: consider introducing lifetime `'K` here
  |
3 | fn foo<'K, 'K>(_x: Foo<'K>, _y: Foo<'K>) {}
  |        +++

So when we do NFC-normalize the lifetimes, we are now accepting more code (but is consistent with the original RFC and regular identifiers). This needs a lang FCP, and probably an update to the reference on NFC-normalization.

@rustbot label: +I-lang-nominated

[gmorenz]

I think the reference is actually fine - I'm not sure if something changed since the original bug report or the original bug report was just a generous reading of the current reference as accepting the non-normalizing implementation.

The lifetime syntax page is here, and simply references the lifetime identifiers as either IDENTIFIER_OR_KEYWORD or NON_KEYWORD_IDENTIFIER as defined here which specifies that they should be normalized.

[traviscross]

We talked about this on the lang call today. We agreed that idents used for lifetimes should work in the same way here as idents in other positions.

@rfcbot fcp merge lang

[rust-rfcbot]

Team member @traviscross has proposed to merge this. The next step is review by the rest of the tagged team members:

• @joshtriplett
• @nikomatsakis
• @scottmcm
• @tmandry
• @traviscross

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

[tmandry]

@rfcbot reviewed

[rust-rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[scottmcm]

I absolutely think we should always have lifetimes treated like tick-then-ident, so if we're doing NFC for idents then 100% agreed we should do the same for lifetimes.

@rfcbot reviewed

[madsmtm]

Possibly perf-sensitive, we're now calling unicode_normalization::is_nfc_quick on every lifetime identifier (not that I really see a way of doing this work faster if it is indeed a regression).

@bors try @rust-timer queue

Implementation looks fine, feel free to r=me when FCP and perf finishes.

[rust-bors]

☀️ Try build successful (CI)
Build commit: 94d5cd8 (94d5cd8975524cac058eb459d5a2c53720210d2b, parent: b1b08cdef5ed3bca71c0d6638fccadc90d2e0ea3)

[rust-timer]

Finished benchmarking commit (94d5cd8): comparison URL.

Overall result: ❌✅ regressions and improvements - 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.1%	[0.1%, 0.1%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.1%]	1
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary 1.0%)

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

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

Cycles

Results (primary -2.8%, secondary 4.7%)

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

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

Binary size

Results (primary 0.0%, secondary 0.0%)

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

	mean	range	count
Regressions ❌ (primary)	0.0%	[0.0%, 0.0%]	4
Regressions ❌ (secondary)	0.0%	[0.0%, 0.1%]	14
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.0%	[0.0%, 0.0%]	4

Bootstrap: 467.027s -> 468.583s (0.33%)
Artifact size: 386.67 MiB -> 386.76 MiB (0.02%)

[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.

[madsmtm]

@bors r+

I'm unsure if there is a measurable perf regression, or if it's just spurious. In any case, not really something we can do differently.

[bors]

📌 Commit b1cde92 has been approved by madsmtm

It is now in the queue for this repository.

[gmorenz]

If there is a perf regression this suggests that the normalization code might be able to be an order of magnitude faster. Which given that we're already running this on all other identifiers - and I'd guess that there are a lot more normal identifiers than lifetime ones - might lead to a fairly sizable improvement.

A way to test if this is worth pursuing would be to run benchmarks with normalization commented out for normal identifiers as well.

[bors]

⌛ Testing commit b1cde92 with merge 2cd4ee6...

[bors]

☀️ Test successful - checks-actions
Approved by: madsmtm
Pushing 2cd4ee6 to main...

[github-actions]

What is this?

This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing 430d829 (parent) -> 2cd4ee6 (this PR)

Test differences

Show 6 test diffs

Stage 1

• [ui] tests/ui/lexer/ident_normalization.rs: [missing] -> pass (J0)

Stage 2

• [ui] tests/ui/lexer/ident_normalization.rs: [missing] -> pass (J1)

Additionally, 4 doctest diffs were found. These are ignored, as they are noisy.

Job group index

• J0: x86_64-gnu-llvm-20-3, x86_64-gnu-llvm-21-3
• J1: aarch64-apple, aarch64-gnu, aarch64-gnu-llvm-20-1, aarch64-msvc-1, arm-android, armhf-gnu, dist-i586-gnu-i586-i686-musl, i686-gnu-1, i686-gnu-nopt-1, i686-msvc-1, test-various, x86_64-gnu, x86_64-gnu-debug, x86_64-gnu-gcc, x86_64-gnu-llvm-20, x86_64-gnu-llvm-20-2, x86_64-gnu-llvm-21-2, x86_64-gnu-nopt, x86_64-gnu-stable, x86_64-mingw-1, x86_64-msvc-1

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard 2cd4ee6bcf517345dd76d7b102d56b2e873cddbc --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

1. dist-various-1: 4105.9s -> 6357.6s (+54.8%)
2. dist-x86_64-apple: 7002.3s -> 8299.8s (+18.5%)
3. dist-ohos-aarch64: 4731.2s -> 4128.5s (-12.7%)
4. x86_64-gnu-llvm-21-3: 6244.2s -> 7018.2s (+12.4%)
5. dist-aarch64-llvm-mingw: 5438.7s -> 6066.3s (+11.5%)
6. dist-sparcv9-solaris: 4974.8s -> 5495.5s (+10.5%)
7. x86_64-gnu-llvm-20: 2465.2s -> 2706.4s (+9.8%)
8. aarch64-msvc-2: 6531.2s -> 6008.5s (-8.0%)
9. dist-ohos-armv7: 4434.7s -> 4083.5s (-7.9%)
10. dist-s390x-linux: 5326.2s -> 4929.7s (-7.4%)

How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

[rust-timer]

Finished benchmarking commit (2cd4ee6): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

Max RSS (memory usage)

Results (primary 1.5%, secondary -2.5%)

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

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

Cycles

Results (secondary -0.8%)

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

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

Binary size

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

Bootstrap: 473.205s -> 474.624s (0.30%)
Artifact size: 389.51 MiB -> 389.52 MiB (0.00%)