[PATCH] D71141: [Wdocumentation] Use C2x/C++14 deprecated attribute
Aaron Ballman via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Sat Dec 7 06:25:59 PST 2019
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM!
================
Comment at: clang/lib/AST/CommentSema.cpp:693
+ StringRef AttributeSpelling =
+ CPlusPlus14 ? "[[deprecated]]" : "__attribute__((deprecated))";
if (PP) {
----------------
Mordante wrote:
> Mordante wrote:
> > aaron.ballman wrote:
> > > Mordante wrote:
> > > > aaron.ballman wrote:
> > > > > This attribute also exists with this spelling in C2x, FWIW.
> > > > True, but unless I'm mistaken `CPlusPlus17` and `CPlusPlus2a` also include `CPlusPlus14`. Do you prefer a different name for the Boolean?
> > > I'm talking about C2x, not C++2a. The name for the variable is fine, but we should prefer `[[deprecated]]` in C2x mode to `__attribute__((deprecated))`.
> > >
> > > I think the correct predicate is: `getLangOpts().DoubleSquareBracketAttributes` -- if the user says they want to use double-square bracket attributes, we should probably prefer them to GNU-style attributes.
> > Ah sorry I misread. I'll have a look at C2x. Thanks for the information.
> `getLangOpts().DoubleSquareBracketAttributes` will not work since it includes C++11, which doesn't support `[[deprecated]]`, so I will just test for C++14 and C2x.
> (I had a look at the proper syntax in C2x and found N2334 ;-))
> getLangOpts().DoubleSquareBracketAttributes will not work since it includes C++11, which doesn't support [[deprecated]], so I will just test for C++14 and C2x.
We support `[[deprecated]]` in C++11 mode as an extension, but I suppose the concern there is pedantic warnings?
I think what you're doing now is fine, but it suggests that we need a better interface to `clang::hasAttribute()` so that you can test for support through the same machinery as `__has_cpp_attribute`/`__has_c_attribute`. It wouldn't be suitable to use here because you want to know something more specific than it can tell you (whether the attribute can be used without triggering an extension diagnostic), but this seems like a reasonable utility for us to have someday.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71141/new/
https://reviews.llvm.org/D71141
More information about the cfe-commits
mailing list