Skip to content

Commit b9911d5

Browse files
committed
[clang] Generate note on declaration for nodiscard-related attributes
1 parent 7e72e5b commit b9911d5

File tree

14 files changed

+116
-80
lines changed

14 files changed

+116
-80
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 Decl *, 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: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5939,8 +5939,8 @@ def warn_unavailable_fwdclass_message : Warning<
59395939
"%0 may be unavailable because the receiver type is unknown">,
59405940
InGroup<UnavailableDeclarations>;
59415941
def note_availability_specified_here : Note<
5942-
"%0 has been explicitly marked "
5943-
"%select{unavailable|deleted|deprecated}1 here">;
5942+
"%select{|%1 }0has been explicitly marked "
5943+
"%select{unavailable|deleted|deprecated|nodiscard|warn_unused_result|pure|const}2 here">;
59445944
def note_partial_availability_specified_here : Note<
59455945
"%0 has been marked as being introduced in %1 %2 %select{|in %5 environment }4here, "
59465946
"but the deployment target is %1 %3%select{| %6 environment }4">;
@@ -9263,7 +9263,7 @@ def warn_unused_container_subscript_expr : Warning<
92639263
"container access result unused - container access should not be used for side effects">,
92649264
InGroup<UnusedValue>;
92659265
def warn_unused_call : Warning<
9266-
"ignoring return value of function declared with %0 attribute">,
9266+
"ignoring return value of function declared with %select{pure|const}0 attribute">,
92679267
InGroup<UnusedValue>;
92689268
def warn_unused_constructor : Warning<
92699269
"ignoring temporary created by a constructor declared with %0 attribute">,

clang/lib/AST/Expr.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1616,22 +1616,23 @@ 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 Decl *, 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 {D, D ? D->getAttr<WarnUnusedResultAttr>() : nullptr};
16351636
}
16361637

16371638
SourceLocation CallExpr::getBeginLoc() const {

clang/lib/Sema/SemaAvailability.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -670,8 +670,12 @@ static void DoEmitAvailabilityWarning(Sema &S, AvailabilityResult K,
670670
S.Diag(UnknownObjCClass->getLocation(), diag::note_forward_class);
671671
}
672672

673-
S.Diag(NoteLocation, diag_available_here)
674-
<< OffendingDecl << available_here_select_kind;
673+
if (diag_available_here == diag::note_availability_specified_here)
674+
S.Diag(NoteLocation, diag_available_here)
675+
<< true << OffendingDecl << available_here_select_kind;
676+
else
677+
S.Diag(NoteLocation, diag_available_here)
678+
<< OffendingDecl << available_here_select_kind;
675679
}
676680

677681
void Sema::handleDelayedAvailabilityCheck(DelayedDiagnostic &DD, Decl *Ctx) {

clang/lib/Sema/SemaExpr.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ void Sema::NoteDeletedFunction(FunctionDecl *Decl) {
140140
return NoteDeletedInheritingConstructor(Ctor);
141141

142142
Diag(Decl->getLocation(), diag::note_availability_specified_here)
143-
<< Decl << 1;
143+
<< true << Decl << 1;
144144
}
145145

146146
/// Determine whether a FunctionDecl was ever declared with an

clang/lib/Sema/SemaStmt.cpp

Lines changed: 53 additions & 24 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,30 @@ 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 Decl *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

214215
if (Msg.empty()) {
215216
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;
217+
S.Diag(Loc, diag::warn_unused_constructor) << A << R1 << R2;
218+
else
219+
S.Diag(Loc, diag::warn_unused_result) << A << R1 << R2;
220+
} else if (IsCtor) {
221+
S.Diag(Loc, diag::warn_unused_constructor_msg) << A << Msg << R1 << R2;
222+
} else {
223+
S.Diag(Loc, diag::warn_unused_result_msg) << A << Msg << R1 << R2;
218224
}
219225

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;
226+
// nodiscard = 3, warn_unused_result = 4
227+
unsigned available_here_select_kind =
228+
(A->getSpelling() == StringRef{"nodiscard"} ? 3 : 4);
229+
return S.Diag(A->getLocation(), diag::note_availability_specified_here)
230+
<< isa<NamedDecl>(OffendingDecl) << dyn_cast<NamedDecl>(OffendingDecl)
231+
<< available_here_select_kind;
224232
}
225233

226234
void Sema::DiagnoseUnusedExprResult(const Stmt *S, unsigned DiagID) {
@@ -290,9 +298,10 @@ void Sema::DiagnoseUnusedExprResult(const Stmt *S, unsigned DiagID) {
290298
if (E->getType()->isVoidType())
291299
return;
292300

293-
if (DiagnoseNoDiscard(*this, cast_or_null<WarnUnusedResultAttr>(
294-
CE->getUnusedResultAttr(Context)),
295-
Loc, R1, R2, /*isCtor=*/false))
301+
const auto &[OffendingDecl, A] = CE->getUnusedResultAttr(Context);
302+
if (DiagnoseNoDiscard(*this, OffendingDecl,
303+
cast_or_null<WarnUnusedResultAttr>(A), Loc, R1, R2,
304+
/*isCtor=*/false))
296305
return;
297306

298307
// If the callee has attribute pure, const, or warn_unused_result, warn with
@@ -302,27 +311,47 @@ void Sema::DiagnoseUnusedExprResult(const Stmt *S, unsigned DiagID) {
302311
if (const Decl *FD = CE->getCalleeDecl()) {
303312
if (ShouldSuppress)
304313
return;
305-
if (FD->hasAttr<PureAttr>()) {
306-
Diag(Loc, diag::warn_unused_call) << R1 << R2 << "pure";
307-
return;
308-
}
309-
if (FD->hasAttr<ConstAttr>()) {
310-
Diag(Loc, diag::warn_unused_call) << R1 << R2 << "const";
314+
315+
const InheritableAttr *A = nullptr;
316+
if (const auto *PA = FD->getAttr<PureAttr>())
317+
A = PA;
318+
else if (const auto *CA = FD->getAttr<ConstAttr>())
319+
A = CA;
320+
321+
if (A) {
322+
// pure = 0, const = 1 for warn_unused_call
323+
unsigned available_here_select_kind = (isa<PureAttr>(A) ? 0 : 1);
324+
Diag(Loc, diag::warn_unused_call)
325+
<< R1 << R2 << available_here_select_kind;
326+
available_here_select_kind += 5; // pure = 5, const = 6 for availability
327+
if (const auto *ND = dyn_cast<NamedDecl>(OffendingDecl)) {
328+
if (!ND->getIdentifier()->getBuiltinID())
329+
Diag(A->getLocation(), diag::note_availability_specified_here)
330+
<< true << ND << available_here_select_kind;
331+
} else {
332+
Diag(A->getLocation(), diag::note_availability_specified_here)
333+
<< false << ND << available_here_select_kind;
334+
}
311335
return;
312336
}
313337
}
314338
} else if (const auto *CE = dyn_cast<CXXConstructExpr>(E)) {
315339
if (const CXXConstructorDecl *Ctor = CE->getConstructor()) {
340+
const NamedDecl *OffendingDecl = Ctor;
316341
const auto *A = Ctor->getAttr<WarnUnusedResultAttr>();
317-
A = A ? A : Ctor->getParent()->getAttr<WarnUnusedResultAttr>();
318-
if (DiagnoseNoDiscard(*this, A, Loc, R1, R2, /*isCtor=*/true))
342+
if (!A) {
343+
OffendingDecl = Ctor->getParent();
344+
A = OffendingDecl->getAttr<WarnUnusedResultAttr>();
345+
}
346+
if (DiagnoseNoDiscard(*this, OffendingDecl, A, Loc, R1, R2,
347+
/*isCtor=*/true))
319348
return;
320349
}
321350
} else if (const auto *ILE = dyn_cast<InitListExpr>(E)) {
322351
if (const TagDecl *TD = ILE->getType()->getAsTagDecl()) {
323352

324-
if (DiagnoseNoDiscard(*this, TD->getAttr<WarnUnusedResultAttr>(), Loc, R1,
325-
R2, /*isCtor=*/false))
353+
if (DiagnoseNoDiscard(*this, TD, TD->getAttr<WarnUnusedResultAttr>(), Loc,
354+
R1, R2, /*isCtor=*/false))
326355
return;
327356
}
328357
} else if (ShouldSuppress)
@@ -336,8 +365,8 @@ void Sema::DiagnoseUnusedExprResult(const Stmt *S, unsigned DiagID) {
336365
}
337366
const ObjCMethodDecl *MD = ME->getMethodDecl();
338367
if (MD) {
339-
if (DiagnoseNoDiscard(*this, MD->getAttr<WarnUnusedResultAttr>(), Loc, R1,
340-
R2, /*isCtor=*/false))
368+
if (DiagnoseNoDiscard(*this, MD, MD->getAttr<WarnUnusedResultAttr>(), Loc,
369+
R1, R2, /*isCtor=*/false))
341370
return;
342371
}
343372
} 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
}

0 commit comments

Comments
 (0)