diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 95a0bbf734056..46290b6c39c9b 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -139,6 +139,9 @@ Bug Fixes to Compiler Builtins Bug Fixes to Attribute Support ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +- ``[[nodiscard]]`` is now respected on Objective-C and Objective-C++ methods. + (#GH141504) + Bug Fixes to C++ Support ^^^^^^^^^^^^^^^^^^^^^^^^ - Diagnose binding a reference to ``*nullptr`` during constant evaluation. (#GH48665) diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h index 523c0326d47ef..708c6e2925fd0 100644 --- a/clang/include/clang/AST/Expr.h +++ b/clang/include/clang/AST/Expr.h @@ -16,6 +16,7 @@ #include "clang/AST/APNumericStorage.h" #include "clang/AST/APValue.h" #include "clang/AST/ASTVector.h" +#include "clang/AST/Attr.h" #include "clang/AST/ComputeDependence.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclAccessPair.h" @@ -262,6 +263,12 @@ class Expr : public ValueStmt { SourceRange &R1, SourceRange &R2, ASTContext &Ctx) const; + /// Returns the WarnUnusedResultAttr that is declared on the callee + /// or its return type declaration, together with a NamedDecl that + /// refers to the declaration the attribute is attached to. + static std::pair + getUnusedResultAttrImpl(const Decl *Callee, QualType ReturnType); + /// isLValue - True if this expression is an "l-value" according to /// the rules of the current language. C and C++ give somewhat /// different rules for this concept, but in general, the result of @@ -3190,11 +3197,13 @@ class CallExpr : public Expr { /// type. QualType getCallReturnType(const ASTContext &Ctx) const; - /// Returns the WarnUnusedResultAttr that is either declared on the called - /// function, or its return type declaration, together with a NamedDecl that - /// refers to the declaration the attribute is attached onto. - std::pair - getUnusedResultAttr(const ASTContext &Ctx) const; + /// Returns the WarnUnusedResultAttr that is declared on the callee + /// or its return type declaration, together with a NamedDecl that + /// refers to the declaration the attribute is attached to. + std::pair + getUnusedResultAttr(const ASTContext &Ctx) const { + return getUnusedResultAttrImpl(getCalleeDecl(), getCallReturnType(Ctx)); + } /// Returns true if this call expression should warn on unused results. bool hasUnusedResultAttr(const ASTContext &Ctx) const { diff --git a/clang/include/clang/AST/ExprObjC.h b/clang/include/clang/AST/ExprObjC.h index 8210be38608a6..b9232305f036a 100644 --- a/clang/include/clang/AST/ExprObjC.h +++ b/clang/include/clang/AST/ExprObjC.h @@ -13,6 +13,7 @@ #ifndef LLVM_CLANG_AST_EXPROBJC_H #define LLVM_CLANG_AST_EXPROBJC_H +#include "clang/AST/Attr.h" #include "clang/AST/ComputeDependence.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclObjC.h" @@ -1234,6 +1235,19 @@ class ObjCMessageExpr final /// of `instancetype` (in that case it's an expression type). QualType getCallReturnType(ASTContext &Ctx) const; + /// Returns the WarnUnusedResultAttr that is declared on the callee + /// or its return type declaration, together with a NamedDecl that + /// refers to the declaration the attribute is attached to. + std::pair + getUnusedResultAttr(ASTContext &Ctx) const { + return getUnusedResultAttrImpl(getMethodDecl(), getCallReturnType(Ctx)); + } + + /// Returns true if this message send should warn on unused results. + bool hasUnusedResultAttr(ASTContext &Ctx) const { + return getUnusedResultAttr(Ctx).second != nullptr; + } + /// Source range of the receiver. SourceRange getReceiverRange() const; diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp index 2e1a9a3d9ad63..d85655b1596f4 100644 --- a/clang/lib/AST/Expr.cpp +++ b/clang/lib/AST/Expr.cpp @@ -1629,20 +1629,20 @@ QualType CallExpr::getCallReturnType(const ASTContext &Ctx) const { return FnType->getReturnType(); } -std::pair -CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const { +std::pair +Expr::getUnusedResultAttrImpl(const Decl *Callee, QualType ReturnType) { // If the callee is marked nodiscard, return that attribute - if (const Decl *D = getCalleeDecl()) - if (const auto *A = D->getAttr()) + if (Callee != nullptr) + if (const auto *A = Callee->getAttr()) return {nullptr, A}; // If the return type is a struct, union, or enum that is marked nodiscard, // then return the return type attribute. - if (const TagDecl *TD = getCallReturnType(Ctx)->getAsTagDecl()) + if (const TagDecl *TD = ReturnType->getAsTagDecl()) if (const auto *A = TD->getAttr()) return {TD, A}; - for (const auto *TD = getCallReturnType(Ctx)->getAs(); TD; + for (const auto *TD = ReturnType->getAs(); TD; TD = TD->desugar()->getAs()) if (const auto *A = TD->getDecl()->getAttr()) return {TD->getDecl(), A}; @@ -2844,12 +2844,11 @@ bool Expr::isUnusedResultAWarning(const Expr *&WarnE, SourceLocation &Loc, return true; } - if (const ObjCMethodDecl *MD = ME->getMethodDecl()) - if (MD->hasAttr()) { - WarnE = this; - Loc = getExprLoc(); - return true; - } + if (ME->hasUnusedResultAttr(Ctx)) { + WarnE = this; + Loc = getExprLoc(); + return true; + } return false; } diff --git a/clang/lib/AST/ExprObjC.cpp b/clang/lib/AST/ExprObjC.cpp index 50d3a4474c4d7..83419a11c728f 100644 --- a/clang/lib/AST/ExprObjC.cpp +++ b/clang/lib/AST/ExprObjC.cpp @@ -12,6 +12,7 @@ #include "clang/AST/ExprObjC.h" #include "clang/AST/ASTContext.h" +#include "clang/AST/Attr.h" #include "clang/AST/ComputeDependence.h" #include "clang/AST/SelectorLocationsKind.h" #include "clang/AST/Type.h" diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp index f85826aecadf3..3f89843a9081a 100644 --- a/clang/lib/Sema/SemaStmt.cpp +++ b/clang/lib/Sema/SemaStmt.cpp @@ -295,8 +295,7 @@ void DiagnoseUnused(Sema &S, const Expr *E, std::optional DiagID) { return; auto [OffendingDecl, A] = CE->getUnusedResultAttr(S.Context); - if (DiagnoseNoDiscard(S, OffendingDecl, - cast_or_null(A), Loc, R1, R2, + if (DiagnoseNoDiscard(S, OffendingDecl, A, Loc, R1, R2, /*isCtor=*/false)) return; @@ -344,13 +343,11 @@ void DiagnoseUnused(Sema &S, const Expr *E, std::optional DiagID) { S.Diag(Loc, diag::err_arc_unused_init_message) << R1; return; } - const ObjCMethodDecl *MD = ME->getMethodDecl(); - if (MD) { - if (DiagnoseNoDiscard(S, nullptr, MD->getAttr(), - Loc, R1, R2, - /*isCtor=*/false)) - return; - } + + auto [OffendingDecl, A] = ME->getUnusedResultAttr(S.Context); + if (DiagnoseNoDiscard(S, OffendingDecl, A, Loc, R1, R2, + /*isCtor=*/false)) + return; } else if (const PseudoObjectExpr *POE = dyn_cast(E)) { const Expr *Source = POE->getSyntacticForm(); // Handle the actually selected call of an OpenMP specialized call. diff --git a/clang/test/SemaCXX/warn-unused-result.cpp b/clang/test/SemaCXX/warn-unused-result.cpp index fe7d5ea79e9a7..447654eccd563 100644 --- a/clang/test/SemaCXX/warn-unused-result.cpp +++ b/clang/test/SemaCXX/warn-unused-result.cpp @@ -365,6 +365,31 @@ void id_print_name() { } } // namespace GH117975 +namespace inheritance { +// Test that [[nodiscard]] is not inherited by derived class types, +// but is inherited by member functions +struct [[nodiscard]] E { + [[nodiscard]] explicit E(int); + explicit E(const char*); + [[nodiscard]] int f(); +}; +struct F : E { + using E::E; +}; +E e(); +F f(); +void test() { + e(); // expected-warning {{ignoring return value of type 'E' declared with 'nodiscard' attribute}} + f(); // no warning: derived class type does not inherit the attribute + E(1); // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}} + E("x"); // expected-warning {{ignoring temporary of type 'E' declared with 'nodiscard' attribute}} + F(1); // no warning: inherited constructor does not inherit the attribute either + F("x"); // no warning + e().f(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}} + f().f(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}} +} +} // namespace inheritance + namespace BuildStringOnClangScope { [[clang::warn_unused_result("Discarded result")]] @@ -381,4 +406,4 @@ void doGccThings() { makeGccTrue(); // expected-warning {{ignoring return value of function declared with 'gnu::warn_unused_result' attribute}} } -} +} // namespace BuildStringOnClangScope diff --git a/clang/test/SemaObjC/attr-nodiscard.m b/clang/test/SemaObjC/attr-nodiscard.m new file mode 100644 index 0000000000000..6d04665da25ca --- /dev/null +++ b/clang/test/SemaObjC/attr-nodiscard.m @@ -0,0 +1,25 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s + +struct [[nodiscard]] expected {}; + +typedef struct expected E; + +@interface INTF +- (int) a [[nodiscard]]; ++ (int) b [[nodiscard]]; +- (struct expected) c; ++ (struct expected) d; +- (E) e; ++ (E) f; +- (void) g [[nodiscard]]; // expected-warning {{attribute 'nodiscard' cannot be applied to Objective-C method without return value}} +@end + +void foo(INTF *a) { + [a a]; // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}} + [INTF b]; // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}} + [a c]; // expected-warning {{ignoring return value of type 'expected' declared with 'nodiscard' attribute}} + [INTF d]; // expected-warning {{ignoring return value of type 'expected' declared with 'nodiscard' attribute}} + [a e]; // expected-warning {{ignoring return value of type 'expected' declared with 'nodiscard' attribute}} + [INTF f]; // expected-warning {{ignoring return value of type 'expected' declared with 'nodiscard' attribute}} + [a g]; +} diff --git a/clang/test/SemaObjCXX/attr-nodiscard.mm b/clang/test/SemaObjCXX/attr-nodiscard.mm new file mode 100644 index 0000000000000..e1eefb74d3961 --- /dev/null +++ b/clang/test/SemaObjCXX/attr-nodiscard.mm @@ -0,0 +1,26 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s + +template +struct [[nodiscard]] expected {}; + +using E = expected; + +@interface INTF +- (int) a [[nodiscard]]; ++ (int) b [[nodiscard]]; +- (expected) c; ++ (expected) d; +- (E) e; ++ (E) f; +- (void) g [[nodiscard]]; // expected-warning {{attribute 'nodiscard' cannot be applied to Objective-C method without return value}} +@end + +void foo(INTF *a) { + [a a]; // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}} + [INTF b]; // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}} + [a c]; // expected-warning {{ignoring return value of type 'expected' declared with 'nodiscard' attribute}} + [INTF d]; // expected-warning {{ignoring return value of type 'expected' declared with 'nodiscard' attribute}} + [a e]; // expected-warning {{ignoring return value of type 'expected' declared with 'nodiscard' attribute}} + [INTF f]; // expected-warning {{ignoring return value of type 'expected' declared with 'nodiscard' attribute}} + [a g]; +}