[Wdocumentation] Use C2x/C++14 deprecated attribute

[Mordante]

Summary

This replaces the non-standard __attribute__((deprecated)) with the standard [[deprecated]] when compiling in C2x/C++14 mode.

Discovered while looking at https://bugs.llvm.org/show_bug.cgi?id=43753

Depends: D71140

[aaron.ballman]

clang/lib/AST/CommentSema.cpp

693

This attribute also exists with this spelling in C2x, FWIW.

[Mordante]

clang/lib/AST/CommentSema.cpp

693

True, but unless I'm mistaken CPlusPlus17 and CPlusPlus2a also include CPlusPlus14. Do you prefer a different name for the Boolean?

[aaron.ballman]

clang/lib/AST/CommentSema.cpp

693

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.

[Mordante]

clang/lib/AST/CommentSema.cpp

693

Ah sorry I misread. I'll have a look at C2x. Thanks for the information.

[Mordante]

clang/lib/AST/CommentSema.cpp

693

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 ;-))

[Mordante]

Added C2x support as requested.

[aaron.ballman]

LGTM!

clang/lib/AST/CommentSema.cpp

693

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.

[Mordante]

Thanks for the review! I'll wait with committing until @gribozavr2 had time to review this patch and D71140.

clang/lib/AST/CommentSema.cpp

693

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.

[gribozavr2]

LGTM with comments fixed.

clang/lib/AST/CommentSema.cpp

697

I know I'm nit-picking, but I'd appreciate periods at the end of sentences.

clang/test/Sema/warn-documentation-fixits.c

27 ↗

(On Diff #232695)

"[[ATTRIBUTE]]"?

I expected either "[[deprecated]]" or "ATTRIBUTE".

Oh, these are FileCheck's regex brackets, which make it just a literal "ATTRIBUTE"... I think it would be clearer without the brackets :)

clang/test/Sema/warn-documentation-fixits.cpp

130–138

s/[[ATTRIBUTE]]/ATTRIBUTE/ here as well.

[Mordante]

Thanks for your review of this set of patches!

clang/lib/AST/CommentSema.cpp

697

No problem, I added them.

clang/test/Sema/warn-documentation-fixits.c

27 ↗

(On Diff #232695)

These are not simple regex bracket, but they are substitution blocks [1]. These are defined by the RUN scrips to select between the __attribute__((deprecated)) and [[deprecated]] replacements.

[1] https://llvm.org/docs/CommandGuide/FileCheck.html#filecheck-string-substitution-blocks

[gribozavr2]

clang/test/Sema/warn-documentation-fixits.c

27 ↗

(On Diff #232695)

Oh, it is FileCheck's -D flag... sorry I misread it. Please carry on.

clang/lib/AST/CommentSema.cpp

void Sema::checkDeprecatedCommand(const BlockCommandComment *Command) {
   if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
     // Don't emit a Fix-It for non-member function definitions.  GCC does not
     // accept attributes on them.
     const DeclContext *Ctx = FD->getDeclContext();
     if ((!Ctx || !Ctx->isRecord()) &&
         FD->doesThisDeclarationHaveABody())
       return;
 
-    StringRef AttributeSpelling = "__attribute__((deprecated))";
+    const bool CPlusPlus14 = FD->getASTContext().getLangOpts().CPlusPlus14;
+    StringRef AttributeSpelling =
+        CPlusPlus14 ? "[[deprecated]]" : "__attribute__((deprecated))";
     if (PP) {
-      TokenValue Tokens[] = {
-        tok::kw___attribute, tok::l_paren, tok::l_paren,
+      // Try to find a replacement macro:
+      // - In C++14 we prefer [[deprecated]]
+      // - If not found or an older C++ look for __attribute__((deprecated))
+      StringRef MacroName;
+      if (CPlusPlus14) {
+        TokenValue Tokens[] = {tok::l_square, tok::l_square,
                                PP->getIdentifierInfo("deprecated"),
-        tok::r_paren, tok::r_paren
-      };
-      StringRef MacroName = PP->getLastMacroWithSpelling(FD->getLocation(),
-                                                         Tokens);
+                               tok::r_square, tok::r_square};
+        MacroName = PP->getLastMacroWithSpelling(FD->getLocation(), Tokens);
+        if (!MacroName.empty())
+          AttributeSpelling = MacroName;
+      }
+
+      if (MacroName.empty()) {
+        TokenValue Tokens[] = {
+            tok::kw___attribute, tok::l_paren,
+            tok::l_paren,        PP->getIdentifierInfo("deprecated"),
+            tok::r_paren,        tok::r_paren};
+        StringRef MacroName =
+            PP->getLastMacroWithSpelling(FD->getLocation(), Tokens);
         if (!MacroName.empty())
           AttributeSpelling = MacroName;
       }
+    }
 
     SmallString<64> TextToInsert = AttributeSpelling;
     TextToInsert += " ";
     SourceLocation Loc = FD->getSourceRange().getBegin();
     Diag(Loc, diag::note_add_deprecation_attr)
         << FixItHint::CreateInsertion(Loc, TextToInsert);
   }
 }

clang/test/Sema/warn-documentation-fixits.cpp

 // RUN: %clang_cc1 -fsyntax-only -Wdocumentation -Wdocumentation-pedantic -fcomment-block-commands=foobar -verify %s
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wdocumentation -Wdocumentation-pedantic -fcomment-block-commands=foobar -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
-// RUN: %clang_cc1 -std=c++14 -fsyntax-only -Wdocumentation -Wdocumentation-pedantic -fcomment-block-commands=foobar -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck --check-prefixes=CHECK,CHECK14 %s
+// RUN  %clang_cc1 -std=c++11 -fsyntax-only -Wdocumentation -Wdocumentation-pedantic -fcomment-block-commands=foobar -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck -DATTRIBUTE="__attribute__((deprecated))" %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -Wdocumentation -Wdocumentation-pedantic -fcomment-block-commands=foobar -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck --check-prefixes=CHECK,CHECK14 -DATTRIBUTE="[[deprecated]]" %s
 
 // expected-warning@+1 {{parameter 'ZZZZZZZZZZ' not found in the function declaration}} expected-note@+1 {{did you mean 'a'?}}
 /// \param ZZZZZZZZZZ Blah blah.
 int test1(int a);
 
 // expected-warning@+1 {{parameter 'aab' not found in the function declaration}} expected-note@+1 {{did you mean 'aaa'?}}
 /// \param aab Blah blah.
 int test2(int aaa, int bbb);

 #endif
 };
 #define MY_ATTR_DEPRECATED __attribute__((deprecated))
 
 // expected-warning@+1 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}} expected-note@+2 {{add a deprecation attribute to the declaration to silence this warning}}
 /// \deprecated
 void test_deprecated_9(int a);
 
+#if __cplusplus >= 201402L
+#define ATTRIBUTE_DEPRECATED [[deprecated]]
+
+// expected-warning@+1 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}} expected-note@+2 {{add a deprecation attribute to the declaration to silence this warning}}
+/// \deprecated
+void test_deprecated_10(int a);
+#endif
+
 // rdar://12381408
 // expected-warning@+2  {{unknown command tag name 'retur'; did you mean 'return'?}}
 /// \brief testing fixit
 /// \retur int in FooBar
 int FooBar();
 
 // expected-warning@+1  {{unknown command tag name 'fooba'; did you mean 'foobar'?}}
 /// \fooba bbb IS_DOXYGEN_END
 int gorf();
 
 // expected-warning@+1 {{unknown command tag name}}
 /// \t bbb IS_DOXYGEN_END
 int Bar();
 
 // expected-warning@+2  {{unknown command tag name 'encode'; did you mean 'endcode'?}}
 // expected-warning@+1  {{'\endcode' command does not terminate a verbatim text block}}
 /// \encode PR18051
 int PR18051();
 
 // CHECK: fix-it:"{{.*}}":{6:12-6:22}:"a"
 // CHECK: fix-it:"{{.*}}":{10:12-10:15}:"aaa"
 // CHECK: fix-it:"{{.*}}":{14:13-14:23}:"T"
 // CHECK: fix-it:"{{.*}}":{19:13-19:18}:"SomeTy"
-// CHECK: fix-it:"{{.*}}":{26:1-26:1}:"__attribute__((deprecated)) "
-// CHECK: fix-it:"{{.*}}":{30:1-30:1}:"__attribute__((deprecated)) "
-// CHECK: fix-it:"{{.*}}":{35:3-35:3}:"__attribute__((deprecated)) "
-// CHECK: fix-it:"{{.*}}":{39:3-39:3}:"__attribute__((deprecated)) "
-// CHECK: fix-it:"{{.*}}":{47:3-47:3}:"__attribute__((deprecated)) "
-// CHECK: fix-it:"{{.*}}":{51:3-51:3}:"__attribute__((deprecated)) "
-// CHECK: fix-it:"{{.*}}":{76:3-76:3}:"__attribute__((deprecated)) "
-// CHECK: fix-it:"{{.*}}":{81:3-81:3}:"__attribute__((deprecated)) "
-// CHECK14: fix-it:"{{.*}}":{87:3-87:3}:"__attribute__((deprecated)) "
+// CHECK: fix-it:"{{.*}}":{26:1-26:1}:"[[ATTRIBUTE]] "
+// CHECK: fix-it:"{{.*}}":{30:1-30:1}:"[[ATTRIBUTE]] "
+// CHECK: fix-it:"{{.*}}":{35:3-35:3}:"[[ATTRIBUTE]] "
+// CHECK: fix-it:"{{.*}}":{39:3-39:3}:"[[ATTRIBUTE]] "
+// CHECK: fix-it:"{{.*}}":{47:3-47:3}:"[[ATTRIBUTE]] "
+// CHECK: fix-it:"{{.*}}":{51:3-51:3}:"[[ATTRIBUTE]] "
+// CHECK: fix-it:"{{.*}}":{76:3-76:3}:"[[ATTRIBUTE]] "
+// CHECK: fix-it:"{{.*}}":{81:3-81:3}:"[[ATTRIBUTE]] "
+// CHECK14: fix-it:"{{.*}}":{87:3-87:3}:"[[ATTRIBUTE]] "
 // CHECK: fix-it:"{{.*}}":{97:1-97:1}:"MY_ATTR_DEPRECATED "
-// CHECK: fix-it:"{{.*}}":{102:6-102:11}:"return"
-// CHECK: fix-it:"{{.*}}":{106:6-106:11}:"foobar"
-// CHECK: fix-it:"{{.*}}":{115:6-115:12}:"endcode"
+// CHECK14: fix-it:"{{.*}}":{104:1-104:1}:"ATTRIBUTE_DEPRECATED "
+// CHECK: fix-it:"{{.*}}":{110:6-110:11}:"return"
+// CHECK: fix-it:"{{.*}}":{114:6-114:11}:"foobar"
+// CHECK: fix-it:"{{.*}}":{123:6-123:12}:"endcode"

clang/test/Sema/warn-documentation.cpp

 
 // ----
 
 
 /// Aaa
 /// \deprecated Bbb
 void test_deprecated_1(int a) __attribute__((deprecated));
 
+#if __cplusplus >= 201402L
+/// Aaa
+/// \deprecated Bbb
+[[deprecated]] void test_deprecated_no_warning_std14(int a);
+#endif
+
 // We don't want \deprecated to warn about empty paragraph.  It is fine to use
 // \deprecated by itself without explanations.
 
 /// Aaa
 /// \deprecated
 void test_deprecated_2(int a) __attribute__((deprecated));
 
 /// Aaa