[PATCH] D71141: [Wdocumentation] Use C2x/C++14 deprecated attribute

[Mark de Wever via Phabricator via cfe-commits]

Mordante marked an inline comment as done.
Mordante added a comment.

Thanks for the review! I'll wait with committing until @gribozavr2 had time to review this patch and D71140 <https://reviews.llvm.org/D71140>.



================
Comment at: clang/lib/AST/CommentSema.cpp:693
+    StringRef AttributeSpelling =
+        CPlusPlus14 ? "[[deprecated]]" : "__attribute__((deprecated))";
     if (PP) {
----------------
aaron.ballman wrote:
> 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.
My reason not to use `[[deprecated]]` in C++11 is standard compliance. The original bug reporter was using clang-tidy to improve the code. We don't know whether users of clang-tidy only use the clang compiler or also other compilers which don't offer `[[deprecated]]` as C++11 extension.


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

https://reviews.llvm.org/D71141