Skip to content

[clang] Diagnose [[nodiscard]] return types in Objective-C++ #142541

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jul 28, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
19 changes: 14 additions & 5 deletions clang/include/clang/AST/Expr.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "clang/AST/APNumericStorage.h"
#include "clang/AST/APValue.h"
#include "clang/AST/ASTVector.h"
#include "clang/AST/Attr.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you forward-declare WarnUnusedResultAttr instead?

#include "clang/AST/ComputeDependence.h"
#include "clang/AST/Decl.h"
#include "clang/AST/DeclAccessPair.h"
Expand Down Expand Up @@ -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<const NamedDecl *, const WarnUnusedResultAttr *>
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
Expand Down Expand Up @@ -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<const NamedDecl *, const Attr *>
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<const NamedDecl *, const WarnUnusedResultAttr *>
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 {
Expand Down
14 changes: 14 additions & 0 deletions clang/include/clang/AST/ExprObjC.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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<const NamedDecl *, const WarnUnusedResultAttr *>
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;

Expand Down
23 changes: 11 additions & 12 deletions clang/lib/AST/Expr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1629,20 +1629,20 @@ QualType CallExpr::getCallReturnType(const ASTContext &Ctx) const {
return FnType->getReturnType();
}

std::pair<const NamedDecl *, const Attr *>
CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
std::pair<const NamedDecl *, const WarnUnusedResultAttr *>
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<WarnUnusedResultAttr>())
if (Callee != nullptr)
if (const auto *A = Callee->getAttr<WarnUnusedResultAttr>())
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<WarnUnusedResultAttr>())
return {TD, A};

for (const auto *TD = getCallReturnType(Ctx)->getAs<TypedefType>(); TD;
for (const auto *TD = ReturnType->getAs<TypedefType>(); TD;
TD = TD->desugar()->getAs<TypedefType>())
if (const auto *A = TD->getDecl()->getAttr<WarnUnusedResultAttr>())
return {TD->getDecl(), A};
Expand Down Expand Up @@ -2844,12 +2844,11 @@ bool Expr::isUnusedResultAWarning(const Expr *&WarnE, SourceLocation &Loc,
return true;
}

if (const ObjCMethodDecl *MD = ME->getMethodDecl())
if (MD->hasAttr<WarnUnusedResultAttr>()) {
WarnE = this;
Loc = getExprLoc();
return true;
}
if (ME->hasUnusedResultAttr(Ctx)) {
WarnE = this;
Loc = getExprLoc();
return true;
}

return false;
}
Expand Down
1 change: 1 addition & 0 deletions clang/lib/AST/ExprObjC.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
15 changes: 6 additions & 9 deletions clang/lib/Sema/SemaStmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -295,8 +295,7 @@ void DiagnoseUnused(Sema &S, const Expr *E, std::optional<unsigned> DiagID) {
return;

auto [OffendingDecl, A] = CE->getUnusedResultAttr(S.Context);
if (DiagnoseNoDiscard(S, OffendingDecl,
cast_or_null<WarnUnusedResultAttr>(A), Loc, R1, R2,
if (DiagnoseNoDiscard(S, OffendingDecl, A, Loc, R1, R2,
/*isCtor=*/false))
return;

Expand Down Expand Up @@ -344,13 +343,11 @@ void DiagnoseUnused(Sema &S, const Expr *E, std::optional<unsigned> 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<WarnUnusedResultAttr>(),
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<PseudoObjectExpr>(E)) {
const Expr *Source = POE->getSyntacticForm();
// Handle the actually selected call of an OpenMP specialized call.
Expand Down
27 changes: 26 additions & 1 deletion clang/test/SemaCXX/warn-unused-result.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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")]]
Expand All @@ -381,4 +406,4 @@ void doGccThings() {
makeGccTrue(); // expected-warning {{ignoring return value of function declared with 'gnu::warn_unused_result' attribute}}
}

}
} // namespace BuildStringOnClangScope
25 changes: 25 additions & 0 deletions clang/test/SemaObjC/attr-nodiscard.m
Original file line number Diff line number Diff line change
@@ -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];
}
26 changes: 26 additions & 0 deletions clang/test/SemaObjCXX/attr-nodiscard.mm
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// RUN: %clang_cc1 -fsyntax-only -verify %s

template<class T>
struct [[nodiscard]] expected {};

using E = expected<int>;

@interface INTF
- (int) a [[nodiscard]];
+ (int) b [[nodiscard]];
- (expected<int>) c;
+ (expected<int>) 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<int>' declared with 'nodiscard' attribute}}
[INTF d]; // expected-warning {{ignoring return value of type 'expected<int>' declared with 'nodiscard' attribute}}
[a e]; // expected-warning {{ignoring return value of type 'expected<int>' declared with 'nodiscard' attribute}}
[INTF f]; // expected-warning {{ignoring return value of type 'expected<int>' declared with 'nodiscard' attribute}}
[a g];
}