From e7964789dabaa5b9fb26c50f54deafb7fb8db4d6 Mon Sep 17 00:00:00 2001 From: Aaron Ballman Date: Mon, 7 Mar 2016 22:44:55 +0000 Subject: [PATCH] Implement support for [[nodiscard]] in C++1z that is based off existing support for warn_unused_result, and treat it as an extension pre-C++1z. This also means extending the existing warn_unused_result attribute so that it can be placed on an enum as well as a class. llvm-svn: 262872 --- clang/include/clang/AST/Decl.h | 6 +++- clang/include/clang/Basic/Attr.td | 11 ++++--- clang/include/clang/Basic/AttrDocs.td | 25 ++++++++++++++ .../clang/Basic/DiagnosticSemaKinds.td | 8 +++-- clang/include/clang/Sema/AttributeList.h | 3 +- clang/lib/AST/Decl.cpp | 16 ++++++--- clang/lib/Parse/ParseDeclCXX.cpp | 3 +- clang/lib/Sema/SemaDeclAttr.cpp | 6 ++++ clang/lib/Sema/SemaStmt.cpp | 12 +++---- clang/test/Sema/unused-expr.c | 8 ++--- clang/test/SemaCXX/nodiscard.cpp | 33 +++++++++++++++++++ .../SemaObjC/method-warn-unused-attribute.m | 4 +-- clang/www/cxx_status.html | 2 +- 13 files changed, 108 insertions(+), 29 deletions(-) create mode 100644 clang/test/SemaCXX/nodiscard.cpp diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h index 314e8f9f0404..04c4b46c9f9b 100644 --- a/clang/include/clang/AST/Decl.h +++ b/clang/include/clang/AST/Decl.h @@ -2025,12 +2025,16 @@ public: return getType()->getAs()->getCallResultType(getASTContext()); } + /// \brief Returns the WarnUnusedResultAttr that is either declared on this + /// function, or its return type declaration. + const Attr *getUnusedResultAttr() const; + /// \brief Returns true if this function or its return type has the /// warn_unused_result attribute. If the return type has the attribute and /// this function is a method of the return type's class, then false will be /// returned to avoid spurious warnings on member methods such as assignment /// operators. - bool hasUnusedResultAttr() const; + bool hasUnusedResultAttr() const { return getUnusedResultAttr() != nullptr; } /// \brief Returns the storage class as written in the source. For the /// computed linkage of symbol, see getLinkage. diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 221b50040f55..4ab7c0294b73 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -1537,11 +1537,12 @@ def WarnUnused : InheritableAttr { } def WarnUnusedResult : InheritableAttr { - let Spellings = [GCC<"warn_unused_result">, - CXX11<"clang", "warn_unused_result">]; - let Subjects = SubjectList<[ObjCMethod, CXXRecord, FunctionLike], WarnDiag, - "ExpectedFunctionMethodOrClass">; - let Documentation = [Undocumented]; + let Spellings = [CXX11<"", "nodiscard", 201603>, + CXX11<"clang", "warn_unused_result">, + GCC<"warn_unused_result">]; + let Subjects = SubjectList<[ObjCMethod, Enum, CXXRecord, FunctionLike], + WarnDiag, "ExpectedFunctionMethodEnumOrClass">; + let Documentation = [WarnUnusedResultsDocs]; } def Weak : InheritableAttr { diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 1808cea4c8bd..5de6b192715f 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -729,6 +729,31 @@ When one method overrides another, the overriding method can be more widely avai }]; } +def WarnUnusedResultsDocs : Documentation { + let Category = DocCatFunction; + let Heading = "nodiscard, warn_unused_result, clang::warn_unused_result, gnu::warn_unused_result"; + let Content = [{ +Clang supports the ability to diagnose when the results of a function call +expression are discarded under suspicious circumstances. A diagnostic is +generated when a function or its return type is marked with ``[[nodiscard]]`` +(or ``__attribute__((warn_unused_result))``) and the function call appears as a +potentially-evaluated discarded-value expression that is not explicitly cast to +`void`. + +.. code-block: c++ + struct [[nodiscard]] error_info { /*...*/ }; + error_info enable_missile_safety_mode(); + + void launch_missiles(); + void test_missiles() { + enable_missile_safety_mode(); // diagnoses + launch_missiles(); + } + error_info &foo(); + void f() { foo(); } // Does not diagnose, error_info is a reference. + }]; +} + def FallthroughDocs : Documentation { let Category = DocCatStmt; let Content = [{ diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index c5faa6eb74b7..e76be07ff51d 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -2475,7 +2475,7 @@ def warn_attribute_wrong_decl_type : Warning< "variables, functions and classes|Objective-C protocols|" "functions and global variables|structs, unions, and typedefs|structs and typedefs|" "interface or protocol declarations|kernel functions|non-K&R-style functions|" - "variables, enums, fields and typedefs}1">, + "variables, enums, fields and typedefs|functions, methods, enums, and classes}1">, InGroup; def err_attribute_wrong_decl_type : Error; def warn_type_attribute_wrong_type : Warning< @@ -6587,11 +6587,13 @@ def warn_side_effects_typeid : Warning< "expression with side effects will be evaluated despite being used as an " "operand to 'typeid'">, InGroup; def warn_unused_result : Warning< - "ignoring return value of function declared with warn_unused_result " - "attribute">, InGroup>; + "ignoring return value of function declared with %0 attribute">, + InGroup>; def warn_unused_volatile : Warning< "expression result unused; assign into a variable to force a volatile load">, InGroup>; +def ext_nodiscard_attr_is_a_cxx1z_extension : ExtWarn< + "use of the 'nodiscard' attribute is a C++1z extension">, InGroup; def warn_unused_comparison : Warning< "%select{%select{|in}1equality|relational}0 comparison result unused">, diff --git a/clang/include/clang/Sema/AttributeList.h b/clang/include/clang/Sema/AttributeList.h index ddcc3e909076..cfa3c0d17034 100644 --- a/clang/include/clang/Sema/AttributeList.h +++ b/clang/include/clang/Sema/AttributeList.h @@ -893,7 +893,8 @@ enum AttributeDeclKind { ExpectedObjectiveCInterfaceOrProtocol, ExpectedKernelFunction, ExpectedFunctionWithProtoType, - ExpectedVariableEnumFieldOrTypedef + ExpectedVariableEnumFieldOrTypedef, + ExpectedFunctionMethodEnumOrClass }; } // end namespace clang diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index 041b530bf07d..abaec2d7e10a 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -2935,16 +2935,22 @@ SourceRange FunctionDecl::getReturnTypeSourceRange() const { return RTRange; } -bool FunctionDecl::hasUnusedResultAttr() const { +const Attr *FunctionDecl::getUnusedResultAttr() const { QualType RetType = getReturnType(); if (RetType->isRecordType()) { const CXXRecordDecl *Ret = RetType->getAsCXXRecordDecl(); const auto *MD = dyn_cast(this); - if (Ret && Ret->hasAttr() && - !(MD && MD->getCorrespondingMethodInClass(Ret, true))) - return true; + if (Ret && !(MD && MD->getCorrespondingMethodInClass(Ret, true))) { + if (const auto *R = Ret->getAttr()) + return R; + } + } else if (const auto *ET = RetType->getAs()) { + if (const EnumDecl *ED = ET->getDecl()) { + if (const auto *R = ED->getAttr()) + return R; + } } - return hasAttr(); + return getAttr(); } /// \brief For an inline function definition in C, or for a gnu_inline function diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp index 798284b3d39a..15aded52b45e 100644 --- a/clang/lib/Parse/ParseDeclCXX.cpp +++ b/clang/lib/Parse/ParseDeclCXX.cpp @@ -3635,7 +3635,8 @@ static bool IsBuiltInOrStandardCXX11Attribute(IdentifierInfo *AttrName, case AttributeList::AT_FallThrough: case AttributeList::AT_CXX11NoReturn: return true; - + case AttributeList::AT_WarnUnusedResult: + return !ScopeName && AttrName->getName().equals("nodiscard"); default: return false; } diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index bb02e0b59c48..ce78ab009268 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -2463,6 +2463,12 @@ static void handleWarnUnusedResult(Sema &S, Decl *D, const AttributeList &Attr) return; } + // If this is spelled as the standard C++1z attribute, but not in C++1z, warn + // about using it as an extension. + if (!S.getLangOpts().CPlusPlus1z && Attr.isCXX11Attribute() && + !Attr.getScopeName()) + S.Diag(Attr.getLoc(), diag::ext_nodiscard_attr_is_a_cxx1z_extension); + D->addAttr(::new (S.Context) WarnUnusedResultAttr(Attr.getRange(), S.Context, Attr.getAttributeSpellingListIndex())); diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp index c58bf4640049..b73b4f088414 100644 --- a/clang/lib/Sema/SemaStmt.cpp +++ b/clang/lib/Sema/SemaStmt.cpp @@ -249,10 +249,10 @@ void Sema::DiagnoseUnusedExprResult(const Stmt *S) { // is written in a macro body, only warn if it has the warn_unused_result // attribute. if (const Decl *FD = CE->getCalleeDecl()) { - const FunctionDecl *Func = dyn_cast(FD); - if (Func ? Func->hasUnusedResultAttr() - : FD->hasAttr()) { - Diag(Loc, diag::warn_unused_result) << R1 << R2; + if (const Attr *A = isa(FD) + ? cast(FD)->getUnusedResultAttr() + : FD->getAttr()) { + Diag(Loc, diag::warn_unused_result) << A << R1 << R2; return; } if (ShouldSuppress) @@ -276,8 +276,8 @@ void Sema::DiagnoseUnusedExprResult(const Stmt *S) { } const ObjCMethodDecl *MD = ME->getMethodDecl(); if (MD) { - if (MD->hasAttr()) { - Diag(Loc, diag::warn_unused_result) << R1 << R2; + if (const auto *A = MD->getAttr()) { + Diag(Loc, diag::warn_unused_result) << A << R1 << R2; return; } } diff --git a/clang/test/Sema/unused-expr.c b/clang/test/Sema/unused-expr.c index 09359687d532..58ad8278f201 100644 --- a/clang/test/Sema/unused-expr.c +++ b/clang/test/Sema/unused-expr.c @@ -76,7 +76,7 @@ void t4(int a) { // rdar://7186119 int t5f(void) __attribute__((warn_unused_result)); void t5() { - t5f(); // expected-warning {{ignoring return value of function declared with warn_unused_result}} + t5f(); // expected-warning {{ignoring return value of function declared with 'warn_unused_result' attribute}} } @@ -88,11 +88,11 @@ int t6() { if (fn1() < 0 || fn2(2,1) < 0 || fn3(2) < 0) // no warnings return -1; - fn1(); // expected-warning {{ignoring return value of function declared with warn_unused_result attribute}} + fn1(); // expected-warning {{ignoring return value of function declared with 'warn_unused_result' attribute}} fn2(92, 21); // expected-warning {{ignoring return value of function declared with pure attribute}} fn3(42); // expected-warning {{ignoring return value of function declared with const attribute}} __builtin_abs(0); // expected-warning {{ignoring return value of function declared with const attribute}} - (void)0, fn1(); // expected-warning {{ignoring return value of function declared with warn_unused_result attribute}} + (void)0, fn1(); // expected-warning {{ignoring return value of function declared with 'warn_unused_result' attribute}} return 0; } @@ -101,7 +101,7 @@ int t7 __attribute__ ((warn_unused_result)); // expected-warning {{'warn_unused_ // PR4010 int (*fn4)(void) __attribute__ ((warn_unused_result)); void t8() { - fn4(); // expected-warning {{ignoring return value of function declared with warn_unused_result attribute}} + fn4(); // expected-warning {{ignoring return value of function declared with 'warn_unused_result' attribute}} } void t9() __attribute__((warn_unused_result)); // expected-warning {{attribute 'warn_unused_result' cannot be applied to functions without return value}} diff --git a/clang/test/SemaCXX/nodiscard.cpp b/clang/test/SemaCXX/nodiscard.cpp new file mode 100644 index 000000000000..5fa4177095bf --- /dev/null +++ b/clang/test/SemaCXX/nodiscard.cpp @@ -0,0 +1,33 @@ +// RUN: %clang_cc1 -fsyntax-only -std=c++1z -verify %s +// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify -DEXT %s + +#if !defined(EXT) +static_assert(__has_cpp_attribute(nodiscard) == 201603); + +struct [[nodiscard]] S {}; +S get_s(); +S& get_s_ref(); + +enum [[nodiscard]] E {}; +E get_e(); + +[[nodiscard]] int get_i(); + +void f() { + get_s(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}} + 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 + get_s_ref(); + (void)get_s(); + (void)get_i(); + (void)get_e(); +} + +[[nodiscard nodiscard]] int wrong1(); // expected-error {{attribute 'nodiscard' cannot appear multiple times in an attribute specifier}} + +namespace [[nodiscard]] N {} // expected-warning {{'nodiscard' attribute only applies to functions, methods, enums, and classes}} +#else +struct [[nodiscard]] S {}; // expected-warning {{use of the 'nodiscard' attribute is a C++1z extension}} +#endif // EXT diff --git a/clang/test/SemaObjC/method-warn-unused-attribute.m b/clang/test/SemaObjC/method-warn-unused-attribute.m index 042f4422f808..b83dabf3bbdc 100644 --- a/clang/test/SemaObjC/method-warn-unused-attribute.m +++ b/clang/test/SemaObjC/method-warn-unused-attribute.m @@ -9,8 +9,8 @@ void foo(INTF *a) { [a garf]; - [a fee]; // expected-warning {{ignoring return value of function declared with warn_unused_result attribute}} - [INTF c]; // expected-warning {{ignoring return value of function declared with warn_unused_result attribute}} + [a fee]; // expected-warning {{ignoring return value of function declared with 'warn_unused_result' attribute}} + [INTF c]; // expected-warning {{ignoring return value of function declared with 'warn_unused_result' attribute}} } diff --git a/clang/www/cxx_status.html b/clang/www/cxx_status.html index f116aee4dcc6..5027ae8174e8 100644 --- a/clang/www/cxx_status.html +++ b/clang/www/cxx_status.html @@ -634,7 +634,7 @@ as the draft C++1z standard evolves.

[[nodiscard]] attribute P0189R1 - No + SVN [[maybe_unused]] attribute -- 2.34.1