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%)