[clang] [llvm] [Clang] restrict use of attribute names reserved by the C++ standard (PR #106036)

[Aaron Ballman via llvm-commits]

AaronBallman wrote:

> > > > Oh, I see, you're suggesting we remove the `getStdNamespace` check from this PR. Yeah, I think that's reasonable.
> > > 
> > > 
> > > Yep, that is my suggestion, sorry I was insufficiently clear.
> > > > But I'd somewhat question whether this PR and warning really has anything to do with the attribute names being "reserved" at that point -- we're not checking whether they're reserved or not, and it really doesn't matter. Warning on a `#define` that clobbers the name of a standard attribute is just generally a good thing to do, regardless of whether you're using the standard library.
> > > 
> > > 
> > > I agree with this 100%. The link to the 'reserved by the standard' is I think a good additional justification.
> > > I think the proposal, complaining about these as reserved, is a good idea/good patch. BUT I think getting caught up in the "well, when is it technically NOT UB" is a waste of time, given that the warning is a good idea even without that justification.
> > 
> > 
> > I think the warning is justified even without a standard library header being included, but I also wonder if that means putting this under `-Wreserved-identifier` is the wrong home and maybe this is a `-Wattributes` warning group instead. We could reword the diagnostic to something along the lines of "macro name conflicts with the name of a %select{vendor attribute prefix|standard attribute|attribute name}0" and we warn on all three of these cases:
> > ```
> > #define msvc 12 // conflicts with [[msvc::no_unique_address]]
> > #define annotate 12 // conflicts with [[clang::annotate]]
> > #define nodiscard 12 // conflicts with [[nodiscard]]
> > ```
> > 
> > 
> >     
> >       
> >     
> > 
> >       
> >     
> > 
> >     
> >   
> > WDYT?
> 
> First, I think this needs its OWN warning group, as I can see justification for disabling just this.

Agreed, I didn't mean to imply otherwise, I was speaking about the top-level umbrella (sorry for the confusion).

> Second, I think that the 'standard' attribute interference is a level of severity HIGHER than the others thanks to being in the standard (and thus, perhaps, likely more to be used/interfered with).

I think the problem is the same regardless of whether the attribute is a standard attribute or a vendor attribute -- either way, redefining the meaning of something known to the implementation seems worth of letting the user know about.

> Third: DOES annotate conflict with attribute annotate? Isn't that a function/would have to be a function macro?

Ah, good point, but the basic premise still stands.

> Fourth: I think the 'reserved name' has a level of gravitas/concreteness that makes the diagnostic more meaningful/immediately obvious to folks. A diagnostic of, "This name you chose for your macro might make this attribute no workie" yields "Yeah, but i wont use that so I'm ok". A diagnostic of, "This name is UB because the standard reserves it!" yields a level of pause/consideration that we otherwise wouldn't get.

Yes, but the point is: these names aren't reserved but the pain can still happen. e.g., `cdecl` is not a reserved identifier, but re-defining its meaning can still break code in ways that could have better diagnostics: https://godbolt.org/z/vTfWPsYj9. Though, I suppose to reduce the chattiness, we could elect to diagnose when we see an attribute that is ill-formed due to the macro. e.g.,
```
#define cdecl 12
#define stdcall 100 // No warning, doesn't cause problems (yet)

void func() [[gnu::cdecl]]; // Expands to `[[gnu::12]] which is an error, so warn on the #define above
```
> SO I think the diagnostic being associated with it being reserved is COMPLETELY valid/justifiable. I think "this is a bad idea, UB or not" is a reason to not try at all to suppress this if we think you're not ACTUALLY breaking the rule (by not including an StdLib header).

If we're only going to diagnose conflicts with standard attribute names, then I can squint enough to agree. But I think that's an arbitrary limitation; we all just agreed that this isn't about using a reserved identifier so much as about writing a macro which will conflict with uses of attributes of the same name.

https://github.com/llvm/llvm-project/pull/106036