Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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 @@ -710,6 +710,9 @@ Bug Fixes to Attribute Support
- Clang will warn if a complete type specializes a deprecated partial specialization.
(#GH44496)

- ``[[nodiscard]]`` is now respected on Objective-C and Objective-C++ methods.
(#GH141504)

Bug Fixes to C++ Support
^^^^^^^^^^^^^^^^^^^^^^^^

Expand Down
11 changes: 11 additions & 0 deletions clang/include/clang/AST/ExprObjC.h
Original file line number Diff line number Diff line change
Expand Up @@ -1236,6 +1236,17 @@ class ObjCMessageExpr final
/// of `instancetype` (in that case it's an expression type).
QualType getCallReturnType(ASTContext &Ctx) const;

/// Returns the WarnUnusedResultAttr that is either declared on the called
/// method, 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(ASTContext &Ctx) const;

/// 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
11 changes: 5 additions & 6 deletions clang/lib/AST/Expr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2869,12 +2869,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
21 changes: 21 additions & 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 Expand Up @@ -272,6 +273,26 @@ QualType ObjCMessageExpr::getCallReturnType(ASTContext &Ctx) const {
return Ctx.getReferenceQualifiedType(this);
}

std::pair<const NamedDecl *, const Attr *>
ObjCMessageExpr::getUnusedResultAttr(ASTContext &Ctx) const {
// If the callee is marked nodiscard, return that attribute
if (const ObjCMethodDecl *MD = getMethodDecl())
if (const auto *A = MD->getAttr<WarnUnusedResultAttr>())
return {nullptr, A};
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this be {MD, A}?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is this way in CallExpr::getUnusedResultAttribute too. Probably the NamedDecl is supposed to be set only when it is a type declaration.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you better explain that? What happens if this DOES set the MD correctly? Do we have a test that shows the behavior change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, if this is changed to return {MD, A} then SemaObjC/method-warn-unused-attribute.m fails with

ignoring return value of type 'fee' declared with 'warn_unused_result' attribute

instead of the expected

ignoring return value of function declared with 'warn_unused_result' attribute

Copy link
Member

Choose a reason for hiding this comment

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

For more context, see #112521, but tl;dr all of this is because we want to issue an unused result diagnostic in two different situations: either because the function is declared nodiscard or because the return type is declared nodiscard. If both are nodiscard, then we prefer printing the function as the cause instead of the type.

Copy link
Member

@Sirraide Sirraide Jun 9, 2025

Choose a reason for hiding this comment

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

Specifically,

  • {null, Attribute} means ‘the function is nodiscard’;
  • {TD, Attribute} means ‘the type is nodiscard, please include TD in the diagnostic;
  • {null, null} means ‘neither is nodiscard’.

I thought there was a comment about this somewhere, but if not should probably add one

Copy link
Member

@Sirraide Sirraide Jun 9, 2025

Choose a reason for hiding this comment

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

The attribute I believe is returned so we can get the diagnostic message from it but don’t quote me on that.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why can't this return the MethodDecl here?


// 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 auto *A = TD->getAttr<WarnUnusedResultAttr>())
return {TD, A};

for (const auto *TD = getCallReturnType(Ctx)->getAs<TypedefType>(); TD;
TD = TD->desugar()->getAs<TypedefType>())
if (const auto *A = TD->getDecl()->getAttr<WarnUnusedResultAttr>())
return {TD->getDecl(), A};
return {nullptr, nullptr};
}

SourceRange ObjCMessageExpr::getReceiverRange() const {
switch (getReceiverKind()) {
case Instance:
Expand Down
13 changes: 6 additions & 7 deletions clang/lib/Sema/SemaStmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -344,13 +344,12 @@ 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,
cast_or_null<WarnUnusedResultAttr>(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
25 changes: 25 additions & 0 deletions clang/test/SemaCXX/warn-unused-result.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -364,3 +364,28 @@ void id_print_name() {
((int(*)())f)();
}
} // 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
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 expected E;

@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];
}
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];
}
Loading