Skip to content

Commit 05e44dc

Browse files
committed
[clang] Generate note on declaration for nodiscard-related attributes
1 parent 96c3207 commit 05e44dc

File tree

12 files changed

+98
-71
lines changed

12 files changed

+98
-71
lines changed

clang/include/clang/AST/Expr.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3182,11 +3182,12 @@ class CallExpr : public Expr {
31823182

31833183
/// Returns the WarnUnusedResultAttr that is either declared on the called
31843184
/// function, or its return type declaration.
3185-
const Attr *getUnusedResultAttr(const ASTContext &Ctx) const;
3185+
std::pair<const NamedDecl *, const Attr *>
3186+
getUnusedResultAttr(const ASTContext &Ctx) const;
31863187

31873188
/// Returns true if this call expression should warn on unused results.
31883189
bool hasUnusedResultAttr(const ASTContext &Ctx) const {
3189-
return getUnusedResultAttr(Ctx) != nullptr;
3190+
return getUnusedResultAttr(Ctx).second != nullptr;
31903191
}
31913192

31923193
SourceLocation getRParenLoc() const { return RParenLoc; }

clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9290,6 +9290,8 @@ def warn_unused_result_typedef_unsupported_spelling : Warning<
92909290
def warn_unused_volatile : Warning<
92919291
"expression result unused; assign into a variable to force a volatile load">,
92929292
InGroup<DiagGroup<"unused-volatile-lvalue">>;
9293+
def note_nodiscard_specified_here : Note<
9294+
"%0 has been explicitly marked %1 here">;
92939295

92949296
def ext_cxx14_attr : Extension<
92959297
"use of the %0 attribute is a C++14 extension">, InGroup<CXX14Attrs>;

clang/lib/AST/Expr.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1616,22 +1616,24 @@ QualType CallExpr::getCallReturnType(const ASTContext &Ctx) const {
16161616
return FnType->getReturnType();
16171617
}
16181618

1619-
const Attr *CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
1619+
std::pair<const NamedDecl *, const Attr *>
1620+
CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
16201621
// If the return type is a struct, union, or enum that is marked nodiscard,
16211622
// then return the return type attribute.
16221623
if (const TagDecl *TD = getCallReturnType(Ctx)->getAsTagDecl())
16231624
if (const auto *A = TD->getAttr<WarnUnusedResultAttr>())
1624-
return A;
1625+
return {TD, A};
16251626

16261627
for (const auto *TD = getCallReturnType(Ctx)->getAs<TypedefType>(); TD;
16271628
TD = TD->desugar()->getAs<TypedefType>())
16281629
if (const auto *A = TD->getDecl()->getAttr<WarnUnusedResultAttr>())
1629-
return A;
1630+
return {TD->getDecl(), A};
16301631

16311632
// Otherwise, see if the callee is marked nodiscard and return that attribute
16321633
// instead.
16331634
const Decl *D = getCalleeDecl();
1634-
return D ? D->getAttr<WarnUnusedResultAttr>() : nullptr;
1635+
return {dyn_cast<NamedDecl>(D),
1636+
D ? D->getAttr<WarnUnusedResultAttr>() : nullptr};
16351637
}
16361638

16371639
SourceLocation CallExpr::getBeginLoc() const {

clang/lib/Sema/SemaStmt.cpp

Lines changed: 42 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include "clang/AST/ASTContext.h"
1515
#include "clang/AST/ASTDiagnostic.h"
1616
#include "clang/AST/ASTLambda.h"
17+
#include "clang/AST/Attrs.inc"
1718
#include "clang/AST/CXXInheritance.h"
1819
#include "clang/AST/CharUnits.h"
1920
#include "clang/AST/DeclObjC.h"
@@ -204,23 +205,29 @@ static bool DiagnoseUnusedComparison(Sema &S, const Expr *E) {
204205
return true;
205206
}
206207

207-
static bool DiagnoseNoDiscard(Sema &S, const WarnUnusedResultAttr *A,
208-
SourceLocation Loc, SourceRange R1,
209-
SourceRange R2, bool IsCtor) {
208+
static bool DiagnoseNoDiscard(Sema &S, const NamedDecl *OffendingDecl,
209+
const WarnUnusedResultAttr *A, SourceLocation Loc,
210+
SourceRange R1, SourceRange R2, bool IsCtor) {
210211
if (!A)
211212
return false;
212213
StringRef Msg = A->getMessage();
213214

215+
bool result;
214216
if (Msg.empty()) {
215217
if (IsCtor)
216-
return S.Diag(Loc, diag::warn_unused_constructor) << A << R1 << R2;
217-
return S.Diag(Loc, diag::warn_unused_result) << A << R1 << R2;
218-
}
218+
result = S.Diag(Loc, diag::warn_unused_constructor) << A << R1 << R2;
219+
else
220+
result = S.Diag(Loc, diag::warn_unused_result) << A << R1 << R2;
221+
} else if (IsCtor)
222+
result = S.Diag(Loc, diag::warn_unused_constructor_msg)
223+
<< A << Msg << R1 << R2;
224+
else
225+
result = S.Diag(Loc, diag::warn_unused_result_msg) << A << Msg << R1 << R2;
219226

220-
if (IsCtor)
221-
return S.Diag(Loc, diag::warn_unused_constructor_msg) << A << Msg << R1
222-
<< R2;
223-
return S.Diag(Loc, diag::warn_unused_result_msg) << A << Msg << R1 << R2;
227+
if (OffendingDecl)
228+
S.Diag(A->getLocation(), diag::note_nodiscard_specified_here)
229+
<< OffendingDecl << A->getSpelling();
230+
return result;
224231
}
225232

226233
void Sema::DiagnoseUnusedExprResult(const Stmt *S, unsigned DiagID) {
@@ -290,9 +297,12 @@ void Sema::DiagnoseUnusedExprResult(const Stmt *S, unsigned DiagID) {
290297
if (E->getType()->isVoidType())
291298
return;
292299

293-
if (DiagnoseNoDiscard(*this, cast_or_null<WarnUnusedResultAttr>(
294-
CE->getUnusedResultAttr(Context)),
295-
Loc, R1, R2, /*isCtor=*/false))
300+
const NamedDecl *OffendingDecl;
301+
const Attr *A;
302+
std::tie(OffendingDecl, A) = CE->getUnusedResultAttr(Context);
303+
if (DiagnoseNoDiscard(*this, OffendingDecl,
304+
cast_or_null<WarnUnusedResultAttr>(A), Loc, R1, R2,
305+
/*isCtor=*/false))
296306
return;
297307

298308
// If the callee has attribute pure, const, or warn_unused_result, warn with
@@ -302,27 +312,38 @@ void Sema::DiagnoseUnusedExprResult(const Stmt *S, unsigned DiagID) {
302312
if (const Decl *FD = CE->getCalleeDecl()) {
303313
if (ShouldSuppress)
304314
return;
305-
if (FD->hasAttr<PureAttr>()) {
315+
if (const auto *A = FD->getAttr<PureAttr>()) {
306316
Diag(Loc, diag::warn_unused_call) << R1 << R2 << "pure";
317+
if (OffendingDecl && !OffendingDecl->getIdentifier()->getBuiltinID())
318+
Diag(A->getLocation(), diag::note_nodiscard_specified_here)
319+
<< OffendingDecl << "pure";
307320
return;
308321
}
309-
if (FD->hasAttr<ConstAttr>()) {
322+
if (const auto *A = FD->getAttr<ConstAttr>()) {
310323
Diag(Loc, diag::warn_unused_call) << R1 << R2 << "const";
324+
if (OffendingDecl && !OffendingDecl->getIdentifier()->getBuiltinID())
325+
Diag(A->getLocation(), diag::note_nodiscard_specified_here)
326+
<< OffendingDecl << "const";
311327
return;
312328
}
313329
}
314330
} else if (const auto *CE = dyn_cast<CXXConstructExpr>(E)) {
315331
if (const CXXConstructorDecl *Ctor = CE->getConstructor()) {
332+
const NamedDecl *OffendingDecl = Ctor;
316333
const auto *A = Ctor->getAttr<WarnUnusedResultAttr>();
317-
A = A ? A : Ctor->getParent()->getAttr<WarnUnusedResultAttr>();
318-
if (DiagnoseNoDiscard(*this, A, Loc, R1, R2, /*isCtor=*/true))
334+
if (!A) {
335+
OffendingDecl = Ctor->getParent();
336+
A = OffendingDecl->getAttr<WarnUnusedResultAttr>();
337+
}
338+
if (DiagnoseNoDiscard(*this, OffendingDecl, A, Loc, R1, R2,
339+
/*isCtor=*/true))
319340
return;
320341
}
321342
} else if (const auto *ILE = dyn_cast<InitListExpr>(E)) {
322343
if (const TagDecl *TD = ILE->getType()->getAsTagDecl()) {
323344

324-
if (DiagnoseNoDiscard(*this, TD->getAttr<WarnUnusedResultAttr>(), Loc, R1,
325-
R2, /*isCtor=*/false))
345+
if (DiagnoseNoDiscard(*this, TD, TD->getAttr<WarnUnusedResultAttr>(), Loc,
346+
R1, R2, /*isCtor=*/false))
326347
return;
327348
}
328349
} else if (ShouldSuppress)
@@ -336,8 +357,8 @@ void Sema::DiagnoseUnusedExprResult(const Stmt *S, unsigned DiagID) {
336357
}
337358
const ObjCMethodDecl *MD = ME->getMethodDecl();
338359
if (MD) {
339-
if (DiagnoseNoDiscard(*this, MD->getAttr<WarnUnusedResultAttr>(), Loc, R1,
340-
R2, /*isCtor=*/false))
360+
if (DiagnoseNoDiscard(*this, MD, MD->getAttr<WarnUnusedResultAttr>(), Loc,
361+
R1, R2, /*isCtor=*/false))
341362
return;
342363
}
343364
} else if (const PseudoObjectExpr *POE = dyn_cast<PseudoObjectExpr>(E)) {

clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,18 @@
22
// RUN: %clang_cc1 -fsyntax-only -std=c++17 -verify=expected,cxx11-17,since-cxx17 -pedantic %s
33
// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify=expected,since-cxx17 -pedantic %s
44

5-
struct [[nodiscard]] S {};
5+
struct [[nodiscard]] S {}; // expected-note 4 {{'S' has been explicitly marked nodiscard here}}
66
// cxx11-warning@-1 {{use of the 'nodiscard' attribute is a C++17 extension}}
77
S get_s();
88
S& get_s_ref();
99

10-
enum [[nodiscard]] E {};
10+
enum [[nodiscard]] E {}; // expected-note 2 {{'E' has been explicitly marked nodiscard here}}
1111
// cxx11-warning@-1 {{use of the 'nodiscard' attribute is a C++17 extension}}
1212
E get_e();
1313

14-
[[nodiscard]] int get_i();
14+
[[nodiscard]] int get_i(); // expected-note {{'get_i' has been explicitly marked nodiscard here}}
1515
// cxx11-warning@-1 {{use of the 'nodiscard' attribute is a C++17 extension}}
16-
[[nodiscard]] volatile int &get_vi();
16+
[[nodiscard]] volatile int &get_vi(); // expected-note {{'get_vi' has been explicitly marked nodiscard here}}
1717
// cxx11-warning@-1 {{use of the 'nodiscard' attribute is a C++17 extension}}
1818

1919
void f() {
@@ -32,6 +32,7 @@ void f() {
3232

3333
[[nodiscard]] volatile char &(*fp)(); // expected-warning {{'nodiscard' attribute only applies to functions, classes, or enumerations}}
3434
// cxx11-warning@-1 {{use of the 'nodiscard' attribute is a C++17 extension}}
35+
// expected-note@-2 {{'fp' has been explicitly marked nodiscard here}}
3536
void g() {
3637
fp(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
3738

@@ -67,20 +68,20 @@ void f() {
6768
}
6869
} // namespace PR31526
6970

70-
struct [[nodiscard("reason")]] ReasonStruct {};
71+
struct [[nodiscard("reason")]] ReasonStruct {}; // expected-note {{'ReasonStruct' has been explicitly marked nodiscard here}}
7172
// cxx11-17-warning@-1 {{use of the 'nodiscard' attribute is a C++20 extension}}
7273
struct LaterReason;
73-
struct [[nodiscard("later reason")]] LaterReason {};
74+
struct [[nodiscard("later reason")]] LaterReason {}; // expected-note {{'LaterReason' has been explicitly marked nodiscard here}}
7475
// cxx11-17-warning@-1 {{use of the 'nodiscard' attribute is a C++20 extension}}
7576

7677
ReasonStruct get_reason();
7778
LaterReason get_later_reason();
78-
[[nodiscard("another reason")]] int another_reason();
79+
[[nodiscard("another reason")]] int another_reason(); // expected-note {{'another_reason' has been explicitly marked nodiscard here}}
7980
// cxx11-17-warning@-1 {{use of the 'nodiscard' attribute is a C++20 extension}}
8081

8182
[[nodiscard("conflicting reason")]] int conflicting_reason();
8283
// cxx11-17-warning@-1 {{use of the 'nodiscard' attribute is a C++20 extension}}
83-
[[nodiscard("special reason")]] int conflicting_reason();
84+
[[nodiscard("special reason")]] int conflicting_reason(); // expected-note {{'conflicting_reason' has been explicitly marked nodiscard here}}
8485
// cxx11-17-warning@-1 {{use of the 'nodiscard' attribute is a C++20 extension}}
8586

8687
void cxx20_use() {
@@ -91,23 +92,23 @@ void cxx20_use() {
9192
}
9293

9394
namespace p1771 {
94-
struct[[nodiscard("Don't throw me away!")]] ConvertTo{};
95+
struct[[nodiscard("Don't throw me away!")]] ConvertTo{}; // expected-note 3 {{'ConvertTo' has been explicitly marked nodiscard here}}
9596
// cxx11-17-warning@-1 {{use of the 'nodiscard' attribute is a C++20 extension}}
9697
struct S {
97-
[[nodiscard]] S();
98+
[[nodiscard]] S(); // expected-note {{'S' has been explicitly marked nodiscard here}}
9899
// cxx11-warning@-1 {{use of the 'nodiscard' attribute is a C++17 extension}}
99-
[[nodiscard("Don't let that S-Char go!")]] S(char);
100+
[[nodiscard("Don't let that S-Char go!")]] S(char); // expected-note 2 {{'S' has been explicitly marked nodiscard here}}
100101
// cxx11-17-warning@-1 {{use of the 'nodiscard' attribute is a C++20 extension}}
101102
S(int);
102103
[[gnu::warn_unused_result]] S(double);
103104
operator ConvertTo();
104-
[[nodiscard]] operator int();
105+
[[nodiscard]] operator int(); // expected-note 2 {{'operator int' has been explicitly marked nodiscard here}}
105106
// cxx11-warning@-1 {{use of the 'nodiscard' attribute is a C++17 extension}}
106-
[[nodiscard("Don't throw away as a double")]] operator double();
107+
[[nodiscard("Don't throw away as a double")]] operator double(); // expected-note {{'operator double' has been explicitly marked nodiscard here}}
107108
// cxx11-17-warning@-1 {{use of the 'nodiscard' attribute is a C++20 extension}}
108109
};
109110

110-
struct[[nodiscard("Don't throw me away either!")]] Y{};
111+
struct[[nodiscard("Don't throw me away either!")]] Y{}; // expected-note {{'Y' has been explicitly marked nodiscard here}}
111112
// cxx11-17-warning@-1 {{use of the 'nodiscard' attribute is a C++20 extension}}
112113

113114
void usage() {

clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p3.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// RUN: %clang_cc1 -std=c++1z -verify %s
22

33
namespace std_example {
4-
struct [[nodiscard]] error_info{
4+
struct [[nodiscard]] error_info{ // expected-note {{'error_info' has been explicitly marked nodiscard here}}
55
// ...
66
};
77

clang/test/OpenMP/declare_variant_messages.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ int score_and_cond_const();
133133
template <int C>
134134
int score_and_cond_const_inst();
135135

136-
__attribute__((pure)) int pure() { return 0; }
136+
__attribute__((pure)) int pure() { return 0; } // expected-note {{'pure' has been explicitly marked pure here}}
137137

138138
#pragma omp declare variant(pure) match(user = {condition(1)}) // expected-warning {{ignoring return value of function declared with pure attribute}}
139139
int unused_warning_after_specialization() { return foo(); }

clang/test/Sema/c2x-nodiscard.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ struct [[nodiscard]] S1 { // ok
99
struct [[nodiscard, nodiscard]] S2 { // ok
1010
int i;
1111
};
12-
struct [[nodiscard("Wrong")]] S3 {
12+
struct [[nodiscard("Wrong")]] S3 { // expected-note {{'S3' has been explicitly marked nodiscard here}}
1313
int i;
1414
};
1515

@@ -20,15 +20,15 @@ enum [[nodiscard]] E1 { One };
2020

2121
[[nodiscard]] int i; // expected-warning {{'nodiscard' attribute only applies to Objective-C methods, enums, structs, unions, classes, functions, function pointers, and typedefs}}
2222

23-
struct [[nodiscard]] S4 {
23+
struct [[nodiscard]] S4 { // expected-note {{'S4' has been explicitly marked nodiscard here}}
2424
int i;
2525
};
2626
struct S4 get_s(void);
2727

28-
enum [[nodiscard]] E2 { Two };
28+
enum [[nodiscard]] E2 { Two }; // expected-note {{'E2' has been explicitly marked nodiscard here}}
2929
enum E2 get_e(void);
3030

31-
[[nodiscard]] int get_i(void);
31+
[[nodiscard]] int get_i(void); // expected-note {{'get_i' has been explicitly marked nodiscard here}}
3232

3333
void f2(void) {
3434
get_s(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
@@ -43,7 +43,7 @@ void f2(void) {
4343
(void)get_e();
4444
}
4545

46-
struct [[nodiscard]] error_info{
46+
struct [[nodiscard]] error_info{ // expected-note {{'error_info' has been explicitly marked nodiscard here}}
4747
int i;
4848
};
4949

@@ -54,7 +54,7 @@ void test_missiles(void) {
5454
launch_missiles();
5555
}
5656

57-
[[nodiscard]] int f3();
57+
[[nodiscard]] int f3(); // expected-note {{'f3' has been explicitly marked nodiscard here}}
5858

5959
void GH104391() {
6060
#define M (unsigned int) f3()

clang/test/Sema/unused-expr.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -73,15 +73,15 @@ void t4(int a) {
7373
for (;;b < 1) {} // expected-warning{{relational comparison result unused}}
7474
}
7575

76-
int t5f(void) __attribute__((warn_unused_result));
76+
int t5f(void) __attribute__((warn_unused_result)); // expected-note {{'t5f' has been explicitly marked warn_unused_result here}}
7777
void t5(void) {
7878
t5f(); // expected-warning {{ignoring return value of function declared with 'warn_unused_result' attribute}}
7979
}
8080

8181

82-
int fn1(void) __attribute__ ((warn_unused_result));
83-
int fn2() __attribute__ ((pure));
84-
int fn3() __attribute__ ((__const));
82+
int fn1(void) __attribute__ ((warn_unused_result)); // expected-note 3 {{'fn1' has been explicitly marked warn_unused_result here}}
83+
int fn2() __attribute__ ((pure)); // expected-note 2 {{'fn2' has been explicitly marked pure here}}
84+
int fn3() __attribute__ ((__const)); // expected-note {{'fn3' has been explicitly marked const here}}
8585
int t6(void) {
8686
if (fn1() < 0 || fn2(2,1) < 0 || fn3(2) < 0) // no warnings
8787
return -1;
@@ -97,7 +97,7 @@ int t6(void) {
9797
int t7 __attribute__ ((warn_unused_result)); // expected-warning {{'warn_unused_result' attribute only applies to Objective-C methods, enums, structs, unions, classes, functions, function pointers, and typedefs}}
9898

9999
// PR4010
100-
int (*fn4)(void) __attribute__ ((warn_unused_result));
100+
int (*fn4)(void) __attribute__ ((warn_unused_result)); // expected-note {{'fn4' has been explicitly marked warn_unused_result here}}
101101
void t8(void) {
102102
fn4(); // expected-warning {{ignoring return value of function declared with 'warn_unused_result' attribute}}
103103
}

clang/test/SemaCXX/coroutines.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1497,7 +1497,7 @@ class awaitable_unused_warn {
14971497
using handle_type = std::coroutine_handle<>;
14981498
constexpr bool await_ready() noexcept { return false; }
14991499
void await_suspend(handle_type) noexcept {}
1500-
[[nodiscard]] int await_resume() noexcept { return 1; }
1500+
[[nodiscard]] int await_resume() noexcept { return 1; } // expected-note 2 {{'await_resume' has been explicitly marked nodiscard here}}
15011501
};
15021502

15031503
template <class Await>

0 commit comments

Comments
 (0)