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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to