<div dir="ltr"><div dir="ltr">On Sat, 20 Jul 2019 at 09:55, Aaron Ballman via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Author: aaronballman<br>
Date: Sat Jul 20 00:56:34 2019<br>
New Revision: 366626<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=366626&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=366626&view=rev</a><br>
Log:<br>
Implement P1301R4, which allows specifying an optional message on the [[nodiscard]] attribute.<br>
<br>
This also bumps the attribute feature test value and introduces the notion of a C++2a extension warning.<br>
<br>
Modified:<br>
    cfe/trunk/include/clang/Basic/Attr.td<br>
    cfe/trunk/include/clang/Basic/AttrDocs.td<br>
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
    cfe/trunk/lib/Sema/SemaDeclAttr.cpp<br>
    cfe/trunk/lib/Sema/SemaStmt.cpp<br>
    cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p1.cpp<br>
    cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp<br>
    cfe/trunk/test/Preprocessor/has_attribute.cpp<br>
    cfe/trunk/test/Sema/c2x-nodiscard.c<br>
    cfe/trunk/test/SemaCXX/cxx11-attr-print.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Basic/Attr.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=366626&r1=366625&r2=366626&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=366626&r1=366625&r2=366626&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Basic/Attr.td (original)<br>
+++ cfe/trunk/include/clang/Basic/Attr.td Sat Jul 20 00:56:34 2019<br>
@@ -2335,10 +2335,11 @@ def WarnUnused : InheritableAttr {<br>
 }<br>
<br>
 def WarnUnusedResult : InheritableAttr {<br>
-  let Spellings = [CXX11<"", "nodiscard", 201603>, C2x<"", "nodiscard">,<br>
+  let Spellings = [CXX11<"", "nodiscard", 201907>, C2x<"", "nodiscard">,<br></blockquote><div><br></div><div>We shouldn't bump the version until we also implement P1771R1, as this number indicates that both papers are implemented.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
                    CXX11<"clang", "warn_unused_result">,<br>
                    GCC<"warn_unused_result">];<br>
   let Subjects = SubjectList<[ObjCMethod, Enum, Record, FunctionLike]>;<br>
+  let Args = [StringArgument<"Message", 1>];<br>
   let Documentation = [WarnUnusedResultsDocs];<br>
 }<br>
<br>
<br>
Modified: cfe/trunk/include/clang/Basic/AttrDocs.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=366626&r1=366625&r2=366626&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=366626&r1=366625&r2=366626&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Basic/AttrDocs.td (original)<br>
+++ cfe/trunk/include/clang/Basic/AttrDocs.td Sat Jul 20 00:56:34 2019<br>
@@ -1482,6 +1482,13 @@ generated when a function or its return<br>
 potentially-evaluated discarded-value expression that is not explicitly cast to<br>
 `void`.<br>
<br>
+A string literal may optionally be provided to the attribute, which will be<br>
+reproduced in any resulting diagnostics. Redeclarations using different forms<br>
+of the attribute (with or without the string literal or with different string<br>
+literal contents) are allowed. If there are redeclarations of the entity with<br>
+differing string literals, it is unspecified which one will be used by Clang<br>
+in any resulting diagnostics.<br>
+<br>
 .. code-block: c++<br>
   struct [[nodiscard]] error_info { /*...*/ };<br>
   error_info enable_missile_safety_mode();<br>
<br>
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=366626&r1=366625&r2=366626&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=366626&r1=366625&r2=366626&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)<br>
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Sat Jul 20 00:56:34 2019<br>
@@ -7436,6 +7436,9 @@ def warn_side_effects_typeid : Warning<<br>
 def warn_unused_result : Warning<<br>
   "ignoring return value of function declared with %0 attribute">,<br>
   InGroup<UnusedResult>;<br>
+def warn_unused_result_msg : Warning<<br>
+  "ignoring return value of function declared with %0 attribute: %1">,<br>
+  InGroup<UnusedResult>;<br>
 def warn_unused_volatile : Warning<<br>
   "expression result unused; assign into a variable to force a volatile load">,<br>
   InGroup<DiagGroup<"unused-volatile-lvalue">>;<br>
@@ -7444,6 +7447,8 @@ def ext_cxx14_attr : Extension<<br>
   "use of the %0 attribute is a C++14 extension">, InGroup<CXX14>;<br>
 def ext_cxx17_attr : Extension<<br>
   "use of the %0 attribute is a C++17 extension">, InGroup<CXX17>;<br>
+def ext_cxx2a_attr : Extension<<br>
+  "use of the %0 attribute is a C++2a extension">, InGroup<CXX2a>;<br>
<br>
 def warn_unused_comparison : Warning<<br>
   "%select{equality|inequality|relational|three-way}0 comparison result unused">,<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=366626&r1=366625&r2=366626&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=366626&r1=366625&r2=366626&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Sat Jul 20 00:56:34 2019<br>
@@ -2841,14 +2841,30 @@ static void handleWarnUnusedResult(Sema<br>
       return;<br>
     }<br>
<br>
-  // If this is spelled as the standard C++17 attribute, but not in C++17, warn<br>
-  // about using it as an extension.<br>
-  if (!S.getLangOpts().CPlusPlus17 && AL.isCXX11Attribute() &&<br>
-      !AL.getScopeName())<br>
-    S.Diag(AL.getLoc(), diag::ext_cxx17_attr) << AL;<br>
+  StringRef Str;<br>
+  if ((AL.isCXX11Attribute() || AL.isC2xAttribute()) && !AL.getScopeName()) {<br>
+    // If this is spelled as the standard C++17 attribute, but not in C++17,<br>
+    // warn about using it as an extension. If there are attribute arguments,<br>
+    // then claim it's a C++2a extension instead.<br>
+    // FIXME: If WG14 does not seem likely to adopt the same feature, add an<br>
+    // extension warning for C2x mode.<br>
+    const LangOptions &LO = S.getLangOpts();<br>
+    if (AL.getNumArgs() == 1) {<br>
+      if (LO.CPlusPlus && !LO.CPlusPlus2a)<br>
+        S.Diag(AL.getLoc(), diag::ext_cxx2a_attr) << AL;<br>
+<br>
+      // Since this this is spelled [[nodiscard]], get the optional string<br>
+      // literal. If in C++ mode, but not in C++2a mode, diagnose as an<br>
+      // extension.<br>
+      // FIXME: C2x should support this feature as well, even as an extension.<br>
+      if (!S.checkStringLiteralArgumentAttr(AL, 0, Str, nullptr))<br>
+        return;<br>
+    } else if (LO.CPlusPlus && !LO.CPlusPlus17)<br>
+      S.Diag(AL.getLoc(), diag::ext_cxx17_attr) << AL;<br>
+  }<br>
<br>
   D->addAttr(::new (S.Context)<br>
-             WarnUnusedResultAttr(AL.getRange(), S.Context,<br>
+             WarnUnusedResultAttr(AL.getRange(), S.Context, Str,<br>
                                   AL.getAttributeSpellingListIndex()));<br>
 }<br>
<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaStmt.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=366626&r1=366625&r2=366626&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=366626&r1=366625&r2=366626&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaStmt.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaStmt.cpp Sat Jul 20 00:56:34 2019<br>
@@ -258,8 +258,13 @@ void Sema::DiagnoseUnusedExprResult(cons<br>
     if (E->getType()->isVoidType())<br>
       return;<br>
<br>
-    if (const Attr *A = CE->getUnusedResultAttr(Context)) {<br>
-      Diag(Loc, diag::warn_unused_result) << A << R1 << R2;<br>
+    if (const auto *A = cast_or_null<WarnUnusedResultAttr>(<br>
+            CE->getUnusedResultAttr(Context))) {<br>
+      StringRef Msg = A->getMessage();<br>
+      if (!Msg.empty())<br>
+        Diag(Loc, diag::warn_unused_result_msg) << A << Msg << R1 << R2;<br>
+      else<br>
+        Diag(Loc, diag::warn_unused_result) << A << R1 << R2;<br>
       return;<br>
     }<br>
<br>
@@ -290,7 +295,11 @@ void Sema::DiagnoseUnusedExprResult(cons<br>
     const ObjCMethodDecl *MD = ME->getMethodDecl();<br>
     if (MD) {<br>
       if (const auto *A = MD->getAttr<WarnUnusedResultAttr>()) {<br>
-        Diag(Loc, diag::warn_unused_result) << A << R1 << R2;<br>
+        StringRef Msg = A->getMessage();<br>
+        if (!Msg.empty())<br>
+          Diag(Loc, diag::warn_unused_result_msg) << A << Msg << R1 << R2;<br>
+        else<br>
+          Diag(Loc, diag::warn_unused_result) << A << R1 << R2;<br>
         return;<br>
       }<br>
     }<br>
<br>
Modified: cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p1.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p1.cpp?rev=366626&r1=366625&r2=366626&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p1.cpp?rev=366626&r1=366625&r2=366626&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p1.cpp (original)<br>
+++ cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p1.cpp Sat Jul 20 00:56:34 2019<br>
@@ -1,8 +1,8 @@<br>
-// RUN: %clang_cc1 -fsyntax-only -std=c++1z -verify %s<br>
+// RUN: %clang_cc1 -fsyntax-only -std=c++2a -verify %s<br>
<br>
 struct [[nodiscard]] S1 {}; // ok<br>
 struct [[nodiscard nodiscard]] S2 {}; // expected-error {{attribute 'nodiscard' cannot appear multiple times in an attribute specifier}}<br>
-struct [[nodiscard("Wrong")]] S3 {}; // expected-error {{'nodiscard' cannot have an argument list}}<br>
+struct [[nodiscard("Wrong")]] S3 {};<br>
<br>
 [[nodiscard]] int f();<br>
 enum [[nodiscard]] E {};<br>
<br>
Modified: cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp?rev=366626&r1=366625&r2=366626&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp?rev=366626&r1=366625&r2=366626&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp (original)<br>
+++ cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp Sat Jul 20 00:56:34 2019<br>
@@ -1,5 +1,6 @@<br>
+// RUN: %clang_cc1 -fsyntax-only -std=c++2a -verify -Wc++2a-extensions %s<br>
 // RUN: %clang_cc1 -fsyntax-only -std=c++17 -verify -Wc++17-extensions %s<br>
-// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify -DEXT -Wc++17-extensions %s<br>
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify -DEXT -Wc++17-extensions -Wc++2a-extensions %s<br>
<br>
 struct [[nodiscard]] S {};<br>
 S get_s();<br>
@@ -61,10 +62,33 @@ void f() {<br>
 }<br>
 } // namespace PR31526<br>
<br>
+struct [[nodiscard("reason")]] ReasonStruct {};<br>
+struct LaterReason;<br>
+struct [[nodiscard("later reason")]] LaterReason {};<br>
+<br>
+ReasonStruct get_reason();<br>
+LaterReason get_later_reason();<br>
+[[nodiscard("another reason")]] int another_reason();<br>
+<br>
+[[nodiscard("conflicting reason")]] int conflicting_reason();<br>
+[[nodiscard("special reason")]] int conflicting_reason();<br>
+<br>
+void cxx2a_use() {<br>
+  get_reason(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute: reason}}<br>
+  get_later_reason(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute: later reason}}<br>
+  another_reason(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute: another reason}}<br>
+  conflicting_reason(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute: special reason}}<br>
+}<br>
+<br>
 #ifdef EXT<br>
-// expected-warning@4 {{use of the 'nodiscard' attribute is a C++17 extension}}<br>
-// expected-warning@8 {{use of the 'nodiscard' attribute is a C++17 extension}}<br>
-// expected-warning@11 {{use of the 'nodiscard' attribute is a C++17 extension}}<br>
+// expected-warning@5 {{use of the 'nodiscard' attribute is a C++17 extension}}<br>
+// expected-warning@9 {{use of the 'nodiscard' attribute is a C++17 extension}}<br>
 // expected-warning@12 {{use of the 'nodiscard' attribute is a C++17 extension}}<br>
-// expected-warning@28 {{use of the 'nodiscard' attribute is a C++17 extension}}<br>
+// expected-warning@13 {{use of the 'nodiscard' attribute is a C++17 extension}}<br>
+// expected-warning@29 {{use of the 'nodiscard' attribute is a C++17 extension}}<br>
+// expected-warning@65 {{use of the 'nodiscard' attribute is a C++2a extension}}<br>
+// expected-warning@67 {{use of the 'nodiscard' attribute is a C++2a extension}}<br>
+// expected-warning@71 {{use of the 'nodiscard' attribute is a C++2a extension}}<br>
+// expected-warning@73 {{use of the 'nodiscard' attribute is a C++2a extension}}<br>
+// expected-warning@74 {{use of the 'nodiscard' attribute is a C++2a extension}}<br>
 #endif<br>
<br>
Modified: cfe/trunk/test/Preprocessor/has_attribute.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/has_attribute.cpp?rev=366626&r1=366625&r2=366626&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/has_attribute.cpp?rev=366626&r1=366625&r2=366626&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/Preprocessor/has_attribute.cpp (original)<br>
+++ cfe/trunk/test/Preprocessor/has_attribute.cpp Sat Jul 20 00:56:34 2019<br>
@@ -63,7 +63,7 @@ CXX11(unlikely)<br>
 // CHECK: maybe_unused: 201603L<br>
 // ITANIUM: no_unique_address: 201803L<br>
 // WINDOWS: no_unique_address: 0<br>
-// CHECK: nodiscard: 201603L<br>
+// CHECK: nodiscard: 201907L<br>
 // CHECK: noreturn: 200809L<br>
 // FIXME(201803L) CHECK: unlikely: 0<br>
<br>
<br>
Modified: cfe/trunk/test/Sema/c2x-nodiscard.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/c2x-nodiscard.c?rev=366626&r1=366625&r2=366626&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/c2x-nodiscard.c?rev=366626&r1=366625&r2=366626&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/Sema/c2x-nodiscard.c (original)<br>
+++ cfe/trunk/test/Sema/c2x-nodiscard.c Sat Jul 20 00:56:34 2019<br>
@@ -6,10 +6,12 @@ struct [[nodiscard]] S1 { // ok<br>
 struct [[nodiscard nodiscard]] S2 { // expected-error {{attribute 'nodiscard' cannot appear multiple times in an attribute specifier}}<br>
   int i;<br>
 };<br>
-struct [[nodiscard("Wrong")]] S3 { // expected-error {{'nodiscard' cannot have an argument list}}<br>
+struct [[nodiscard("Wrong")]] S3 { // FIXME: may need an extension warning.<br>
   int i;<br>
 };<br>
<br>
+struct S3 get_s3(void);<br>
+<br>
 [[nodiscard]] int f1(void);<br>
 enum [[nodiscard]] E1 { One };<br>
<br>
@@ -27,11 +29,13 @@ enum E2 get_e(void);<br>
<br>
 void f2(void) {<br>
   get_s(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}<br>
+  get_s3(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute: Wrong}}<br>
   get_i(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}<br>
   get_e(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}<br>
<br>
   // Okay, warnings are not encouraged<br>
   (void)get_s();<br>
+  (void)get_s3();<br>
   (void)get_i();<br>
   (void)get_e();<br>
 }<br>
<br>
Modified: cfe/trunk/test/SemaCXX/cxx11-attr-print.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/cxx11-attr-print.cpp?rev=366626&r1=366625&r2=366626&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/cxx11-attr-print.cpp?rev=366626&r1=366625&r2=366626&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/SemaCXX/cxx11-attr-print.cpp (original)<br>
+++ cfe/trunk/test/SemaCXX/cxx11-attr-print.cpp Sat Jul 20 00:56:34 2019<br>
@@ -37,13 +37,15 @@ void foo() __attribute__((const));<br>
 // CHECK: void bar() __attribute__((__const));<br>
 void bar() __attribute__((__const));<br>
<br>
-// CHECK: int f1() __attribute__((warn_unused_result));<br>
+// FIXME: It's unfortunate that the string literal prints with the below three<br>
+// cases given that the string is only exposed via the [[nodiscard]] spelling.<br>
+// CHECK: int f1() __attribute__((warn_unused_result("")));<br>
 int f1() __attribute__((warn_unused_result));<br>
<br>
-// CHECK: {{\[}}[clang::warn_unused_result]];<br>
+// CHECK: {{\[}}[clang::warn_unused_result("")]];<br>
 int f2 [[clang::warn_unused_result]] ();<br>
<br>
-// CHECK: {{\[}}[gnu::warn_unused_result]];<br>
+// CHECK: {{\[}}[gnu::warn_unused_result("")]];<br>
 int f3 [[gnu::warn_unused_result]] ();<br>
<br>
 // FIXME: ast-print need to print C++11<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div>