diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -474,6 +474,9 @@ unsigned character literals. This fixes `Issue 54886 `_. - Stopped allowing constraints on non-template functions to be compliant with dcl.decl.general p4. +- Added the attribute ``[[clang::discardable]]``, which cancels out ``[[nodiscard]]``. + This makes it possible for libraries to liberally use ``[[nodiscard]]`` inside + ``#pragma clang attribute``. C++20 Feature Support ^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h --- a/clang/include/clang/AST/Expr.h +++ b/clang/include/clang/AST/Expr.h @@ -3107,6 +3107,10 @@ /// function, or its return type declaration. const Attr *getUnusedResultAttr(const ASTContext &Ctx) const; + /// Returns the DisableWarnUnusedResultAttr that is either declared on the + /// called function, or its return type declaration. + const Attr *getDisableUnusedResultAttr(const ASTContext &Ctx) const; + /// Returns true if this call expression should warn on unused results. bool hasUnusedResultAttr(const ASTContext &Ctx) const { return getUnusedResultAttr(Ctx) != nullptr; diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -2981,6 +2981,12 @@ }]; } +def DisableWarnUnusedResult : InheritableAttr { + let Spellings = [Clang<"discardable", 0>]; + let Subjects = SubjectList<[ObjCMethod, Enum, Record, FunctionLike, TypedefName]>; + let Documentation = [DisableWarnUnusedResultDocs]; +} + def Weak : InheritableAttr { let Spellings = [GCC<"weak">]; let Subjects = SubjectList<[Var, Function, CXXRecord]>; diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -1862,6 +1862,37 @@ }]; } +def DisableWarnUnusedResultDocs : Documentation { + let Category = DocCatFunction; + let Heading = "clang::discardable"; + let Content = [{ +Although the default behaviour for C and C++ is to not diagnose ignored function +calls, we can make this the default by using ``#pragma clang attribute +push([[nodiscard]], apply_to = function)``. When we have a function that can +have its value discarded, we can use ``[[clang::discardable]]`` to indicate that +the ``[[nodiscard]]`` attribute should be ignored. + +``[[clang::discardable]]`` can be placed anywhere ``[[nodiscard]]`` is allowed, +but the presence of either is prioritised when it's applied to a callable. + +.. code-block c++ + struct S1 {}; + [[nodiscard, clang::discardable]] S1 f1(); + + struct [[nodiscard]] S2 {}; + [[clang::discardable]] int f2(); + + struct [[clang::discardable]] S3 {}; + [[nodiscard]] int f3(); + + int main() { + f1(); // no warning (cancels out) + f2(); // no warning (function attribute overrides type attribute) + f3(); // warno (function attribute overrides type attribute) + } + }]; +} + def FallthroughDocs : Documentation { let Category = DocCatStmt; let Heading = "fallthrough"; diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp --- a/clang/lib/AST/Expr.cpp +++ b/clang/lib/AST/Expr.cpp @@ -14,6 +14,7 @@ #include "clang/AST/APValue.h" #include "clang/AST/ASTContext.h" #include "clang/AST/Attr.h" +#include "clang/AST/Attrs.inc" #include "clang/AST/ComputeDependence.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclObjC.h" @@ -1534,6 +1535,35 @@ return D ? D->getAttr() : nullptr; } +const Attr *CallExpr::getDisableUnusedResultAttr(const ASTContext &Ctx) const { + // Callees have the final say on whether or not their call is discardable, so + // we exit early if the callee has an opinion. + + const Decl *D = getCalleeDecl(); + const auto *CallResult = + D ? D->getAttr() : nullptr; + if (CallResult != nullptr) + return CallResult; + + bool CallIsNodiscard = + D ? D->getAttr() != nullptr : false; + if (CallIsNodiscard && CallResult == nullptr) + return nullptr; + + // If the return type is a class or enum type that is marked discardable and + // the call isn't marked nodiscard, return the type attribute. + if (const TagDecl *TD = getCallReturnType(Ctx)->getAsTagDecl()) + if (const auto *A = TD->getAttr()) + return A; + + for (const auto *TD = getCallReturnType(Ctx)->getAs(); TD; + TD = TD->desugar()->getAs()) + if (const auto *A = TD->getDecl()->getAttr()) + return A; + + return nullptr; +} + SourceLocation CallExpr::getBeginLoc() const { if (isa(this)) return cast(this)->getBeginLoc(); @@ -2711,7 +2741,8 @@ } if (const ObjCMethodDecl *MD = ME->getMethodDecl()) - if (MD->hasAttr()) { + if (MD->hasAttr() && + !MD->hasAttr()) { WarnE = this; Loc = getExprLoc(); return true; diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -3167,6 +3167,39 @@ D->addAttr(::new (S.Context) WarnUnusedResultAttr(S.Context, AL, Str)); } +static void handleDisableWarnUnusedResult(Sema &S, Decl *D, + const ParsedAttr &AL) { + if (D->getFunctionType() && + D->getFunctionType()->getReturnType()->isVoidType() && + !isa(D)) { + S.Diag(AL.getLoc(), diag::warn_attribute_void_function_method) << AL << 0; + return; + } + if (const auto *MD = dyn_cast(D)) + if (MD->getReturnType()->isVoidType()) { + S.Diag(AL.getLoc(), diag::warn_attribute_void_function_method) << AL << 1; + return; + } + + if (AL.isStandardAttributeSyntax() && !AL.getScopeName()) { + // The standard attribute cannot be applied to variable declarations such + // as a function pointer. + if (isa(D)) + S.Diag(AL.getLoc(), diag::warn_attribute_wrong_decl_type_str) + << AL << "functions, classes, or enumerations"; + } + + if ((!AL.isGNUAttribute() && + !(AL.isStandardAttributeSyntax() && AL.isClangScope())) && + isa(D)) { + S.Diag(AL.getLoc(), diag::warn_unused_result_typedef_unsupported_spelling) + << AL.isGNUScope(); + return; + } + + D->addAttr(::new (S.Context) DisableWarnUnusedResultAttr(S.Context, AL)); +} + static void handleWeakImportAttr(Sema &S, Decl *D, const ParsedAttr &AL) { // weak_import only applies to variable & function declarations. bool isDef = false; @@ -8795,6 +8828,9 @@ case ParsedAttr::AT_WarnUnusedResult: handleWarnUnusedResult(S, D, AL); break; + case ParsedAttr::AT_DisableWarnUnusedResult: + handleDisableWarnUnusedResult(S, D, AL); + break; case ParsedAttr::AT_WeakRef: handleWeakRefAttr(S, D, AL); break; diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp --- a/clang/lib/Sema/SemaStmt.cpp +++ b/clang/lib/Sema/SemaStmt.cpp @@ -282,8 +282,12 @@ if (E->getType()->isVoidType()) return; - if (DiagnoseNoDiscard(*this, cast_or_null( - CE->getUnusedResultAttr(Context)), + if (CE->getDisableUnusedResultAttr(Context)) + return; + + if (DiagnoseNoDiscard(*this, + cast_or_null( + CE->getUnusedResultAttr(Context)), Loc, R1, R2, /*isCtor=*/false)) return; @@ -305,6 +309,11 @@ } } else if (const auto *CE = dyn_cast(E)) { if (const CXXConstructorDecl *Ctor = CE->getConstructor()) { + const auto *D = Ctor->getAttr(); + D = D ? D : Ctor->getParent()->getAttr(); + if (D) + return; + const auto *A = Ctor->getAttr(); A = A ? A : Ctor->getParent()->getAttr(); if (DiagnoseNoDiscard(*this, A, Loc, R1, R2, /*isCtor=*/true)) @@ -312,6 +321,8 @@ } } else if (const auto *ILE = dyn_cast(E)) { if (const TagDecl *TD = ILE->getType()->getAsTagDecl()) { + if (TD->getAttr()) + return; if (DiagnoseNoDiscard(*this, TD->getAttr(), Loc, R1, R2, /*isCtor=*/false)) @@ -328,6 +339,8 @@ } const ObjCMethodDecl *MD = ME->getMethodDecl(); if (MD) { + if (MD->getAttr()) + return; if (DiagnoseNoDiscard(*this, MD->getAttr(), Loc, R1, R2, /*isCtor=*/false)) return; diff --git a/clang/test/Misc/pragma-attribute-supported-attributes-list.test b/clang/test/Misc/pragma-attribute-supported-attributes-list.test --- a/clang/test/Misc/pragma-attribute-supported-attributes-list.test +++ b/clang/test/Misc/pragma-attribute-supported-attributes-list.test @@ -60,6 +60,7 @@ // CHECK-NEXT: DiagnoseAsBuiltin (SubjectMatchRule_function) // CHECK-NEXT: DisableSanitizerInstrumentation (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_variable_is_global) // CHECK-NEXT: DisableTailCalls (SubjectMatchRule_function, SubjectMatchRule_objc_method) +// CHECK-NEXT: DisableWarnUnusedResult (SubjectMatchRule_objc_method, SubjectMatchRule_enum, SubjectMatchRule_record, SubjectMatchRule_hasType_functionType, SubjectMatchRule_type_alias) // CHECK-NEXT: EnableIf (SubjectMatchRule_function) // CHECK-NEXT: EnforceTCB (SubjectMatchRule_function, SubjectMatchRule_objc_method) // CHECK-NEXT: EnforceTCBLeaf (SubjectMatchRule_function, SubjectMatchRule_objc_method) diff --git a/clang/test/SemaCXX/discardable.cpp b/clang/test/SemaCXX/discardable.cpp new file mode 100644 --- /dev/null +++ b/clang/test/SemaCXX/discardable.cpp @@ -0,0 +1,45 @@ +// RUN: %clang_cc1 -fsyntax-only -Wunknown-attributes -verify -std=c++17 %s + +[[clang::discardable]] int without_nodiscard(); + +[[nodiscard, clang::discardable]] int no_warn_fundamental(); + +struct naked {}; +[[nodiscard, clang::discardable]] naked no_warn_class_type(); +[[nodiscard, clang::discardable]] naked* no_warn_pointer(); +[[nodiscard, clang::discardable]] naked& no_warn_reference(); + +struct [[nodiscard]] nodiscard_type {}; +[[clang::discardable]] nodiscard_type no_warn_function_override(); + +struct [[clang::discardable]] discardable_type {}; +discardable_type no_warn_discardable_type(); +[[nodiscard]] discardable_type warns_function_override(); + +struct [[nodiscard, clang::discardable]] discardable_type2 {}; +discardable_type2 no_warn_discardable_type2(); + +struct discardable_members { + [[nodiscard, clang::discardable]] discardable_members(int); + [[nodiscard, clang::discardable]] int f() const; +}; + +void test_expression_statements() { + without_nodiscard(); + + no_warn_fundamental(); + + no_warn_class_type(); + no_warn_pointer(); + no_warn_reference(); + + no_warn_function_override(); + + no_warn_discardable_type(); + warns_function_override(); // expected-warning{{ignoring return value}} + + no_warn_discardable_type2(); + + discardable_members(0); + discardable_members(0).f(); +}