r366626 - Implement P1301R4, which allows specifying an optional message on the [[nodiscard]] attribute.
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 22 05:49:11 PDT 2019
On Sun, Jul 21, 2019 at 7:40 AM Richard Smith <richard at metafoo.co.uk> wrote:
>
> On Sat, 20 Jul 2019 at 09:55, Aaron Ballman via cfe-commits <cfe-commits at 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 at 4 {{use of the 'nodiscard' attribute is a C++17 extension}}
>> -// expected-warning at 8 {{use of the 'nodiscard' attribute is a C++17 extension}}
>> -// expected-warning at 11 {{use of the 'nodiscard' attribute is a C++17 extension}}
>> +// expected-warning at 5 {{use of the 'nodiscard' attribute is a C++17 extension}}
>> +// expected-warning at 9 {{use of the 'nodiscard' attribute is a C++17 extension}}
>> // expected-warning at 12 {{use of the 'nodiscard' attribute is a C++17 extension}}
>> -// expected-warning at 28 {{use of the 'nodiscard' attribute is a C++17 extension}}
>> +// expected-warning at 13 {{use of the 'nodiscard' attribute is a C++17 extension}}
>> +// expected-warning at 29 {{use of the 'nodiscard' attribute is a C++17 extension}}
>> +// expected-warning at 65 {{use of the 'nodiscard' attribute is a C++2a extension}}
>> +// expected-warning at 67 {{use of the 'nodiscard' attribute is a C++2a extension}}
>> +// expected-warning at 71 {{use of the 'nodiscard' attribute is a C++2a extension}}
>> +// expected-warning at 73 {{use of the 'nodiscard' attribute is a C++2a extension}}
>> +// expected-warning at 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 at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list