Page MenuHomePhabricator

[Clang] Allow additional mathematical symbols in identifiers.
ClosedPublic

Authored by cor3ntin on Oct 30 2022, 3:25 PM.

Details

Summary

Implement the proposed UAX Profile
"Mathematical notation profile for default identifiers".

This implements a not-yet approved Unicode for a vetted
UAX31 identifier profile
https://www.unicode.org/L2/L2022/22230-math-profile.pdf

This change mitigates the reported disruption caused
by the implementation of UAX31 in C++ and C2x,
as these mathematical symbols are commonly used in the
scientific community.

Fixes #54732

Diff Detail

Event Timeline

cor3ntin created this revision.Oct 30 2022, 3:25 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 30 2022, 3:25 PM
cor3ntin requested review of this revision.Oct 30 2022, 3:25 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 30 2022, 3:25 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript

Note that we should wait end of next week to hear back from the Unicode Technical Committee before merging this

cor3ntin updated this revision to Diff 471882.Oct 30 2022, 3:36 PM

Remove extra semi colon.

I apologize for the delay Corentin, I hope to get through a number of my outstanding code reviews tomorrow.

tahonermann accepted this revision.Dec 8 2022, 2:00 PM

This looks great to me. I added a minor comment regarding the grammar of the added diagnostic, but am accepting the review regardless.

clang/include/clang/Basic/DiagnosticLexKinds.td
129–130

I think this diagnostic should also have an "an" before "identifier" if you wish to take on correcting it as well.

135–137

The grammar seems slightly off here. I think there should be an "an" before "identifier":

  • "mathematical notation character <U+%0> in an identifier is a Clang extension"

This would be consistent with err_character_not_allowed_identifier and warn_c99_compat_unicode_id.

This revision is now accepted and ready to land.Dec 8 2022, 2:00 PM

Oh, also, the review summary mentions "UAX32" (twice); those should be "UAX31". Just please make sure the commit message is correct.

Also, I'm terribly sorry for being so slow to get this reviewed!

Oh, also, the review summary mentions "UAX32" (twice); those should be "UAX31". Just please make sure the commit message is correct.

Also, I'm terribly sorry for being so slow to get this reviewed!

I'm glad you noticed that...!

cor3ntin updated this revision to Diff 481556.Dec 9 2022, 1:29 AM

Fix grammar in diagnostic.

cor3ntin edited the summary of this revision. (Show Details)Dec 9 2022, 1:30 AM
cor3ntin added a reviewer: Restricted Project.

Thanks Tom. I'm adding more reviewers in case someone else wants to look at it

tahonermann accepted this revision.Dec 9 2022, 7:41 AM
cor3ntin updated this revision to Diff 482778.Dec 14 2022, 3:02 AM

clang-format

This revision was landed with ongoing or failed builds.Dec 16 2022, 1:21 AM
This revision was automatically updated to reflect the committed changes.