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