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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman marked an inline comment as done.
aaron.ballman added a comment.

In D132877#3756328 <https://reviews.llvm.org/D132877#3756328>, @efriedma wrote:

> If we don't specifically call out the deprecation period in the diagnostic, usage will grow beyond what we expect.  (Most people don't read the release notes; they'll just see it's possible to turn off the error message, and turn it off.)

I'm okay with that approach (I definitely agree putting the info in the diagnostic will get more eyeballs on it than putting it in the release notes), but at the same time, we use downgradable errors with the intent to turn them into a hard error in the future somewhat frequently and don't add any notice of pending doom for them. These diagnostics are all exposed in a way that makes it clear the code is an error, so anyone who blanket disables the error should not be too terribly surprised when it becomes a hard error later. (Putting the deprecation message into the warning itself has the same problem -- users who disable the warning entirely won't remember what the text of the warning was anyway, and users who inherit build system flags from may never even see the diagnostic messages at all.) So personally, I'm also fine not rewording the diagnostic.

> If we're okay with people deciding their own rules for what they want to allow in identifiers, we can just make the flag permanently available, though, and just drop the whole "deprecation period" discussion.  Or alternatively, we could add a separate diagnostic specifically for older standard modes: allow only the characters that were allowed by the older standards, and don't emit it for C++23 and newer.  That way, usage naturally goes away as people upgrade their code to newer standard modes.

There's quite a bit of discussion on the issue thread that goes into this, but there's pretty strong sentiment from Clang maintainers against making the flag permanently available. This behavior is controlled by standards bodies and allowing users to ignore that effectively makes a custom language subset (goes against our usual policy for language extensions). As Erich mentions, this was adopted as a DR in C++ so there's no older standards there. WG14 doesn't have the notion of DRs any longer and our replacement mechanism wasn't in place when the paper was adopted into C23. Technically that makes it a C23-only change, but I don't see any value to making this a DR in C++ but not in C given that both languages implement the same identifier rules and there's a reasonable user expectation that a valid identifier in C is also valid in C++ and vice versa (otherwise you run into problems linking C and C++ together).



================
Comment at: clang/docs/ReleaseNotes.rst:128
+  advised because we intend to turn the warning back into a hard error in Clang
+  18 after giving users a chance to update their code.
 
----------------
efriedma wrote:
> If this makes it into clang 15, we don't want to relnote it for clang 16.
Agreed -- I added the release note so folks could review it, but the plan is to either add it directly to the 15.x release notes or if we don't want to backport they'll be typical Clang 16 release notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132877