Skip to content

Commit 2f23767

Browse files
authored
[Clang] [Sema] Enable nodiscard warnings for function pointers (#154250)
A call through a function pointer has no associated FunctionDecl, but it still might have a nodiscard return type. Ensure there is a codepath to emit the nodiscard warning in this case. Fixes #142453
1 parent ea634fe commit 2f23767

File tree

6 files changed

+136
-21
lines changed

6 files changed

+136
-21
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -216,8 +216,8 @@ Bug Fixes to Compiler Builtins
216216
Bug Fixes to Attribute Support
217217
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
218218

219-
- ``[[nodiscard]]`` is now respected on Objective-C and Objective-C++ methods.
220-
(#GH141504)
219+
- ``[[nodiscard]]`` is now respected on Objective-C and Objective-C++ methods
220+
(#GH141504) and on types returned from indirect calls (#GH142453).
221221
- Fixes some late parsed attributes, when applied to function definitions, not being parsed
222222
in function try blocks, and some situations where parsing of the function body
223223
is skipped, such as error recovery and code completion. (#GH153551)

clang/lib/AST/Expr.cpp

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2776,23 +2776,22 @@ bool Expr::isUnusedResultAWarning(const Expr *&WarnE, SourceLocation &Loc,
27762776
case UserDefinedLiteralClass: {
27772777
// If this is a direct call, get the callee.
27782778
const CallExpr *CE = cast<CallExpr>(this);
2779-
if (const Decl *FD = CE->getCalleeDecl()) {
2780-
// If the callee has attribute pure, const, or warn_unused_result, warn
2781-
// about it. void foo() { strlen("bar"); } should warn.
2782-
//
2783-
// Note: If new cases are added here, DiagnoseUnusedExprResult should be
2784-
// updated to match for QoI.
2785-
if (CE->hasUnusedResultAttr(Ctx) ||
2786-
FD->hasAttr<PureAttr>() || FD->hasAttr<ConstAttr>()) {
2787-
WarnE = this;
2788-
Loc = CE->getCallee()->getBeginLoc();
2789-
R1 = CE->getCallee()->getSourceRange();
2790-
2791-
if (unsigned NumArgs = CE->getNumArgs())
2792-
R2 = SourceRange(CE->getArg(0)->getBeginLoc(),
2793-
CE->getArg(NumArgs - 1)->getEndLoc());
2794-
return true;
2795-
}
2779+
// If the callee has attribute pure, const, or warn_unused_result, warn
2780+
// about it. void foo() { strlen("bar"); } should warn.
2781+
// Note: If new cases are added here, DiagnoseUnusedExprResult should be
2782+
// updated to match for QoI.
2783+
const Decl *FD = CE->getCalleeDecl();
2784+
bool PureOrConst =
2785+
FD && (FD->hasAttr<PureAttr>() || FD->hasAttr<ConstAttr>());
2786+
if (CE->hasUnusedResultAttr(Ctx) || PureOrConst) {
2787+
WarnE = this;
2788+
Loc = getBeginLoc();
2789+
R1 = getSourceRange();
2790+
2791+
if (unsigned NumArgs = CE->getNumArgs())
2792+
R2 = SourceRange(CE->getArg(0)->getBeginLoc(),
2793+
CE->getArg(NumArgs - 1)->getEndLoc());
2794+
return true;
27962795
}
27972796
return false;
27982797
}

clang/test/Sema/c2x-nodiscard.c

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,10 @@ void f2(void) {
4141
(void)get_s3();
4242
(void)get_i();
4343
(void)get_e();
44+
45+
One; // expected-warning {{expression result unused}}
46+
(enum E2)(0); // expected-warning {{expression result unused}}
47+
(struct S4){1}; // expected-warning {{expression result unused}}
4448
}
4549

4650
struct [[nodiscard]] error_info{
@@ -60,3 +64,16 @@ void GH104391() {
6064
#define M (unsigned int) f3()
6165
M; // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
6266
}
67+
68+
[[nodiscard]] typedef int NoDInt; // expected-warning {{'[[nodiscard]]' attribute ignored when applied to a typedef}}
69+
typedef __attribute__((warn_unused)) int WUInt; // expected-warning {{'warn_unused' attribute only applies to structs, unions, and classes}}
70+
typedef __attribute__((warn_unused_result)) int WURInt;
71+
NoDInt get_nodint();
72+
WUInt get_wuint();
73+
WURInt get_wurint();
74+
75+
void f4(void) {
76+
get_nodint(); // no warning because attribute is ignored
77+
get_wuint(); // no warning because attribute is ignored
78+
get_wurint(); // expected-warning {{ignoring return value of type 'WURInt' declared with 'warn_unused_result' attribute}}
79+
}

clang/test/SemaCXX/warn-unused-result.cpp

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,3 +407,88 @@ void doGccThings() {
407407
}
408408

409409
} // namespace BuildStringOnClangScope
410+
411+
namespace candiscard {
412+
413+
struct [[nodiscard]] NoDiscard {
414+
[[nodiscard]] NoDiscard(int);
415+
NoDiscard(const char *);
416+
};
417+
418+
struct [[gnu::warn_unused]] WarnUnused {
419+
[[gnu::warn_unused]] WarnUnused(int); // expected-warning {{'gnu::warn_unused' attribute only applies to structs, unions, and classes}}
420+
WarnUnused(const char*);
421+
};
422+
423+
struct [[gnu::warn_unused_result]] WarnUnusedResult {
424+
[[gnu::warn_unused_result]] WarnUnusedResult(int);
425+
WarnUnusedResult(const char*);
426+
};
427+
428+
NoDiscard return_nodiscard();
429+
WarnUnused return_warnunused();
430+
WarnUnusedResult return_warnunusedresult();
431+
432+
NoDiscard (*p_return_nodiscard)();
433+
WarnUnused (*p_return_warnunused)();
434+
WarnUnusedResult (*p_return_warnunusedresult)();
435+
436+
NoDiscard (*(*pp_return_nodiscard)())();
437+
WarnUnused (*(*pp_return_warnunused)())();
438+
WarnUnusedResult (*(*pp_return_warnunusedresult)())();
439+
440+
template <class T> T from_a_template();
441+
442+
void test() {
443+
// Unused but named variables
444+
NoDiscard unused_variable1(1); // no warning
445+
NoDiscard unused_variable2(""); // no warning
446+
WarnUnused unused_variable3(1); // no warning
447+
WarnUnused unused_variable4(""); // no warning
448+
WarnUnusedResult unused_variable5(1); // no warning
449+
WarnUnusedResult unused_variable6(""); // no warning
450+
451+
// Constructor return values
452+
NoDiscard(1); // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
453+
NoDiscard(""); // expected-warning {{ignoring temporary of type 'NoDiscard' declared with 'nodiscard' attribute}}
454+
WarnUnused(1); // expected-warning {{expression result unused}}
455+
WarnUnused(""); // expected-warning {{expression result unused}}
456+
WarnUnusedResult(1); // expected-warning {{ignoring temporary created by a constructor declared with 'gnu::warn_unused_result' attribute}}
457+
WarnUnusedResult(""); // expected-warning {{ignoring temporary of type 'WarnUnusedResult' declared with 'gnu::warn_unused_result' attribute}}
458+
459+
NoDiscard{1}; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
460+
NoDiscard{""}; // expected-warning {{ignoring temporary of type 'NoDiscard' declared with 'nodiscard' attribute}}
461+
WarnUnused{1}; // expected-warning {{expression result unused}}
462+
WarnUnused{""}; // expected-warning {{expression result unused}}
463+
WarnUnusedResult{1}; // expected-warning {{ignoring temporary created by a constructor declared with 'gnu::warn_unused_result' attribute}}
464+
WarnUnusedResult{""}; // expected-warning {{ignoring temporary of type 'WarnUnusedResult' declared with 'gnu::warn_unused_result' attribute}}
465+
466+
static_cast<NoDiscard>(1); // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
467+
static_cast<NoDiscard>(""); // expected-warning {{ignoring temporary of type 'NoDiscard' declared with 'nodiscard' attribute}}
468+
static_cast<WarnUnused>(1); // expected-warning {{expression result unused}}
469+
static_cast<WarnUnused>(""); // expected-warning {{expression result unused}}
470+
static_cast<WarnUnusedResult>(1); // expected-warning {{ignoring temporary created by a constructor declared with 'gnu::warn_unused_result' attribute}}
471+
static_cast<WarnUnusedResult>(""); // expected-warning {{ignoring temporary of type 'WarnUnusedResult' declared with 'gnu::warn_unused_result' attribute}}
472+
473+
// Function return values
474+
return_nodiscard(); // expected-warning {{ignoring return value of type 'NoDiscard' declared with 'nodiscard' attribute}}
475+
return_warnunused(); // no warning
476+
return_warnunusedresult(); // expected-warning {{ignoring return value of type 'WarnUnusedResult' declared with 'gnu::warn_unused_result' attribute}}
477+
478+
// Function pointer return values
479+
p_return_nodiscard(); // expected-warning {{ignoring return value of type 'NoDiscard' declared with 'nodiscard' attribute}}
480+
p_return_warnunused(); // no warning
481+
p_return_warnunusedresult(); // expected-warning {{ignoring return value of type 'WarnUnusedResult' declared with 'gnu::warn_unused_result' attribute}}
482+
483+
// Function pointer expression return values
484+
pp_return_nodiscard()(); // expected-warning {{ignoring return value of type 'NoDiscard' declared with 'nodiscard' attribute}}
485+
pp_return_warnunused()(); // no warning
486+
pp_return_warnunusedresult()(); // expected-warning {{ignoring return value of type 'WarnUnusedResult' declared with 'gnu::warn_unused_result' attribute}}
487+
488+
// From a template
489+
from_a_template<NoDiscard>(); // expected-warning {{ignoring return value of type 'NoDiscard' declared with 'nodiscard' attribute}}
490+
from_a_template<WarnUnused>(); // no warning
491+
from_a_template<WarnUnusedResult>(); // expected-warning {{ignoring return value of type 'WarnUnusedResult' declared with 'gnu::warn_unused_result' attribute}}
492+
}
493+
494+
} // namespace candiscard

clang/test/SemaObjC/attr-nodiscard.m

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44

55
typedef struct expected E;
66

7+
[[nodiscard]] typedef int NI; // expected-warning {{'[[nodiscard]]' attribute ignored when applied to a typedef}}
8+
typedef __attribute__((warn_unused_result)) int WUR;
9+
710
@interface INTF
811
- (int) a [[nodiscard]];
912
+ (int) b [[nodiscard]];
@@ -12,6 +15,8 @@ + (struct expected) d;
1215
- (E) e;
1316
+ (E) f;
1417
- (void) g [[nodiscard]]; // expected-warning {{attribute 'nodiscard' cannot be applied to Objective-C method without return value}}
18+
- (NI) h;
19+
- (WUR) i;
1520
@end
1621

1722
void foo(INTF *a) {
@@ -21,5 +26,7 @@ void foo(INTF *a) {
2126
[INTF d]; // expected-warning {{ignoring return value of type 'expected' declared with 'nodiscard' attribute}}
2227
[a e]; // expected-warning {{ignoring return value of type 'expected' declared with 'nodiscard' attribute}}
2328
[INTF f]; // expected-warning {{ignoring return value of type 'expected' declared with 'nodiscard' attribute}}
24-
[a g];
29+
[a g]; // no warning because g returns void
30+
[a h]; // no warning because attribute is ignored when applied to a typedef
31+
[a i]; // expected-warning {{ignoring return value of type 'WUR' declared with 'warn_unused_result' attribute}}
2532
}

clang/test/SemaObjCXX/attr-nodiscard.mm

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@
55

66
using E = expected<int>;
77

8+
using NI [[nodiscard]] = int; // expected-warning {{'[[nodiscard]]' attribute ignored when applied to a typedef}}
9+
using WURI [[clang::warn_unused_result]] = int;
10+
811
@interface INTF
912
- (int) a [[nodiscard]];
1013
+ (int) b [[nodiscard]];
@@ -13,6 +16,8 @@ + (int) b [[nodiscard]];
1316
- (E) e;
1417
+ (E) f;
1518
- (void) g [[nodiscard]]; // expected-warning {{attribute 'nodiscard' cannot be applied to Objective-C method without return value}}
19+
- (NI) h;
20+
- (WURI) i;
1621
@end
1722

1823
void foo(INTF *a) {
@@ -22,5 +27,7 @@ void foo(INTF *a) {
2227
[INTF d]; // expected-warning {{ignoring return value of type 'expected<int>' declared with 'nodiscard' attribute}}
2328
[a e]; // expected-warning {{ignoring return value of type 'expected<int>' declared with 'nodiscard' attribute}}
2429
[INTF f]; // expected-warning {{ignoring return value of type 'expected<int>' declared with 'nodiscard' attribute}}
25-
[a g];
30+
[a g]; // no warning because g returns void
31+
[a h]; // no warning because attribute is ignored
32+
[a i]; // expected-warning {{ignoring return value of type 'WURI' declared with 'clang::warn_unused_result' attribute}}
2633
}

0 commit comments

Comments
 (0)