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?

https://github.com/llvm/llvm-project/pull/106036
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to