[PATCH] D132877: Downgrade the UAX31 diagnostic to a warning which defaults to an error

Tom Honermann via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 29 13:53:12 PDT 2022


tahonermann added a comment.

I don't have a strong opinion regarding when, or if, the diagnostic is reverted to an always-error. It looks like gcc is not even planning to diagnose identifiers that are ill-formed according to the new rules by default.

With regard to Corentin's opposition to the patch as is, I think it would be acceptable to put this patch in Clang 15 as is and then tighten things up as Corentin suggests for Clang 16. The patch as is is very low risk (it barely even qualifies as a code change). any change to address Corentin's concerns would add some additional risk this late in the release, and the only people impacted are those that opt-in to the new option. I do agree with Corentin's concern that suppressing the error does look like it would have the effect of allowing identifiers that were not previously allowed under the immutable identifier syntax; I think we do want to ensure that identifiers that are in neither immutable identifier syntax nor default identifier syntax are diagnosed (at least in Clang 16).



================
Comment at: clang/docs/ReleaseNotes.rst:119-121
+- Added the ``-Winvalid-identifier-character`` warning group to identify
+  identifier characters which are invalid according to UAX31 but were
+  previously allowed. This warning defaults to an error because these
----------------
Most people reading this won't know what UAX31 is. Linking to it would help some, but the document is fairly inscrutable to the uninitiated. How about something like the following?
  The ``-Winvalid-identifier-character`` warning group was added to manage diagnostics
  regarding use of invalid identifiers following the adoption of N2836 and P1949 by the
  C and C++ committees respectively. This warning default to an error because ...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132877/new/

https://reviews.llvm.org/D132877



More information about the cfe-commits mailing list