[clang] [clang] Generate note on declaration for nodiscard-related attributes (PR #112289)

[Yihe Li via cfe-commits]

https://github.com/Mick235711 created 
https://github.com/llvm/llvm-project/pull/112289

Currently, the following program
```cpp
[[nodiscard]] int fun() { return 1; }
[[deprecated]] int fun2() { return 2; }

int main()
{
fun(); fun2();
}
```
generates the following diagnostics on Clang trunk: ([Compiler 
Explorer](https://godbolt.org/z/48crWEoYY))
```cpp
:6:5: warning: ignoring return value of function declared with 
'nodiscard' attribute [-Wunused-result]
6 | fun(); fun2();
  | ^~~
:6:12: warning: 'fun2' is deprecated [-Wdeprecated-declarations]
6 | fun(); fun2();
  |^
:2:3: note: 'fun2' has been explicitly marked deprecated here
2 | [[deprecated]] int fun2() { return 2; }
  |   ^
2 warnings generated.
```
There seems to exist a discrepancy between `[[deprecated]]` and 
`[[nodiscard]]`. The former generates a warning on the usage of the function, 
together with a note on the declaration of the function. In contrast, the 
latter only generates a warning.

This PR tries to fix this discrepancy by additionally generating a note on the 
declaration of all `[[nodiscard]]`-related attributes (`nodiscard`, 
`warn_unused_result`, `pure`, and `const`). All 4 attributes generate a warning 
on ignoring the return value/constructor result on the trunk, and after this 
PR, all 4 attributes additionally generate a note. The above program will 
output the following diagnostics after this PR:
```cpp
test.cpp:6:5: warning: ignoring return value of function declared with 
'nodiscard' attribute [-Wunused-result]
6 | fun(); fun2();
  | ^~~
test.cpp:1:3: note: 'fun' has been explicitly marked nodiscard here
1 | [[nodiscard]] int fun() { return 1; }
  |   ^
test.cpp:6:12: warning: 'fun2' is deprecated [-Wdeprecated-declarations]
6 | fun(); fun2();
  |^
test.cpp:2:3: note: 'fun2' has been explicitly marked deprecated here
2 | [[deprecated]] int fun2() { return 2; }
  |   ^
2 warnings generated.
```

---
FIrst time contributor here who is not very familiar with Clang's 
infrastructure... Any comments and suggestions are welcome. No new test cases 
are added, as I felt that adding `expected-note`s on existing test cases 
already serves as testing for this PR.

>From 05e44dc97dcb351f719a5f2ce553c8af5aacfac7 Mon Sep 17 00:00:00 2001
From: Yihe Li 
Date: Tue, 15 Oct 2024 08:13:22 +0800
Subject: [PATCH] [clang] Generate note on declaration for nodiscard-related
 attributes

---
 clang/include/clang/AST/Expr.h|  5 +-
 .../clang/Basic/DiagnosticSemaKinds.td|  2 +
 clang/lib/AST/Expr.cpp| 10 +--
 clang/lib/Sema/SemaStmt.cpp   | 63 ---
 .../dcl.attr/dcl.attr.nodiscard/p2.cpp| 29 -
 .../dcl.attr/dcl.attr.nodiscard/p3.cpp|  2 +-
 .../test/OpenMP/declare_variant_messages.cpp  |  2 +-
 clang/test/Sema/c2x-nodiscard.c   | 12 ++--
 clang/test/Sema/unused-expr.c | 10 +--
 clang/test/SemaCXX/coroutines.cpp |  2 +-
 clang/test/SemaCXX/warn-unused-result.cpp | 26 
 .../SemaObjC/method-warn-unused-attribute.m   |  6 +-
 12 files changed, 98 insertions(+), 71 deletions(-)

diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index cbe62411d11bff..8f5679767529fd 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -3182,11 +3182,12 @@ class CallExpr : public Expr {
 
   /// Returns the WarnUnusedResultAttr that is either declared on the called
   /// function, or its return type declaration.
-  const Attr *getUnusedResultAttr(const ASTContext &Ctx) const;
+  std::pair
+  getUnusedResultAttr(const ASTContext &Ctx) const;
 
   /// Returns true if this call expression should warn on unused results.
   bool hasUnusedResultAttr(const ASTContext &Ctx) const {
-return getUnusedResultAttr(Ctx) != nullptr;
+return getUnusedResultAttr(Ctx).second != nullptr;
   }
 
   SourceLocation getRParenLoc() const { return RParenLoc; }
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c709795e7b21d8..3f6e4c35146454 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9290,6 +9290,8 @@ def warn_unused_result_typedef_unsupported_spelling : 
Warning<
 def warn_unused_volatile : Warning<
   "expression result unused; assign into a variable to force a volatile load">,
   InGroup>;
+def note_nodiscard_specified_here : Note<
+  "%0 has been explicitly marked %1 here">;
 
 def ext_cxx14_attr : Extension<
   "use of the %0 attribute is a C++14 extension">, InGroup;
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 9ecbf121e3fc0d..b9450d6a61e57c 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -1616,22 +1616,24 @@ QualType CallExpr::getCallReturnType(const ASTContext 
&Ctx) const {
   ret

[clang] [clang] Generate note on declaration for nodiscard-related attributes (PR #112289)

[Yihe Li via cfe-commits]

@@ -302,27 +312,38 @@ void Sema::DiagnoseUnusedExprResult(const Stmt *S, 
unsigned DiagID) {
 if (const Decl *FD = CE->getCalleeDecl()) {
   if (ShouldSuppress)
 return;
-  if (FD->hasAttr()) {
+  if (const auto *A = FD->getAttr()) {
 Diag(Loc, diag::warn_unused_call) << R1 << R2 << "pure";
+if (OffendingDecl && !OffendingDecl->getIdentifier()->getBuiltinID())

Mick235711 wrote:

That check on builtin is actually added after the test `Seme/enable-if.c` fails:
```cpp
int isdigit(int c) __attribute__((overloadable));
int isdigit(int c) __attribute__((overloadable)) // expected-note {{'isdigit' 
has been explicitly marked unavailable here}}
  __attribute__((enable_if(c <= -1 || c > 255, "'c' must have the value of an 
unsigned char or EOF")))
  __attribute__((unavailable("'c' must have the value of an unsigned char or 
EOF")));

void test3(int c) {
  isdigit(c); // expected-warning{{ignoring return value of function declared 
with pure attribute}}
  isdigit(10); // expected-warning{{ignoring return value of function declared 
with pure attribute}}
#ifndef CODEGEN
  isdigit(-10);  // expected-error{{'isdigit' is unavailable: 'c' must have the 
value of an unsigned char or EOF}}
#endif
}
```
In this part of the test, without the builtin test a note will be generated on 
the first line ("`isdigit` has been explicitly marked pure here"), which is the 
result of isdigit been assigned pure attribute as a builtin.

Though despite this, thinking it over now, it is still debatable on whether 
generating note here on builtin is meaningful/useful... Do you think I should 
remove the test?

https://github.com/llvm/llvm-project/pull/112289
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Generate note on declaration for nodiscard-related attributes (PR #112289)

[Yihe Li via cfe-commits]

https://github.com/Mick235711 edited 
https://github.com/llvm/llvm-project/pull/112289
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Generate note on declaration for nodiscard-related attributes (PR #112289)

[Yihe Li via cfe-commits]

@@ -204,23 +205,29 @@ static bool DiagnoseUnusedComparison(Sema &S, const Expr 
*E) {
   return true;
 }
 
-static bool DiagnoseNoDiscard(Sema &S, const WarnUnusedResultAttr *A,
-  SourceLocation Loc, SourceRange R1,
-  SourceRange R2, bool IsCtor) {
+static bool DiagnoseNoDiscard(Sema &S, const NamedDecl *OffendingDecl,
+  const WarnUnusedResultAttr *A, SourceLocation 
Loc,
+  SourceRange R1, SourceRange R2, bool IsCtor) {
   if (!A)
 return false;
   StringRef Msg = A->getMessage();
 
+  bool result;
   if (Msg.empty()) {
 if (IsCtor)
-  return S.Diag(Loc, diag::warn_unused_constructor) << A << R1 << R2;
-return S.Diag(Loc, diag::warn_unused_result) << A << R1 << R2;
-  }
+  result = S.Diag(Loc, diag::warn_unused_constructor) << A << R1 << R2;
+else
+  result = S.Diag(Loc, diag::warn_unused_result) << A << R1 << R2;
+  } else if (IsCtor)
+result = S.Diag(Loc, diag::warn_unused_constructor_msg)
+ << A << Msg << R1 << R2;
+  else
+result = S.Diag(Loc, diag::warn_unused_result_msg) << A << Msg << R1 << R2;
 
-  if (IsCtor)
-return S.Diag(Loc, diag::warn_unused_constructor_msg) << A << Msg << R1
-  << R2;
-  return S.Diag(Loc, diag::warn_unused_result_msg) << A << Msg << R1 << R2;
+  if (OffendingDecl)

Mick235711 wrote:

Well, the initial motivation for that if is the following call site:
```cpp
const NamedDecl *OffendingDecl;
const Attr *A;
std::tie(OffendingDecl, A) = CE->getUnusedResultAttr(Context);
if (DiagnoseNoDiscard(*this, OffendingDecl,
  cast_or_null(A), Loc, R1, R2,
  /*isCtor=*/false))
  return;
```
In `CallExpr::getUnusedResultAttr` we tried to see if the callee is marked 
nodiscard and if so, returns that instead. However `getCalleeDecl()` returns a 
`const Decl *`, but the diagnostic requires a `const NamedDecl *`, so I 
dyn_cast it down. If the actual dynamic type is always NamedDecl or its 
subclass, maybe here the return value cannot ever be `nullptr` to begin with? I 
was not sure about this so I added the check.

https://github.com/llvm/llvm-project/pull/112289
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Generate note on declaration for nodiscard-related attributes (PR #112289)

[Yihe Li via cfe-commits]

@@ -290,9 +297,12 @@ void Sema::DiagnoseUnusedExprResult(const Stmt *S, 
unsigned DiagID) {
 if (E->getType()->isVoidType())
   return;
 
-if (DiagnoseNoDiscard(*this, cast_or_null(
- CE->getUnusedResultAttr(Context)),
-  Loc, R1, R2, /*isCtor=*/false))
+const NamedDecl *OffendingDecl;
+const Attr *A;
+std::tie(OffendingDecl, A) = CE->getUnusedResultAttr(Context);

Mick235711 wrote:

Oh I don't see structured binding used in this file, so I just copied the 
existing usage of `tie`. Will be fixed in the next push.

https://github.com/llvm/llvm-project/pull/112289
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Generate note on declaration for nodiscard-related attributes (PR #112289)

[Yihe Li via cfe-commits]

@@ -204,23 +205,29 @@ static bool DiagnoseUnusedComparison(Sema &S, const Expr 
*E) {
   return true;
 }
 
-static bool DiagnoseNoDiscard(Sema &S, const WarnUnusedResultAttr *A,
-  SourceLocation Loc, SourceRange R1,
-  SourceRange R2, bool IsCtor) {
+static bool DiagnoseNoDiscard(Sema &S, const NamedDecl *OffendingDecl,
+  const WarnUnusedResultAttr *A, SourceLocation 
Loc,
+  SourceRange R1, SourceRange R2, bool IsCtor) {
   if (!A)
 return false;
   StringRef Msg = A->getMessage();
 
+  bool result;
   if (Msg.empty()) {
 if (IsCtor)
-  return S.Diag(Loc, diag::warn_unused_constructor) << A << R1 << R2;
-return S.Diag(Loc, diag::warn_unused_result) << A << R1 << R2;
-  }
+  result = S.Diag(Loc, diag::warn_unused_constructor) << A << R1 << R2;
+else
+  result = S.Diag(Loc, diag::warn_unused_result) << A << R1 << R2;
+  } else if (IsCtor)
+result = S.Diag(Loc, diag::warn_unused_constructor_msg)
+ << A << Msg << R1 << R2;
+  else
+result = S.Diag(Loc, diag::warn_unused_result_msg) << A << Msg << R1 << R2;
 
-  if (IsCtor)
-return S.Diag(Loc, diag::warn_unused_constructor_msg) << A << Msg << R1
-  << R2;
-  return S.Diag(Loc, diag::warn_unused_result_msg) << A << Msg << R1 << R2;
+  if (OffendingDecl)

Mick235711 wrote:

Well, `S.Diag(...) << [a const Decl *]` just gives me an error that no such 
operator<< exists. Is there some different syntax needed to pass a Decl as 
parameter to the diagnostics builder?

https://github.com/llvm/llvm-project/pull/112289
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Generate note on declaration for nodiscard-related attributes (PR #112289)

[Yihe Li via cfe-commits]

@@ -9290,6 +9290,8 @@ def warn_unused_result_typedef_unsupported_spelling : 
Warning<
 def warn_unused_volatile : Warning<
   "expression result unused; assign into a variable to force a volatile load">,
   InGroup>;
+def note_nodiscard_specified_here : Note<

Mick235711 wrote:

Sure, will be combined in the next push.

https://github.com/llvm/llvm-project/pull/112289
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Generate note on declaration for nodiscard-related attributes (PR #112289)

[Yihe Li via cfe-commits]

Mick235711 wrote:

> That is, keep the current wording if the function is nodiscard, but change it 
> to mention the type instead if the type is marked nodiscard—provided there is 
> a relatively straight-forward way of doing this.

I think this should be okay-ish to implement since we basically already have 
that information when implementing the note anyway. Will try to implement this 
tomorrow, if I can find some time...

https://github.com/llvm/llvm-project/pull/112289
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Generate note on declaration for nodiscard-related attributes (PR #112289)

[Yihe Li via cfe-commits]

@@ -204,23 +205,29 @@ static bool DiagnoseUnusedComparison(Sema &S, const Expr 
*E) {
   return true;
 }
 
-static bool DiagnoseNoDiscard(Sema &S, const WarnUnusedResultAttr *A,
-  SourceLocation Loc, SourceRange R1,
-  SourceRange R2, bool IsCtor) {
+static bool DiagnoseNoDiscard(Sema &S, const NamedDecl *OffendingDecl,
+  const WarnUnusedResultAttr *A, SourceLocation 
Loc,
+  SourceRange R1, SourceRange R2, bool IsCtor) {
   if (!A)
 return false;
   StringRef Msg = A->getMessage();
 
+  bool result;
   if (Msg.empty()) {
 if (IsCtor)
-  return S.Diag(Loc, diag::warn_unused_constructor) << A << R1 << R2;
-return S.Diag(Loc, diag::warn_unused_result) << A << R1 << R2;
-  }
+  result = S.Diag(Loc, diag::warn_unused_constructor) << A << R1 << R2;
+else
+  result = S.Diag(Loc, diag::warn_unused_result) << A << R1 << R2;
+  } else if (IsCtor)
+result = S.Diag(Loc, diag::warn_unused_constructor_msg)
+ << A << Msg << R1 << R2;
+  else
+result = S.Diag(Loc, diag::warn_unused_result_msg) << A << Msg << R1 << R2;
 
-  if (IsCtor)
-return S.Diag(Loc, diag::warn_unused_constructor_msg) << A << Msg << R1
-  << R2;
-  return S.Diag(Loc, diag::warn_unused_result_msg) << A << Msg << R1 << R2;
+  if (OffendingDecl)

Mick235711 wrote:

That is a good idea, will implement this in the next push.

https://github.com/llvm/llvm-project/pull/112289
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Generate note on declaration for nodiscard-related attributes (PR #112289)

[Yihe Li via cfe-commits]

Mick235711 wrote:

> I don’t really see that happening for nodiscard tho—making only some 
> overloads nodiscard instead of none or all of them just seems like a really 
> weird thing to do personally...

I think there is still some merits for these kind of overload sets. For 
instance, API like 
[`basic_spanstream::span()`](https://en.cppreference.com/w/cpp/io/basic_spanstream/span),
 where the same name is used for both setter and getter, is pretty common out 
in the wild:
```cpp
std::span span() const noexcept;
void span( std::span s ) noexcept;
```
In these kind of cases the first overload can be marked [[nodiscard]], which is 
probably reasonable.

> As I said, I feel like that might be by design, though.

> at least for deprecated, you could maybe argue that you might want to know 
> what part of the codebase marked it as deprecated in case there are multiple 
> declarations

Indeed, I'm currently torn about this. On the one hand, the same argument can 
be applied to nodiscard, where you might want to know what part of the codebase 
marked it as nodiscard in case of multiple declarations. On the other hand, the 
motivation is definitely weaker here.

> The one use case I could see is that the nodiscard is on the TYPE instead of 
> the function itself (which, as you said before, is goofy for an overload set 
> with this only partially applied). But I don't really see value besides "this 
> should not be discarded", and the actual location of the attribute isn't 
> particularly valuable. What IS important is the 'reason', which we already 
> have.

I can suggest one motivation here, which potentially applies to [[nodiscard]] 
on types. In the standard, it is allowed to mark only some specializations of a 
class template as nodiscard:
```cpp
template struct S {};
template<> struct [[nodiscard]] S {};

template
S getS(const T&) { return {}; }

int main()
{
getS(2.0); // no warn
getS(2); // warns
}
```
In this case, the current warning is not especially good, the only thing 
mentioned is: ([Compiler Explorer](https://godbolt.org/z/4cWqPfo4T))
```cpp
:10:5: warning: ignoring return value of function declared with 
'nodiscard' attribute [-Wunused-result]
   10 | getS(2);
  | ^~~~ ~
1 warning generated.
```
Nowhere is the return type `S` actually mentioned, let alone the specific 
specialization. Under this PR, this will instead generate:
```cpp
test-2.cpp:10:5: warning: ignoring return value of function declared with 
'nodiscard' attribute [-Wunused-result]
   10 | getS(2);
  | ^~~~ ~
test-2.cpp:2:21: note: 'S' has been explicitly marked nodiscard here
2 | template<> struct [[nodiscard]] S {};
  | ^
1 warning generated.
```
Here the specialization is named, which is potentially helpful. Although I'll 
admit this is a bit of a contrived example.

The noise argument does makes sense though, so I'm currently undecided about 
this too.

https://github.com/llvm/llvm-project/pull/112289
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Generate note on declaration for nodiscard-related attributes (PR #112289)

[Yihe Li via cfe-commits]

@@ -9290,6 +9290,8 @@ def warn_unused_result_typedef_unsupported_spelling : 
Warning<
 def warn_unused_volatile : Warning<
   "expression result unused; assign into a variable to force a volatile load">,
   InGroup>;
+def note_nodiscard_specified_here : Note<

Mick235711 wrote:

That note currently reads:
```
def note_availability_specified_here : Note<
  "%0 has been explicitly marked "
  "%select{unavailable|deleted|deprecated}1 here">;
```
Currently, this is the combined note used for unavailable, deleted, and 
deprecated, and the flavor is chosen by a number (0/1/2) passed as the second 
argument. Probably, if we want to reuse this, that should be changed to a 
string parameter?


https://github.com/llvm/llvm-project/pull/112289
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Generate note on declaration for nodiscard-related attributes (PR #112289)

[Yihe Li via cfe-commits]

@@ -204,23 +205,29 @@ static bool DiagnoseUnusedComparison(Sema &S, const Expr 
*E) {
   return true;
 }
 
-static bool DiagnoseNoDiscard(Sema &S, const WarnUnusedResultAttr *A,
-  SourceLocation Loc, SourceRange R1,
-  SourceRange R2, bool IsCtor) {
+static bool DiagnoseNoDiscard(Sema &S, const NamedDecl *OffendingDecl,
+  const WarnUnusedResultAttr *A, SourceLocation 
Loc,
+  SourceRange R1, SourceRange R2, bool IsCtor) {
   if (!A)
 return false;
   StringRef Msg = A->getMessage();
 
+  bool result;
   if (Msg.empty()) {
 if (IsCtor)
-  return S.Diag(Loc, diag::warn_unused_constructor) << A << R1 << R2;
-return S.Diag(Loc, diag::warn_unused_result) << A << R1 << R2;
-  }
+  result = S.Diag(Loc, diag::warn_unused_constructor) << A << R1 << R2;

Mick235711 wrote:

> Also, 'result' is always going to be true. Diag always returns true, it is 
> just a shortcut to return early with a common pattern we have.

Oh, if that is the case, indeed there I got it wrong. I assumed the return 
value is meaningful. Probably just remove all the return then.

https://github.com/llvm/llvm-project/pull/112289
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Generate note on declaration for nodiscard-related attributes (PR #112289)

[Yihe Li via cfe-commits]

https://github.com/Mick235711 updated 
https://github.com/llvm/llvm-project/pull/112289

>From 2376ab367715ef2f7f77ffc4d5af21393af876bd Mon Sep 17 00:00:00 2001
From: Yihe Li 
Date: Tue, 15 Oct 2024 08:13:22 +0800
Subject: [PATCH] [clang] Generate note on declaration for nodiscard-related
 attributes

---
 clang/include/clang/AST/Expr.h|  5 +-
 .../clang/Basic/DiagnosticSemaKinds.td|  2 +
 clang/lib/AST/Expr.cpp|  9 +--
 clang/lib/Sema/SemaStmt.cpp   | 71 ---
 .../dcl.attr/dcl.attr.nodiscard/p2.cpp| 29 
 .../dcl.attr/dcl.attr.nodiscard/p3.cpp|  2 +-
 .../test/OpenMP/declare_variant_messages.cpp  |  2 +-
 clang/test/Sema/c2x-nodiscard.c   | 12 ++--
 clang/test/Sema/unused-expr.c | 10 +--
 clang/test/SemaCXX/coroutines.cpp |  2 +-
 clang/test/SemaCXX/warn-unused-result.cpp | 26 +++
 .../SemaObjC/method-warn-unused-attribute.m   |  6 +-
 12 files changed, 101 insertions(+), 75 deletions(-)

diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index cbe62411d11bff..4ba109cf059e43 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -3182,11 +3182,12 @@ class CallExpr : public Expr {
 
   /// Returns the WarnUnusedResultAttr that is either declared on the called
   /// function, or its return type declaration.
-  const Attr *getUnusedResultAttr(const ASTContext &Ctx) const;
+  std::pair
+  getUnusedResultAttr(const ASTContext &Ctx) const;
 
   /// Returns true if this call expression should warn on unused results.
   bool hasUnusedResultAttr(const ASTContext &Ctx) const {
-return getUnusedResultAttr(Ctx) != nullptr;
+return getUnusedResultAttr(Ctx).second != nullptr;
   }
 
   SourceLocation getRParenLoc() const { return RParenLoc; }
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c709795e7b21d8..cced726f12b000 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9290,6 +9290,8 @@ def warn_unused_result_typedef_unsupported_spelling : 
Warning<
 def warn_unused_volatile : Warning<
   "expression result unused; assign into a variable to force a volatile load">,
   InGroup>;
+def note_nodiscard_specified_here : Note<
+  "%select{|%1 }0has been explicitly marked %2 here">;
 
 def ext_cxx14_attr : Extension<
   "use of the %0 attribute is a C++14 extension">, InGroup;
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 9ecbf121e3fc0d..6a3eb1238a89e2 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -1616,22 +1616,23 @@ QualType CallExpr::getCallReturnType(const ASTContext 
&Ctx) const {
   return FnType->getReturnType();
 }
 
-const Attr *CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
+std::pair
+CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
   // 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())
-  return A;
+  return {TD, A};
 
   for (const auto *TD = getCallReturnType(Ctx)->getAs(); TD;
TD = TD->desugar()->getAs())
 if (const auto *A = TD->getDecl()->getAttr())
-  return A;
+  return {TD->getDecl(), A};
 
   // Otherwise, see if the callee is marked nodiscard and return that attribute
   // instead.
   const Decl *D = getCalleeDecl();
-  return D ? D->getAttr() : nullptr;
+  return {D, D ? D->getAttr() : nullptr};
 }
 
 SourceLocation CallExpr::getBeginLoc() const {
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 9e235a46707cd4..fb8ce5f2e27704 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -14,6 +14,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTDiagnostic.h"
 #include "clang/AST/ASTLambda.h"
+#include "clang/AST/Attrs.inc"
 #include "clang/AST/CXXInheritance.h"
 #include "clang/AST/CharUnits.h"
 #include "clang/AST/DeclObjC.h"
@@ -204,23 +205,26 @@ static bool DiagnoseUnusedComparison(Sema &S, const Expr 
*E) {
   return true;
 }
 
-static bool DiagnoseNoDiscard(Sema &S, const WarnUnusedResultAttr *A,
-  SourceLocation Loc, SourceRange R1,
-  SourceRange R2, bool IsCtor) {
+static bool DiagnoseNoDiscard(Sema &S, const Decl *OffendingDecl,
+  const WarnUnusedResultAttr *A, SourceLocation 
Loc,
+  SourceRange R1, SourceRange R2, bool IsCtor) {
   if (!A)
 return false;
   StringRef Msg = A->getMessage();
 
   if (Msg.empty()) {
 if (IsCtor)
-  return S.Diag(Loc, diag::warn_unused_constructor) << A << R1 << R2;
-return S.Diag(Loc, diag::warn_unused_result) << A << R1 << R2;
-  }
+  S.Diag(Loc, diag::

[clang] [clang] Generate note on declaration for nodiscard-related attributes (PR #112289)

[Yihe Li via cfe-commits]

@@ -302,27 +307,43 @@ void Sema::DiagnoseUnusedExprResult(const Stmt *S, 
unsigned DiagID) {
 if (const Decl *FD = CE->getCalleeDecl()) {
   if (ShouldSuppress)
 return;
-  if (FD->hasAttr()) {
-Diag(Loc, diag::warn_unused_call) << R1 << R2 << "pure";
-return;
-  }
-  if (FD->hasAttr()) {
-Diag(Loc, diag::warn_unused_call) << R1 << R2 << "const";
+
+  const InheritableAttr *A = nullptr;
+  if (const auto *PA = FD->getAttr())
+A = PA;
+  else if (const auto *CA = FD->getAttr())
+A = CA;
+
+  if (A) {
+StringRef type = (isa(A) ? "pure" : "const");
+Diag(Loc, diag::warn_unused_call) << R1 << R2 << type;
+if (const auto *ND = dyn_cast(OffendingDecl)) {
+  if (!ND->getIdentifier()->getBuiltinID())

Mick235711 wrote:

As I said above, the real motivation for avoiding marking here is twofold:
1. Builtins like `isdigit` are just marked pure internally, the note 
"explicitly marked pure/const here" is just wrong, there is no such attribute 
on that line.
```cpp
int isdigit(int c) __attribute__((overloadable)); // The note actually fires on 
this line, which doesn't have a pure attribute
int isdigit(int c) __attribute__((overloadable)) // expected-note {{'isdigit' 
has been explicitly marked unavailable here}}
  __attribute__((enable_if(c <= -1 || c > 255, "'c' must have the value of an 
unsigned char or EOF")))
  __attribute__((unavailable("'c' must have the value of an unsigned char or 
EOF")));

void test3(int c) {
  isdigit(c); // expected-warning{{ignoring return value of function declared 
with pure attribute}}
  isdigit(10); // expected-warning{{ignoring return value of function declared 
with pure attribute}}
#ifndef CODEGEN
  isdigit(-10);  // expected-error{{'isdigit' is unavailable: 'c' must have the 
value of an unsigned char or EOF}}
#endif
}
```
Unless `A->getLocation()` is not the right invocation to find the attribute 
location, I don't think there is a "builtin" declaration line that can be 
referenced by the note.
2. Language builtins like `__builtin_operator_new` will just fire the note on 
the first use:
```cpp
void test_typo_in_args() {
  __builtin_operator_new(DNE);  // expected-error {{undeclared 
identifier 'DNE'}}
  __builtin_operator_new(DNE, DNE2);// expected-error {{undeclared 
identifier 'DNE'}} expected-error {{'DNE2'}}
  __builtin_operator_delete(DNE);   // expected-error {{'DNE'}}
  __builtin_operator_delete(DNE, DNE2); // expected-error {{'DNE'}} 
expected-error {{'DNE2'}}
}
```
In this test case (`SemaCXX/builtin-operator-new-delete.cpp`), the note is 
generated on the second line, i.e. first use of builtin, which is even more 
wrong as it is not even a declaration.

Excluding builtins from note generation fixes both errors, I think...

https://github.com/llvm/llvm-project/pull/112289
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Generate note on declaration for nodiscard-related attributes (PR #112289)

[Yihe Li via cfe-commits]

https://github.com/Mick235711 edited 
https://github.com/llvm/llvm-project/pull/112289
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Generate note on declaration for nodiscard-related attributes (PR #112289)

[Yihe Li via cfe-commits]

https://github.com/Mick235711 updated 
https://github.com/llvm/llvm-project/pull/112289

>From aaad12bcc38cfd11597e2e0c5ba93f2197e5a4ac Mon Sep 17 00:00:00 2001
From: Yihe Li 
Date: Tue, 15 Oct 2024 08:13:22 +0800
Subject: [PATCH] [clang] Generate note on declaration for nodiscard-related
 attributes

---
 clang/include/clang/AST/Expr.h|  5 +-
 .../clang/Basic/DiagnosticSemaKinds.td|  6 +-
 clang/lib/AST/Expr.cpp|  9 ++-
 clang/lib/Sema/SemaAvailability.cpp   |  8 +-
 clang/lib/Sema/SemaExpr.cpp   |  2 +-
 clang/lib/Sema/SemaStmt.cpp   | 77 +--
 .../dcl.attr/dcl.attr.nodiscard/p2.cpp| 29 +++
 .../dcl.attr/dcl.attr.nodiscard/p3.cpp|  2 +-
 .../test/OpenMP/declare_variant_messages.cpp  |  2 +-
 clang/test/Sema/c2x-nodiscard.c   | 12 +--
 clang/test/Sema/unused-expr.c | 10 +--
 clang/test/SemaCXX/coroutines.cpp |  2 +-
 clang/test/SemaCXX/warn-unused-result.cpp | 26 +++
 .../SemaObjC/method-warn-unused-attribute.m   |  6 +-
 14 files changed, 116 insertions(+), 80 deletions(-)

diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index cbe62411d11bff..4ba109cf059e43 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -3182,11 +3182,12 @@ class CallExpr : public Expr {
 
   /// Returns the WarnUnusedResultAttr that is either declared on the called
   /// function, or its return type declaration.
-  const Attr *getUnusedResultAttr(const ASTContext &Ctx) const;
+  std::pair
+  getUnusedResultAttr(const ASTContext &Ctx) const;
 
   /// Returns true if this call expression should warn on unused results.
   bool hasUnusedResultAttr(const ASTContext &Ctx) const {
-return getUnusedResultAttr(Ctx) != nullptr;
+return getUnusedResultAttr(Ctx).second != nullptr;
   }
 
   SourceLocation getRParenLoc() const { return RParenLoc; }
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c709795e7b21d8..b07764a9d29427 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5939,8 +5939,8 @@ def warn_unavailable_fwdclass_message : Warning<
 "%0 may be unavailable because the receiver type is unknown">,
 InGroup;
 def note_availability_specified_here : Note<
-  "%0 has been explicitly marked "
-  "%select{unavailable|deleted|deprecated}1 here">;
+  "%select{|%1 }0has been explicitly marked "
+  
"%select{unavailable|deleted|deprecated|nodiscard|warn_unused_result|pure|const}2
 here">;
 def note_partial_availability_specified_here : Note<
   "%0 has been marked as being introduced in %1 %2 %select{|in %5 environment 
}4here, "
   "but the deployment target is %1 %3%select{| %6 environment }4">;
@@ -9263,7 +9263,7 @@ def warn_unused_container_subscript_expr : Warning<
  "container access result unused - container access should not be used for 
side effects">,
   InGroup;
 def warn_unused_call : Warning<
-  "ignoring return value of function declared with %0 attribute">,
+  "ignoring return value of function declared with %select{pure|const}0 
attribute">,
   InGroup;
 def warn_unused_constructor : Warning<
   "ignoring temporary created by a constructor declared with %0 attribute">,
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 9ecbf121e3fc0d..6a3eb1238a89e2 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -1616,22 +1616,23 @@ QualType CallExpr::getCallReturnType(const ASTContext 
&Ctx) const {
   return FnType->getReturnType();
 }
 
-const Attr *CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
+std::pair
+CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
   // 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())
-  return A;
+  return {TD, A};
 
   for (const auto *TD = getCallReturnType(Ctx)->getAs(); TD;
TD = TD->desugar()->getAs())
 if (const auto *A = TD->getDecl()->getAttr())
-  return A;
+  return {TD->getDecl(), A};
 
   // Otherwise, see if the callee is marked nodiscard and return that attribute
   // instead.
   const Decl *D = getCalleeDecl();
-  return D ? D->getAttr() : nullptr;
+  return {D, D ? D->getAttr() : nullptr};
 }
 
 SourceLocation CallExpr::getBeginLoc() const {
diff --git a/clang/lib/Sema/SemaAvailability.cpp 
b/clang/lib/Sema/SemaAvailability.cpp
index 798cabaa31a476..da74033f233895 100644
--- a/clang/lib/Sema/SemaAvailability.cpp
+++ b/clang/lib/Sema/SemaAvailability.cpp
@@ -670,8 +670,12 @@ static void DoEmitAvailabilityWarning(Sema &S, 
AvailabilityResult K,
 S.Diag(UnknownObjCClass->getLocation(), diag::note_forward_class);
   }
 
-  S.Diag(NoteLocation, diag_available

[clang] [clang] Generate note on declaration for nodiscard-related attributes (PR #112289)

[Yihe Li via cfe-commits]

https://github.com/Mick235711 updated 
https://github.com/llvm/llvm-project/pull/112289

>From b9911d5b997306376fcf5348a24f8bae7b71bfeb Mon Sep 17 00:00:00 2001
From: Yihe Li 
Date: Tue, 15 Oct 2024 08:13:22 +0800
Subject: [PATCH] [clang] Generate note on declaration for nodiscard-related
 attributes

---
 clang/include/clang/AST/Expr.h|  5 +-
 .../clang/Basic/DiagnosticSemaKinds.td|  6 +-
 clang/lib/AST/Expr.cpp|  9 ++-
 clang/lib/Sema/SemaAvailability.cpp   |  8 +-
 clang/lib/Sema/SemaExpr.cpp   |  2 +-
 clang/lib/Sema/SemaStmt.cpp   | 77 +--
 .../dcl.attr/dcl.attr.nodiscard/p2.cpp| 29 +++
 .../dcl.attr/dcl.attr.nodiscard/p3.cpp|  2 +-
 .../test/OpenMP/declare_variant_messages.cpp  |  2 +-
 clang/test/Sema/c2x-nodiscard.c   | 12 +--
 clang/test/Sema/unused-expr.c | 10 +--
 clang/test/SemaCXX/coroutines.cpp |  2 +-
 clang/test/SemaCXX/warn-unused-result.cpp | 26 +++
 .../SemaObjC/method-warn-unused-attribute.m   |  6 +-
 14 files changed, 116 insertions(+), 80 deletions(-)

diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index cbe62411d11bff..4ba109cf059e43 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -3182,11 +3182,12 @@ class CallExpr : public Expr {
 
   /// Returns the WarnUnusedResultAttr that is either declared on the called
   /// function, or its return type declaration.
-  const Attr *getUnusedResultAttr(const ASTContext &Ctx) const;
+  std::pair
+  getUnusedResultAttr(const ASTContext &Ctx) const;
 
   /// Returns true if this call expression should warn on unused results.
   bool hasUnusedResultAttr(const ASTContext &Ctx) const {
-return getUnusedResultAttr(Ctx) != nullptr;
+return getUnusedResultAttr(Ctx).second != nullptr;
   }
 
   SourceLocation getRParenLoc() const { return RParenLoc; }
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index e78acc8dc8c57b..35e2b4faf6e975 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5939,8 +5939,8 @@ def warn_unavailable_fwdclass_message : Warning<
 "%0 may be unavailable because the receiver type is unknown">,
 InGroup;
 def note_availability_specified_here : Note<
-  "%0 has been explicitly marked "
-  "%select{unavailable|deleted|deprecated}1 here">;
+  "%select{|%1 }0has been explicitly marked "
+  
"%select{unavailable|deleted|deprecated|nodiscard|warn_unused_result|pure|const}2
 here">;
 def note_partial_availability_specified_here : Note<
   "%0 has been marked as being introduced in %1 %2 %select{|in %5 environment 
}4here, "
   "but the deployment target is %1 %3%select{| %6 environment }4">;
@@ -9263,7 +9263,7 @@ def warn_unused_container_subscript_expr : Warning<
  "container access result unused - container access should not be used for 
side effects">,
   InGroup;
 def warn_unused_call : Warning<
-  "ignoring return value of function declared with %0 attribute">,
+  "ignoring return value of function declared with %select{pure|const}0 
attribute">,
   InGroup;
 def warn_unused_constructor : Warning<
   "ignoring temporary created by a constructor declared with %0 attribute">,
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 9ecbf121e3fc0d..6a3eb1238a89e2 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -1616,22 +1616,23 @@ QualType CallExpr::getCallReturnType(const ASTContext 
&Ctx) const {
   return FnType->getReturnType();
 }
 
-const Attr *CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
+std::pair
+CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
   // 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())
-  return A;
+  return {TD, A};
 
   for (const auto *TD = getCallReturnType(Ctx)->getAs(); TD;
TD = TD->desugar()->getAs())
 if (const auto *A = TD->getDecl()->getAttr())
-  return A;
+  return {TD->getDecl(), A};
 
   // Otherwise, see if the callee is marked nodiscard and return that attribute
   // instead.
   const Decl *D = getCalleeDecl();
-  return D ? D->getAttr() : nullptr;
+  return {D, D ? D->getAttr() : nullptr};
 }
 
 SourceLocation CallExpr::getBeginLoc() const {
diff --git a/clang/lib/Sema/SemaAvailability.cpp 
b/clang/lib/Sema/SemaAvailability.cpp
index 798cabaa31a476..da74033f233895 100644
--- a/clang/lib/Sema/SemaAvailability.cpp
+++ b/clang/lib/Sema/SemaAvailability.cpp
@@ -670,8 +670,12 @@ static void DoEmitAvailabilityWarning(Sema &S, 
AvailabilityResult K,
 S.Diag(UnknownObjCClass->getLocation(), diag::note_forward_class);
   }
 
-  S.Diag(NoteLocation, diag_available

[clang] [clang] Improve diagnostic on [[nodiscard]] attribute (PR #112521)

[Yihe Li via cfe-commits]

https://github.com/Mick235711 created 
https://github.com/llvm/llvm-project/pull/112521

A follow-up and alternative proposal to #112289.

In this PR, no new notes are added to avoid noise. Instead, originally, every 
`[[nodiscard]]` trigger, regardless of whether it is on function declaration or 
return type, has the same warning. For instance, for the following program:
```cpp
struct S {};
struct [[nodiscard]] S2 {};
[[nodiscard]] S get1();
S2 get2();
int main() {
  get1();
  get2();
}
```
The generated warning currently is the same for two calls in `main()`: 
([Compiler Explorer](https://godbolt.org/z/8bTa5oqMs))
```cpp
:6:3: warning: ignoring return value of function declared with 
'nodiscard' attribute [-Wunused-result]
6 |   get1();
  |   ^~~~
:7:3: warning: ignoring return value of function declared with 
'nodiscard' attribute [-Wunused-result]
7 |   get2();
  |   ^~~~
2 warnings generated.
```

As suggested by 
https://github.com/llvm/llvm-project/pull/112289#issuecomment-2414341257, this 
PR thus makes a distinction here by making `[[nodiscard]]` that are triggered 
by its placement on return types specifically points out what the return type 
is:
```cpp
test-3.cpp:6:3: warning: ignoring return value of function declared with 
'nodiscard' attribute [-Werror,-Wunused-result]
6 |   get1();
  |   ^~~~
test-3.cpp:7:3: warning: ignoring return value of type 'S2' declared with 
'nodiscard' attribute [-Werror,-Wunused-value]
7 |   get2();
  |   ^~~~
2 warnings generated.
```
In addition to better clarity, this also helps identify which specialization is 
marked as `[[nodiscard]]`, as the standard allows `[[nodiscard]]` to be placed 
on some specializations of a class template only, as demonstrated by 
https://github.com/llvm/llvm-project/pull/112289#issuecomment-2414311233.

For constructor calls, a different warning message is also added. Currently, 
for `[[nodiscard]]` (but not for `__attribute__((warn_unused_result))`), 
temporaries that are discarded as a result of a constructor call generates a 
different warning:
```cpp
warning: ignoring temporary created by a constructor declared with 'nodiscard' 
attribute
```
After this PR, if `[[nodiscard]]` is placed on the constructor itself, nothing 
changed. If `[[nodiscard]]` is placed on the type, the new warning is:
```cpp
warning: ignoring temporary of type 'S' declared with 'nodiscard' attribute
```
("created by a constructor" is removed since "declared with 'nodiscard'" 
applies to the type, not the constructor)

One thing to note here is that for the following scenario:
```cpp
struct [[nodiscard]] S {
  [[nodiscard]] S(int) {}
};
void use() { S(2); }
```
The warning message is generated in the format for constructor placement 
("ignoring temporary created by a constructor"). Similarly, for 
`[[nodiscard]]`/`warn_unused_result` on both functions and its return type, the 
function attribute takes precedence as that is probably "more specific".

Compared to the original PR, `pure` and `const` are not touched, so there are 
no builtin-related issues this time.

Some new test cases are added at the end of `SemaCXX/warn-unused-result.cpp` to 
test the new warning.

>From f673e74f05756c44a0d16420949bd961c0464257 Mon Sep 17 00:00:00 2001
From: Yihe Li 
Date: Wed, 16 Oct 2024 18:53:04 +0800
Subject: [PATCH] [clang] Improve diagnostic on [[nodiscard]] attribute

---
 clang/include/clang/AST/Expr.h|   5 +-
 .../clang/Basic/DiagnosticSemaKinds.td|   6 +
 clang/lib/AST/Expr.cpp|  18 +--
 clang/lib/Sema/SemaStmt.cpp   |  40 ---
 .../dcl.attr/dcl.attr.nodiscard/p2.cpp|  28 ++---
 .../dcl.attr/dcl.attr.nodiscard/p3.cpp|   2 +-
 clang/test/Sema/c2x-nodiscard.c   |   8 +-
 clang/test/SemaCXX/warn-unused-result.cpp | 111 ++
 8 files changed, 154 insertions(+), 64 deletions(-)

diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index cbe62411d11bff..8f5679767529fd 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -3182,11 +3182,12 @@ class CallExpr : public Expr {
 
   /// Returns the WarnUnusedResultAttr that is either declared on the called
   /// function, or its return type declaration.
-  const Attr *getUnusedResultAttr(const ASTContext &Ctx) const;
+  std::pair
+  getUnusedResultAttr(const ASTContext &Ctx) const;
 
   /// Returns true if this call expression should warn on unused results.
   bool hasUnusedResultAttr(const ASTContext &Ctx) const {
-return getUnusedResultAttr(Ctx) != nullptr;
+return getUnusedResultAttr(Ctx).second != nullptr;
   }
 
   SourceLocation getRParenLoc() const { return RParenLoc; }
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c709795e7b21d8..f683bfe1df8dde 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/Di

[clang] [clang] Improve diagnostic on [[nodiscard]] attribute (PR #112521)

[Yihe Li via cfe-commits]

Mick235711 wrote:

CC @Sirraide @erichkeane. (Sorry, I do not have permission to request 
reviewers, so instead used ping)

https://github.com/llvm/llvm-project/pull/112521
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Improve diagnostic on [[nodiscard]] attribute (PR #112521)

[Yihe Li via cfe-commits]

@@ -115,20 +115,20 @@ void usage() {
   S('A'); // expected-warning {{ignoring temporary created by a constructor 
declared with 'nodiscard' attribute: Don't let that S-Char go!}}
   S(1);
   S(2.2);
-  Y(); // expected-warning {{ignoring temporary created by a constructor 
declared with 'nodiscard' attribute: Don't throw me away either!}}
+  Y(); // expected-warning {{ignoring temporary of type 'Y' declared with 
'nodiscard' attribute: Don't throw me away either!}}
   S s;
-  ConvertTo{}; // expected-warning {{ignoring return value of function 
declared with 'nodiscard' attribute: Don't throw me away!}}
+  ConvertTo{}; // expected-warning {{ignoring return value of type 'ConvertTo' 
declared with 'nodiscard' attribute: Don't throw me away!}}

Mick235711 wrote:

BTW, isn't this supposed to be a constructor call? Why is `ConvertTo{}` 
different compared to `Y()` above?

https://github.com/llvm/llvm-project/pull/112521
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Improve diagnostic on [[nodiscard]] attribute (PR #112521)

[Yihe Li via cfe-commits]

https://github.com/Mick235711 updated 
https://github.com/llvm/llvm-project/pull/112521

>From 10f7bb8cd7d6eef26065d20437dc126cd4e7138c Mon Sep 17 00:00:00 2001
From: Yihe Li 
Date: Wed, 16 Oct 2024 18:53:04 +0800
Subject: [PATCH] [clang] Improve diagnostic on [[nodiscard]] attribute

---
 clang/docs/ReleaseNotes.rst   |   2 +
 clang/include/clang/AST/Expr.h|   8 +-
 .../clang/Basic/DiagnosticSemaKinds.td|   6 +
 clang/lib/AST/Expr.cpp|  18 +--
 clang/lib/Sema/SemaStmt.cpp   |  40 ---
 .../dcl.attr/dcl.attr.nodiscard/p2.cpp|  28 ++---
 .../dcl.attr/dcl.attr.nodiscard/p3.cpp|   2 +-
 clang/test/Sema/c2x-nodiscard.c   |   8 +-
 clang/test/SemaCXX/warn-unused-result.cpp | 111 ++
 9 files changed, 158 insertions(+), 65 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index b430b2b0ee3187..fde6e3413b6bb9 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -403,6 +403,8 @@ Improvements to Clang's diagnostics
   name was a reserved name, which we improperly allowed to suppress the
   diagnostic.
 
+- Clang now includes the return type of the function or constructor when 
[[nodiscard]] is triggered by its placement on return types instead of function 
itself.
+
 Improvements to Clang's time-trace
 --
 
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index cbe62411d11bff..592e1ef925796f 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -3181,12 +3181,14 @@ class CallExpr : public Expr {
   QualType getCallReturnType(const ASTContext &Ctx) const;
 
   /// Returns the WarnUnusedResultAttr that is either declared on the called
-  /// function, or its return type declaration.
-  const Attr *getUnusedResultAttr(const ASTContext &Ctx) const;
+  /// function, or its return type declaration, together with a NamedDecl that
+  /// refers to the declaration the attribute is attached onto.
+  std::pair
+  getUnusedResultAttr(const ASTContext &Ctx) const;
 
   /// Returns true if this call expression should warn on unused results.
   bool hasUnusedResultAttr(const ASTContext &Ctx) const {
-return getUnusedResultAttr(Ctx) != nullptr;
+return getUnusedResultAttr(Ctx).second != nullptr;
   }
 
   SourceLocation getRParenLoc() const { return RParenLoc; }
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c709795e7b21d8..f683bfe1df8dde 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9265,6 +9265,12 @@ def warn_unused_container_subscript_expr : Warning<
 def warn_unused_call : Warning<
   "ignoring return value of function declared with %0 attribute">,
   InGroup;
+def warn_unused_return_type : Warning<
+  "ignoring %select{return value|temporary}0 of type %2 declared with %1 
attribute">,
+  InGroup;
+def warn_unused_return_type_msg : Warning<
+  "ignoring %select{return value|temporary}0 of type %2 declared with %1 
attribute: %3">,
+  InGroup;
 def warn_unused_constructor : Warning<
   "ignoring temporary created by a constructor declared with %0 attribute">,
   InGroup;
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 9ecbf121e3fc0d..bbb05ec48523f5 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -1616,22 +1616,24 @@ QualType CallExpr::getCallReturnType(const ASTContext 
&Ctx) const {
   return FnType->getReturnType();
 }
 
-const Attr *CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
+std::pair
+CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
+  // If the callee is marked nodiscard, return that attribute
+  const Decl *D = getCalleeDecl();
+  if (const auto *A = D->getAttr())
+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 auto *A = TD->getAttr())
-  return A;
+  return {TD, A};
 
   for (const auto *TD = getCallReturnType(Ctx)->getAs(); TD;
TD = TD->desugar()->getAs())
 if (const auto *A = TD->getDecl()->getAttr())
-  return A;
-
-  // Otherwise, see if the callee is marked nodiscard and return that attribute
-  // instead.
-  const Decl *D = getCalleeDecl();
-  return D ? D->getAttr() : nullptr;
+  return {TD->getDecl(), A};
+  return {nullptr, nullptr};
 }
 
 SourceLocation CallExpr::getBeginLoc() const {
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 9e235a46707cd4..5895da9daaf22d 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -204,22 +204,28 @@ static bool DiagnoseUnusedComparison(Sema &S, const Expr 
*E) {
   return true;
 }
 
-static bool DiagnoseNoDiscard(Sem

[clang] [clang] Improve diagnostic on [[nodiscard]] attribute (PR #112521)

[Yihe Li via cfe-commits]

@@ -115,20 +115,20 @@ void usage() {
   S('A'); // expected-warning {{ignoring temporary created by a constructor 
declared with 'nodiscard' attribute: Don't let that S-Char go!}}
   S(1);
   S(2.2);
-  Y(); // expected-warning {{ignoring temporary created by a constructor 
declared with 'nodiscard' attribute: Don't throw me away either!}}
+  Y(); // expected-warning {{ignoring temporary of type 'Y' declared with 
'nodiscard' attribute: Don't throw me away either!}}
   S s;
-  ConvertTo{}; // expected-warning {{ignoring return value of function 
declared with 'nodiscard' attribute: Don't throw me away!}}
+  ConvertTo{}; // expected-warning {{ignoring return value of type 'ConvertTo' 
declared with 'nodiscard' attribute: Don't throw me away!}}

Mick235711 wrote:

Yeah, definitely because of that, since the following also gives two different 
warnings ([CE](https://godbolt.org/z/rPb5jaY1x)):
```cpp
struct [[nodiscard]] A {};
void use()
{
A();
A{};
}
```
This probably is not intended (I hope), but I'm not familiar enough with 
related infrastructure to suggest a fix. Either way, that is not in the scope 
of this PR.

https://github.com/llvm/llvm-project/pull/112521
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Improve diagnostic on [[nodiscard]] attribute (PR #112521)

[Yihe Li via cfe-commits]

https://github.com/Mick235711 updated 
https://github.com/llvm/llvm-project/pull/112521

>From 59f7dbdd8eed456b76e93f6260bf0e361242e9fd Mon Sep 17 00:00:00 2001
From: Yihe Li 
Date: Wed, 16 Oct 2024 18:53:04 +0800
Subject: [PATCH] [clang] Improve diagnostic on [[nodiscard]] attribute

---
 clang/docs/ReleaseNotes.rst   |   3 +
 clang/include/clang/AST/Expr.h|   8 +-
 .../clang/Basic/DiagnosticSemaKinds.td|   6 +
 clang/lib/AST/Expr.cpp|  18 +--
 clang/lib/Sema/SemaStmt.cpp   |  40 ---
 .../dcl.attr/dcl.attr.nodiscard/p2.cpp|  28 ++---
 .../dcl.attr/dcl.attr.nodiscard/p3.cpp|   2 +-
 clang/test/Sema/c2x-nodiscard.c   |   8 +-
 clang/test/SemaCXX/warn-unused-result.cpp | 111 ++
 9 files changed, 159 insertions(+), 65 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index dc5564b6db119f..5dd30569fad108 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -416,6 +416,9 @@ Improvements to Clang's diagnostics
   name was a reserved name, which we improperly allowed to suppress the
   diagnostic.
 
+- Clang now includes the return type of the function or constructor in the 
warning generated
+  when `[[nodiscard]]` is triggered by its placement on return types instead 
of function itself.
+
 Improvements to Clang's time-trace
 --
 
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index cbe62411d11bff..592e1ef925796f 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -3181,12 +3181,14 @@ class CallExpr : public Expr {
   QualType getCallReturnType(const ASTContext &Ctx) const;
 
   /// Returns the WarnUnusedResultAttr that is either declared on the called
-  /// function, or its return type declaration.
-  const Attr *getUnusedResultAttr(const ASTContext &Ctx) const;
+  /// function, or its return type declaration, together with a NamedDecl that
+  /// refers to the declaration the attribute is attached onto.
+  std::pair
+  getUnusedResultAttr(const ASTContext &Ctx) const;
 
   /// Returns true if this call expression should warn on unused results.
   bool hasUnusedResultAttr(const ASTContext &Ctx) const {
-return getUnusedResultAttr(Ctx) != nullptr;
+return getUnusedResultAttr(Ctx).second != nullptr;
   }
 
   SourceLocation getRParenLoc() const { return RParenLoc; }
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c458a62d9be48c..d22ceb0920b2ab 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9267,6 +9267,12 @@ def warn_unused_container_subscript_expr : Warning<
 def warn_unused_call : Warning<
   "ignoring return value of function declared with %0 attribute">,
   InGroup;
+def warn_unused_return_type : Warning<
+  "ignoring %select{return value|temporary}0 of type %2 declared with %1 
attribute">,
+  InGroup;
+def warn_unused_return_type_msg : Warning<
+  "ignoring %select{return value|temporary}0 of type %2 declared with %1 
attribute: %3">,
+  InGroup;
 def warn_unused_constructor : Warning<
   "ignoring temporary created by a constructor declared with %0 attribute">,
   InGroup;
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 9ecbf121e3fc0d..bbb05ec48523f5 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -1616,22 +1616,24 @@ QualType CallExpr::getCallReturnType(const ASTContext 
&Ctx) const {
   return FnType->getReturnType();
 }
 
-const Attr *CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
+std::pair
+CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
+  // If the callee is marked nodiscard, return that attribute
+  const Decl *D = getCalleeDecl();
+  if (const auto *A = D->getAttr())
+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 auto *A = TD->getAttr())
-  return A;
+  return {TD, A};
 
   for (const auto *TD = getCallReturnType(Ctx)->getAs(); TD;
TD = TD->desugar()->getAs())
 if (const auto *A = TD->getDecl()->getAttr())
-  return A;
-
-  // Otherwise, see if the callee is marked nodiscard and return that attribute
-  // instead.
-  const Decl *D = getCalleeDecl();
-  return D ? D->getAttr() : nullptr;
+  return {TD->getDecl(), A};
+  return {nullptr, nullptr};
 }
 
 SourceLocation CallExpr::getBeginLoc() const {
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 9e235a46707cd4..5895da9daaf22d 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -204,22 +204,28 @@ static bool DiagnoseUnusedComparison(Sema &S, const Expr 
*E) {
   return true;
 }
 
-sta

[clang] [clang] Improve diagnostic on [[nodiscard]] attribute (PR #112521)

[Yihe Li via cfe-commits]

https://github.com/Mick235711 updated 
https://github.com/llvm/llvm-project/pull/112521

>From e69fcc089b8a0cb2d6c420f829ad74bd26a8ad21 Mon Sep 17 00:00:00 2001
From: Yihe Li 
Date: Wed, 16 Oct 2024 18:53:04 +0800
Subject: [PATCH] [clang] Improve diagnostic on [[nodiscard]] attribute

---
 clang/docs/ReleaseNotes.rst   |   2 +
 clang/include/clang/AST/Expr.h|   8 +-
 .../clang/Basic/DiagnosticSemaKinds.td|   6 +
 clang/lib/AST/Expr.cpp|  18 +--
 clang/lib/Sema/SemaStmt.cpp   |  40 ---
 .../dcl.attr/dcl.attr.nodiscard/p2.cpp|  28 ++---
 .../dcl.attr/dcl.attr.nodiscard/p3.cpp|   2 +-
 clang/test/Sema/c2x-nodiscard.c   |   8 +-
 clang/test/SemaCXX/warn-unused-result.cpp | 111 ++
 9 files changed, 158 insertions(+), 65 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index dc5564b6db119f..7197f38f2890dc 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -416,6 +416,8 @@ Improvements to Clang's diagnostics
   name was a reserved name, which we improperly allowed to suppress the
   diagnostic.
 
+- Clang now includes the return type of the function or constructor when 
[[nodiscard]] is triggered by its placement on return types instead of function 
itself.
+
 Improvements to Clang's time-trace
 --
 
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index cbe62411d11bff..592e1ef925796f 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -3181,12 +3181,14 @@ class CallExpr : public Expr {
   QualType getCallReturnType(const ASTContext &Ctx) const;
 
   /// Returns the WarnUnusedResultAttr that is either declared on the called
-  /// function, or its return type declaration.
-  const Attr *getUnusedResultAttr(const ASTContext &Ctx) const;
+  /// function, or its return type declaration, together with a NamedDecl that
+  /// refers to the declaration the attribute is attached onto.
+  std::pair
+  getUnusedResultAttr(const ASTContext &Ctx) const;
 
   /// Returns true if this call expression should warn on unused results.
   bool hasUnusedResultAttr(const ASTContext &Ctx) const {
-return getUnusedResultAttr(Ctx) != nullptr;
+return getUnusedResultAttr(Ctx).second != nullptr;
   }
 
   SourceLocation getRParenLoc() const { return RParenLoc; }
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c458a62d9be48c..d22ceb0920b2ab 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9267,6 +9267,12 @@ def warn_unused_container_subscript_expr : Warning<
 def warn_unused_call : Warning<
   "ignoring return value of function declared with %0 attribute">,
   InGroup;
+def warn_unused_return_type : Warning<
+  "ignoring %select{return value|temporary}0 of type %2 declared with %1 
attribute">,
+  InGroup;
+def warn_unused_return_type_msg : Warning<
+  "ignoring %select{return value|temporary}0 of type %2 declared with %1 
attribute: %3">,
+  InGroup;
 def warn_unused_constructor : Warning<
   "ignoring temporary created by a constructor declared with %0 attribute">,
   InGroup;
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 9ecbf121e3fc0d..bbb05ec48523f5 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -1616,22 +1616,24 @@ QualType CallExpr::getCallReturnType(const ASTContext 
&Ctx) const {
   return FnType->getReturnType();
 }
 
-const Attr *CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
+std::pair
+CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
+  // If the callee is marked nodiscard, return that attribute
+  const Decl *D = getCalleeDecl();
+  if (const auto *A = D->getAttr())
+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 auto *A = TD->getAttr())
-  return A;
+  return {TD, A};
 
   for (const auto *TD = getCallReturnType(Ctx)->getAs(); TD;
TD = TD->desugar()->getAs())
 if (const auto *A = TD->getDecl()->getAttr())
-  return A;
-
-  // Otherwise, see if the callee is marked nodiscard and return that attribute
-  // instead.
-  const Decl *D = getCalleeDecl();
-  return D ? D->getAttr() : nullptr;
+  return {TD->getDecl(), A};
+  return {nullptr, nullptr};
 }
 
 SourceLocation CallExpr::getBeginLoc() const {
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 9e235a46707cd4..5895da9daaf22d 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -204,22 +204,28 @@ static bool DiagnoseUnusedComparison(Sema &S, const Expr 
*E) {
   return true;
 }
 
-static bool DiagnoseNoDiscard(Sem

[clang] [clang] Improve diagnostic on [[nodiscard]] attribute (PR #112521)

[Yihe Li via cfe-commits]

https://github.com/Mick235711 updated 
https://github.com/llvm/llvm-project/pull/112521

>From 35be44832bdc81ddbdeaa98500e65ec1de0cc049 Mon Sep 17 00:00:00 2001
From: Yihe Li 
Date: Wed, 16 Oct 2024 18:53:04 +0800
Subject: [PATCH] [clang] Improve diagnostic on [[nodiscard]] attribute

---
 clang/docs/ReleaseNotes.rst   |   2 +
 clang/include/clang/AST/Expr.h|   8 +-
 .../clang/Basic/DiagnosticSemaKinds.td|   6 +
 clang/lib/AST/Expr.cpp|  18 +--
 clang/lib/Sema/SemaStmt.cpp   |  40 ---
 .../dcl.attr/dcl.attr.nodiscard/p2.cpp|  28 ++---
 .../dcl.attr/dcl.attr.nodiscard/p3.cpp|   2 +-
 clang/test/Sema/c2x-nodiscard.c   |   8 +-
 clang/test/SemaCXX/warn-unused-result.cpp | 111 ++
 9 files changed, 158 insertions(+), 65 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index dc5564b6db119f..7197f38f2890dc 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -416,6 +416,8 @@ Improvements to Clang's diagnostics
   name was a reserved name, which we improperly allowed to suppress the
   diagnostic.
 
+- Clang now includes the return type of the function or constructor when 
[[nodiscard]] is triggered by its placement on return types instead of function 
itself.
+
 Improvements to Clang's time-trace
 --
 
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index cbe62411d11bff..592e1ef925796f 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -3181,12 +3181,14 @@ class CallExpr : public Expr {
   QualType getCallReturnType(const ASTContext &Ctx) const;
 
   /// Returns the WarnUnusedResultAttr that is either declared on the called
-  /// function, or its return type declaration.
-  const Attr *getUnusedResultAttr(const ASTContext &Ctx) const;
+  /// function, or its return type declaration, together with a NamedDecl that
+  /// refers to the declaration the attribute is attached onto.
+  std::pair
+  getUnusedResultAttr(const ASTContext &Ctx) const;
 
   /// Returns true if this call expression should warn on unused results.
   bool hasUnusedResultAttr(const ASTContext &Ctx) const {
-return getUnusedResultAttr(Ctx) != nullptr;
+return getUnusedResultAttr(Ctx).second != nullptr;
   }
 
   SourceLocation getRParenLoc() const { return RParenLoc; }
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c458a62d9be48c..d22ceb0920b2ab 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9267,6 +9267,12 @@ def warn_unused_container_subscript_expr : Warning<
 def warn_unused_call : Warning<
   "ignoring return value of function declared with %0 attribute">,
   InGroup;
+def warn_unused_return_type : Warning<
+  "ignoring %select{return value|temporary}0 of type %2 declared with %1 
attribute">,
+  InGroup;
+def warn_unused_return_type_msg : Warning<
+  "ignoring %select{return value|temporary}0 of type %2 declared with %1 
attribute: %3">,
+  InGroup;
 def warn_unused_constructor : Warning<
   "ignoring temporary created by a constructor declared with %0 attribute">,
   InGroup;
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 9ecbf121e3fc0d..bbb05ec48523f5 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -1616,22 +1616,24 @@ QualType CallExpr::getCallReturnType(const ASTContext 
&Ctx) const {
   return FnType->getReturnType();
 }
 
-const Attr *CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
+std::pair
+CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
+  // If the callee is marked nodiscard, return that attribute
+  const Decl *D = getCalleeDecl();
+  if (const auto *A = D->getAttr())
+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 auto *A = TD->getAttr())
-  return A;
+  return {TD, A};
 
   for (const auto *TD = getCallReturnType(Ctx)->getAs(); TD;
TD = TD->desugar()->getAs())
 if (const auto *A = TD->getDecl()->getAttr())
-  return A;
-
-  // Otherwise, see if the callee is marked nodiscard and return that attribute
-  // instead.
-  const Decl *D = getCalleeDecl();
-  return D ? D->getAttr() : nullptr;
+  return {TD->getDecl(), A};
+  return {nullptr, nullptr};
 }
 
 SourceLocation CallExpr::getBeginLoc() const {
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 9e235a46707cd4..5895da9daaf22d 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -204,22 +204,28 @@ static bool DiagnoseUnusedComparison(Sema &S, const Expr 
*E) {
   return true;
 }
 
-static bool DiagnoseNoDiscard(Sem

[clang] [clang] Improve diagnostic on [[nodiscard]] attribute (PR #112521)

[Yihe Li via cfe-commits]

Mick235711 wrote:

Release note added.

Incidentally, this also fixes an inconsistency for Clang compared to GCC/MSVC: 
In the following program
```cpp
struct [[nodiscard("Reason 1")]] S {};
[[nodiscard("Reason 2")]] S getS();

int main()
{
getS();
}
```
My opinion is that the function's attribute should take precedence since it is 
"more specific". GCC/MSVC trunk agrees with me and prints Reason 2 in warning, 
while Clang trunk prints Reason 1. ([Compiler 
Explorer](https://godbolt.org/z/4jd1zK31j)). After this PR, Clang will also 
print Reason 2 here.

Also, the same CE link also explores the note generation on `nodiscard` vs 
`deprecated`, which was the motivation for the original PR #112289. Here, GCC 
is the most verbose, generating 1 warning + 2 notes (one on declaration of 
function and one on declaration of type), when the function's return type is 
marked as `nodiscard`. 1 warning + 1 note is generated for both `deprecated` 
and normal `nodiscard` on the function itself. MSVC on the other hand just 
generates 1 warning (no note) for `nodiscard` regardless of placement, and 
doesn't seem to implemented `deprecated` at all. (This stats is only FYI)

https://github.com/llvm/llvm-project/pull/112521
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Improve diagnostic on [[nodiscard]] attribute (PR #112521)

[Yihe Li via cfe-commits]

@@ -416,6 +416,9 @@ Improvements to Clang's diagnostics
   name was a reserved name, which we improperly allowed to suppress the
   diagnostic.
 
+- Clang now includes the return type of the function or constructor in the 
warning generated
+  when `[[nodiscard]]` is triggered by its placement on return types instead 
of function itself.

Mick235711 wrote:

Sure

https://github.com/llvm/llvm-project/pull/112521
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Improve diagnostic on [[nodiscard]] attribute (PR #112521)

[Yihe Li via cfe-commits]

Mick235711 wrote:

> Side note: please try to avoid force-pushing in the future as that makes it 
> really annoying to try and figure out what has changed since the last review. 
> We always squash on merge anyway, so a clean commit history on a pr is 
> irrelevant.
Oh, sorry about that, I don't usually force push, but I assumed that LLVM PRs 
should be one-commit only when I see a similarly-worded line in the 
contributing guidelines. My bad.

https://github.com/llvm/llvm-project/pull/112521
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Improve diagnostic on [[nodiscard]] attribute (PR #112521)

[Yihe Li via cfe-commits]

https://github.com/Mick235711 updated 
https://github.com/llvm/llvm-project/pull/112521

>From 59f7dbdd8eed456b76e93f6260bf0e361242e9fd Mon Sep 17 00:00:00 2001
From: Yihe Li 
Date: Wed, 16 Oct 2024 18:53:04 +0800
Subject: [PATCH 1/2] [clang] Improve diagnostic on [[nodiscard]] attribute

---
 clang/docs/ReleaseNotes.rst   |   3 +
 clang/include/clang/AST/Expr.h|   8 +-
 .../clang/Basic/DiagnosticSemaKinds.td|   6 +
 clang/lib/AST/Expr.cpp|  18 +--
 clang/lib/Sema/SemaStmt.cpp   |  40 ---
 .../dcl.attr/dcl.attr.nodiscard/p2.cpp|  28 ++---
 .../dcl.attr/dcl.attr.nodiscard/p3.cpp|   2 +-
 clang/test/Sema/c2x-nodiscard.c   |   8 +-
 clang/test/SemaCXX/warn-unused-result.cpp | 111 ++
 9 files changed, 159 insertions(+), 65 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index dc5564b6db119f..5dd30569fad108 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -416,6 +416,9 @@ Improvements to Clang's diagnostics
   name was a reserved name, which we improperly allowed to suppress the
   diagnostic.
 
+- Clang now includes the return type of the function or constructor in the 
warning generated
+  when `[[nodiscard]]` is triggered by its placement on return types instead 
of function itself.
+
 Improvements to Clang's time-trace
 --
 
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index cbe62411d11bff..592e1ef925796f 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -3181,12 +3181,14 @@ class CallExpr : public Expr {
   QualType getCallReturnType(const ASTContext &Ctx) const;
 
   /// Returns the WarnUnusedResultAttr that is either declared on the called
-  /// function, or its return type declaration.
-  const Attr *getUnusedResultAttr(const ASTContext &Ctx) const;
+  /// function, or its return type declaration, together with a NamedDecl that
+  /// refers to the declaration the attribute is attached onto.
+  std::pair
+  getUnusedResultAttr(const ASTContext &Ctx) const;
 
   /// Returns true if this call expression should warn on unused results.
   bool hasUnusedResultAttr(const ASTContext &Ctx) const {
-return getUnusedResultAttr(Ctx) != nullptr;
+return getUnusedResultAttr(Ctx).second != nullptr;
   }
 
   SourceLocation getRParenLoc() const { return RParenLoc; }
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c458a62d9be48c..d22ceb0920b2ab 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9267,6 +9267,12 @@ def warn_unused_container_subscript_expr : Warning<
 def warn_unused_call : Warning<
   "ignoring return value of function declared with %0 attribute">,
   InGroup;
+def warn_unused_return_type : Warning<
+  "ignoring %select{return value|temporary}0 of type %2 declared with %1 
attribute">,
+  InGroup;
+def warn_unused_return_type_msg : Warning<
+  "ignoring %select{return value|temporary}0 of type %2 declared with %1 
attribute: %3">,
+  InGroup;
 def warn_unused_constructor : Warning<
   "ignoring temporary created by a constructor declared with %0 attribute">,
   InGroup;
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 9ecbf121e3fc0d..bbb05ec48523f5 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -1616,22 +1616,24 @@ QualType CallExpr::getCallReturnType(const ASTContext 
&Ctx) const {
   return FnType->getReturnType();
 }
 
-const Attr *CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
+std::pair
+CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
+  // If the callee is marked nodiscard, return that attribute
+  const Decl *D = getCalleeDecl();
+  if (const auto *A = D->getAttr())
+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 auto *A = TD->getAttr())
-  return A;
+  return {TD, A};
 
   for (const auto *TD = getCallReturnType(Ctx)->getAs(); TD;
TD = TD->desugar()->getAs())
 if (const auto *A = TD->getDecl()->getAttr())
-  return A;
-
-  // Otherwise, see if the callee is marked nodiscard and return that attribute
-  // instead.
-  const Decl *D = getCalleeDecl();
-  return D ? D->getAttr() : nullptr;
+  return {TD->getDecl(), A};
+  return {nullptr, nullptr};
 }
 
 SourceLocation CallExpr::getBeginLoc() const {
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 9e235a46707cd4..5895da9daaf22d 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -204,22 +204,28 @@ static bool DiagnoseUnusedComparison(Sema &S, const Expr 
*E) {
   return true;
 }
 

[clang] [clang] Generate note on declaration for nodiscard-related attributes (PR #112289)

[Yihe Li via cfe-commits]

https://github.com/Mick235711 closed 
https://github.com/llvm/llvm-project/pull/112289
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Improve diagnostic on [[nodiscard]] attribute (PR #112521)

[Yihe Li via cfe-commits]

https://github.com/Mick235711 updated 
https://github.com/llvm/llvm-project/pull/112521

>From 59f7dbdd8eed456b76e93f6260bf0e361242e9fd Mon Sep 17 00:00:00 2001
From: Yihe Li 
Date: Wed, 16 Oct 2024 18:53:04 +0800
Subject: [PATCH 1/3] [clang] Improve diagnostic on [[nodiscard]] attribute

---
 clang/docs/ReleaseNotes.rst   |   3 +
 clang/include/clang/AST/Expr.h|   8 +-
 .../clang/Basic/DiagnosticSemaKinds.td|   6 +
 clang/lib/AST/Expr.cpp|  18 +--
 clang/lib/Sema/SemaStmt.cpp   |  40 ---
 .../dcl.attr/dcl.attr.nodiscard/p2.cpp|  28 ++---
 .../dcl.attr/dcl.attr.nodiscard/p3.cpp|   2 +-
 clang/test/Sema/c2x-nodiscard.c   |   8 +-
 clang/test/SemaCXX/warn-unused-result.cpp | 111 ++
 9 files changed, 159 insertions(+), 65 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index dc5564b6db119f..5dd30569fad108 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -416,6 +416,9 @@ Improvements to Clang's diagnostics
   name was a reserved name, which we improperly allowed to suppress the
   diagnostic.
 
+- Clang now includes the return type of the function or constructor in the 
warning generated
+  when `[[nodiscard]]` is triggered by its placement on return types instead 
of function itself.
+
 Improvements to Clang's time-trace
 --
 
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index cbe62411d11bff..592e1ef925796f 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -3181,12 +3181,14 @@ class CallExpr : public Expr {
   QualType getCallReturnType(const ASTContext &Ctx) const;
 
   /// Returns the WarnUnusedResultAttr that is either declared on the called
-  /// function, or its return type declaration.
-  const Attr *getUnusedResultAttr(const ASTContext &Ctx) const;
+  /// function, or its return type declaration, together with a NamedDecl that
+  /// refers to the declaration the attribute is attached onto.
+  std::pair
+  getUnusedResultAttr(const ASTContext &Ctx) const;
 
   /// Returns true if this call expression should warn on unused results.
   bool hasUnusedResultAttr(const ASTContext &Ctx) const {
-return getUnusedResultAttr(Ctx) != nullptr;
+return getUnusedResultAttr(Ctx).second != nullptr;
   }
 
   SourceLocation getRParenLoc() const { return RParenLoc; }
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c458a62d9be48c..d22ceb0920b2ab 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9267,6 +9267,12 @@ def warn_unused_container_subscript_expr : Warning<
 def warn_unused_call : Warning<
   "ignoring return value of function declared with %0 attribute">,
   InGroup;
+def warn_unused_return_type : Warning<
+  "ignoring %select{return value|temporary}0 of type %2 declared with %1 
attribute">,
+  InGroup;
+def warn_unused_return_type_msg : Warning<
+  "ignoring %select{return value|temporary}0 of type %2 declared with %1 
attribute: %3">,
+  InGroup;
 def warn_unused_constructor : Warning<
   "ignoring temporary created by a constructor declared with %0 attribute">,
   InGroup;
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 9ecbf121e3fc0d..bbb05ec48523f5 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -1616,22 +1616,24 @@ QualType CallExpr::getCallReturnType(const ASTContext 
&Ctx) const {
   return FnType->getReturnType();
 }
 
-const Attr *CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
+std::pair
+CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
+  // If the callee is marked nodiscard, return that attribute
+  const Decl *D = getCalleeDecl();
+  if (const auto *A = D->getAttr())
+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 auto *A = TD->getAttr())
-  return A;
+  return {TD, A};
 
   for (const auto *TD = getCallReturnType(Ctx)->getAs(); TD;
TD = TD->desugar()->getAs())
 if (const auto *A = TD->getDecl()->getAttr())
-  return A;
-
-  // Otherwise, see if the callee is marked nodiscard and return that attribute
-  // instead.
-  const Decl *D = getCalleeDecl();
-  return D ? D->getAttr() : nullptr;
+  return {TD->getDecl(), A};
+  return {nullptr, nullptr};
 }
 
 SourceLocation CallExpr::getBeginLoc() const {
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 9e235a46707cd4..5895da9daaf22d 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -204,22 +204,28 @@ static bool DiagnoseUnusedComparison(Sema &S, const Expr 
*E) {
   return true;
 }
 

[clang] [clang] Improve diagnostic on [[nodiscard]] attribute (PR #112521)

[Yihe Li via cfe-commits]

@@ -9300,6 +9300,12 @@ def warn_unused_container_subscript_expr : Warning<
 def warn_unused_call : Warning<
   "ignoring return value of function declared with %0 attribute">,
   InGroup;
+def warn_unused_return_type : Warning<
+  "ignoring %select{return value|temporary}0 of type %2 declared with %1 
attribute">,
+  InGroup;
+def warn_unused_return_type_msg : Warning<

Mick235711 wrote:

I originally wanted to do this, but existing `warn_unused_constructor` and 
`warn_unused_result` all have a separate `_msg` version... If we fold together 
the two `unused_return_type`s, should these two be folded together too?

https://github.com/llvm/llvm-project/pull/112521
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Improve diagnostic on [[nodiscard]] attribute (PR #112521)

[Yihe Li via cfe-commits]

https://github.com/Mick235711 updated 
https://github.com/llvm/llvm-project/pull/112521

>From 59f7dbdd8eed456b76e93f6260bf0e361242e9fd Mon Sep 17 00:00:00 2001
From: Yihe Li 
Date: Wed, 16 Oct 2024 18:53:04 +0800
Subject: [PATCH 1/4] [clang] Improve diagnostic on [[nodiscard]] attribute

---
 clang/docs/ReleaseNotes.rst   |   3 +
 clang/include/clang/AST/Expr.h|   8 +-
 .../clang/Basic/DiagnosticSemaKinds.td|   6 +
 clang/lib/AST/Expr.cpp|  18 +--
 clang/lib/Sema/SemaStmt.cpp   |  40 ---
 .../dcl.attr/dcl.attr.nodiscard/p2.cpp|  28 ++---
 .../dcl.attr/dcl.attr.nodiscard/p3.cpp|   2 +-
 clang/test/Sema/c2x-nodiscard.c   |   8 +-
 clang/test/SemaCXX/warn-unused-result.cpp | 111 ++
 9 files changed, 159 insertions(+), 65 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index dc5564b6db119f..5dd30569fad108 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -416,6 +416,9 @@ Improvements to Clang's diagnostics
   name was a reserved name, which we improperly allowed to suppress the
   diagnostic.
 
+- Clang now includes the return type of the function or constructor in the 
warning generated
+  when `[[nodiscard]]` is triggered by its placement on return types instead 
of function itself.
+
 Improvements to Clang's time-trace
 --
 
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index cbe62411d11bff..592e1ef925796f 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -3181,12 +3181,14 @@ class CallExpr : public Expr {
   QualType getCallReturnType(const ASTContext &Ctx) const;
 
   /// Returns the WarnUnusedResultAttr that is either declared on the called
-  /// function, or its return type declaration.
-  const Attr *getUnusedResultAttr(const ASTContext &Ctx) const;
+  /// function, or its return type declaration, together with a NamedDecl that
+  /// refers to the declaration the attribute is attached onto.
+  std::pair
+  getUnusedResultAttr(const ASTContext &Ctx) const;
 
   /// Returns true if this call expression should warn on unused results.
   bool hasUnusedResultAttr(const ASTContext &Ctx) const {
-return getUnusedResultAttr(Ctx) != nullptr;
+return getUnusedResultAttr(Ctx).second != nullptr;
   }
 
   SourceLocation getRParenLoc() const { return RParenLoc; }
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c458a62d9be48c..d22ceb0920b2ab 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9267,6 +9267,12 @@ def warn_unused_container_subscript_expr : Warning<
 def warn_unused_call : Warning<
   "ignoring return value of function declared with %0 attribute">,
   InGroup;
+def warn_unused_return_type : Warning<
+  "ignoring %select{return value|temporary}0 of type %2 declared with %1 
attribute">,
+  InGroup;
+def warn_unused_return_type_msg : Warning<
+  "ignoring %select{return value|temporary}0 of type %2 declared with %1 
attribute: %3">,
+  InGroup;
 def warn_unused_constructor : Warning<
   "ignoring temporary created by a constructor declared with %0 attribute">,
   InGroup;
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 9ecbf121e3fc0d..bbb05ec48523f5 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -1616,22 +1616,24 @@ QualType CallExpr::getCallReturnType(const ASTContext 
&Ctx) const {
   return FnType->getReturnType();
 }
 
-const Attr *CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
+std::pair
+CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
+  // If the callee is marked nodiscard, return that attribute
+  const Decl *D = getCalleeDecl();
+  if (const auto *A = D->getAttr())
+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 auto *A = TD->getAttr())
-  return A;
+  return {TD, A};
 
   for (const auto *TD = getCallReturnType(Ctx)->getAs(); TD;
TD = TD->desugar()->getAs())
 if (const auto *A = TD->getDecl()->getAttr())
-  return A;
-
-  // Otherwise, see if the callee is marked nodiscard and return that attribute
-  // instead.
-  const Decl *D = getCalleeDecl();
-  return D ? D->getAttr() : nullptr;
+  return {TD->getDecl(), A};
+  return {nullptr, nullptr};
 }
 
 SourceLocation CallExpr::getBeginLoc() const {
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 9e235a46707cd4..5895da9daaf22d 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -204,22 +204,28 @@ static bool DiagnoseUnusedComparison(Sema &S, const Expr 
*E) {
   return true;
 }
 

[clang] [clang] Improve diagnostic on [[nodiscard]] attribute (PR #112521)

[Yihe Li via cfe-commits]

@@ -9300,6 +9300,12 @@ def warn_unused_container_subscript_expr : Warning<
 def warn_unused_call : Warning<
   "ignoring return value of function declared with %0 attribute">,
   InGroup;
+def warn_unused_return_type : Warning<
+  "ignoring %select{return value|temporary}0 of type %2 declared with %1 
attribute">,
+  InGroup;
+def warn_unused_return_type_msg : Warning<

Mick235711 wrote:

Sure, I will collapse all three then.

https://github.com/llvm/llvm-project/pull/112521
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Improve diagnostic on [[nodiscard]] attribute (PR #112521)

[Yihe Li via cfe-commits]

https://github.com/Mick235711 updated 
https://github.com/llvm/llvm-project/pull/112521

>From 59f7dbdd8eed456b76e93f6260bf0e361242e9fd Mon Sep 17 00:00:00 2001
From: Yihe Li 
Date: Wed, 16 Oct 2024 18:53:04 +0800
Subject: [PATCH 1/5] [clang] Improve diagnostic on [[nodiscard]] attribute

---
 clang/docs/ReleaseNotes.rst   |   3 +
 clang/include/clang/AST/Expr.h|   8 +-
 .../clang/Basic/DiagnosticSemaKinds.td|   6 +
 clang/lib/AST/Expr.cpp|  18 +--
 clang/lib/Sema/SemaStmt.cpp   |  40 ---
 .../dcl.attr/dcl.attr.nodiscard/p2.cpp|  28 ++---
 .../dcl.attr/dcl.attr.nodiscard/p3.cpp|   2 +-
 clang/test/Sema/c2x-nodiscard.c   |   8 +-
 clang/test/SemaCXX/warn-unused-result.cpp | 111 ++
 9 files changed, 159 insertions(+), 65 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index dc5564b6db119f..5dd30569fad108 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -416,6 +416,9 @@ Improvements to Clang's diagnostics
   name was a reserved name, which we improperly allowed to suppress the
   diagnostic.
 
+- Clang now includes the return type of the function or constructor in the 
warning generated
+  when `[[nodiscard]]` is triggered by its placement on return types instead 
of function itself.
+
 Improvements to Clang's time-trace
 --
 
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index cbe62411d11bff..592e1ef925796f 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -3181,12 +3181,14 @@ class CallExpr : public Expr {
   QualType getCallReturnType(const ASTContext &Ctx) const;
 
   /// Returns the WarnUnusedResultAttr that is either declared on the called
-  /// function, or its return type declaration.
-  const Attr *getUnusedResultAttr(const ASTContext &Ctx) const;
+  /// function, or its return type declaration, together with a NamedDecl that
+  /// refers to the declaration the attribute is attached onto.
+  std::pair
+  getUnusedResultAttr(const ASTContext &Ctx) const;
 
   /// Returns true if this call expression should warn on unused results.
   bool hasUnusedResultAttr(const ASTContext &Ctx) const {
-return getUnusedResultAttr(Ctx) != nullptr;
+return getUnusedResultAttr(Ctx).second != nullptr;
   }
 
   SourceLocation getRParenLoc() const { return RParenLoc; }
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c458a62d9be48c..d22ceb0920b2ab 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9267,6 +9267,12 @@ def warn_unused_container_subscript_expr : Warning<
 def warn_unused_call : Warning<
   "ignoring return value of function declared with %0 attribute">,
   InGroup;
+def warn_unused_return_type : Warning<
+  "ignoring %select{return value|temporary}0 of type %2 declared with %1 
attribute">,
+  InGroup;
+def warn_unused_return_type_msg : Warning<
+  "ignoring %select{return value|temporary}0 of type %2 declared with %1 
attribute: %3">,
+  InGroup;
 def warn_unused_constructor : Warning<
   "ignoring temporary created by a constructor declared with %0 attribute">,
   InGroup;
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 9ecbf121e3fc0d..bbb05ec48523f5 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -1616,22 +1616,24 @@ QualType CallExpr::getCallReturnType(const ASTContext 
&Ctx) const {
   return FnType->getReturnType();
 }
 
-const Attr *CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
+std::pair
+CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
+  // If the callee is marked nodiscard, return that attribute
+  const Decl *D = getCalleeDecl();
+  if (const auto *A = D->getAttr())
+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 auto *A = TD->getAttr())
-  return A;
+  return {TD, A};
 
   for (const auto *TD = getCallReturnType(Ctx)->getAs(); TD;
TD = TD->desugar()->getAs())
 if (const auto *A = TD->getDecl()->getAttr())
-  return A;
-
-  // Otherwise, see if the callee is marked nodiscard and return that attribute
-  // instead.
-  const Decl *D = getCalleeDecl();
-  return D ? D->getAttr() : nullptr;
+  return {TD->getDecl(), A};
+  return {nullptr, nullptr};
 }
 
 SourceLocation CallExpr::getBeginLoc() const {
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 9e235a46707cd4..5895da9daaf22d 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -204,22 +204,28 @@ static bool DiagnoseUnusedComparison(Sema &S, const Expr 
*E) {
   return true;
 }
 

[clang] [clang] Improve diagnostic on [[nodiscard]] attribute (PR #112521)

[Yihe Li via cfe-commits]

@@ -148,6 +148,19 @@ C++ Specific Potentially Breaking Changes
 // Now diagnoses with an error.
 void f(int& i [[clang::lifetimebound]]);
 
+- Clang will now prefer the ``[[nodiscard]]`` declaration on function 
declarations over ``[[nodiscard]]``

Mick235711 wrote:

On further thought, indeed the whole PR does change the diagnostics message 
anyway, so indeed it should also be in the diagnostics section.

https://github.com/llvm/llvm-project/pull/112521
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Improve diagnostic on [[nodiscard]] attribute (PR #112521)

[Yihe Li via cfe-commits]

https://github.com/Mick235711 updated 
https://github.com/llvm/llvm-project/pull/112521

>From 59f7dbdd8eed456b76e93f6260bf0e361242e9fd Mon Sep 17 00:00:00 2001
From: Yihe Li 
Date: Wed, 16 Oct 2024 18:53:04 +0800
Subject: [PATCH 1/2] [clang] Improve diagnostic on [[nodiscard]] attribute

---
 clang/docs/ReleaseNotes.rst   |   3 +
 clang/include/clang/AST/Expr.h|   8 +-
 .../clang/Basic/DiagnosticSemaKinds.td|   6 +
 clang/lib/AST/Expr.cpp|  18 +--
 clang/lib/Sema/SemaStmt.cpp   |  40 ---
 .../dcl.attr/dcl.attr.nodiscard/p2.cpp|  28 ++---
 .../dcl.attr/dcl.attr.nodiscard/p3.cpp|   2 +-
 clang/test/Sema/c2x-nodiscard.c   |   8 +-
 clang/test/SemaCXX/warn-unused-result.cpp | 111 ++
 9 files changed, 159 insertions(+), 65 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index dc5564b6db119f..5dd30569fad108 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -416,6 +416,9 @@ Improvements to Clang's diagnostics
   name was a reserved name, which we improperly allowed to suppress the
   diagnostic.
 
+- Clang now includes the return type of the function or constructor in the 
warning generated
+  when `[[nodiscard]]` is triggered by its placement on return types instead 
of function itself.
+
 Improvements to Clang's time-trace
 --
 
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index cbe62411d11bff..592e1ef925796f 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -3181,12 +3181,14 @@ class CallExpr : public Expr {
   QualType getCallReturnType(const ASTContext &Ctx) const;
 
   /// Returns the WarnUnusedResultAttr that is either declared on the called
-  /// function, or its return type declaration.
-  const Attr *getUnusedResultAttr(const ASTContext &Ctx) const;
+  /// function, or its return type declaration, together with a NamedDecl that
+  /// refers to the declaration the attribute is attached onto.
+  std::pair
+  getUnusedResultAttr(const ASTContext &Ctx) const;
 
   /// Returns true if this call expression should warn on unused results.
   bool hasUnusedResultAttr(const ASTContext &Ctx) const {
-return getUnusedResultAttr(Ctx) != nullptr;
+return getUnusedResultAttr(Ctx).second != nullptr;
   }
 
   SourceLocation getRParenLoc() const { return RParenLoc; }
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c458a62d9be48c..d22ceb0920b2ab 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9267,6 +9267,12 @@ def warn_unused_container_subscript_expr : Warning<
 def warn_unused_call : Warning<
   "ignoring return value of function declared with %0 attribute">,
   InGroup;
+def warn_unused_return_type : Warning<
+  "ignoring %select{return value|temporary}0 of type %2 declared with %1 
attribute">,
+  InGroup;
+def warn_unused_return_type_msg : Warning<
+  "ignoring %select{return value|temporary}0 of type %2 declared with %1 
attribute: %3">,
+  InGroup;
 def warn_unused_constructor : Warning<
   "ignoring temporary created by a constructor declared with %0 attribute">,
   InGroup;
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 9ecbf121e3fc0d..bbb05ec48523f5 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -1616,22 +1616,24 @@ QualType CallExpr::getCallReturnType(const ASTContext 
&Ctx) const {
   return FnType->getReturnType();
 }
 
-const Attr *CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
+std::pair
+CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
+  // If the callee is marked nodiscard, return that attribute
+  const Decl *D = getCalleeDecl();
+  if (const auto *A = D->getAttr())
+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 auto *A = TD->getAttr())
-  return A;
+  return {TD, A};
 
   for (const auto *TD = getCallReturnType(Ctx)->getAs(); TD;
TD = TD->desugar()->getAs())
 if (const auto *A = TD->getDecl()->getAttr())
-  return A;
-
-  // Otherwise, see if the callee is marked nodiscard and return that attribute
-  // instead.
-  const Decl *D = getCalleeDecl();
-  return D ? D->getAttr() : nullptr;
+  return {TD->getDecl(), A};
+  return {nullptr, nullptr};
 }
 
 SourceLocation CallExpr::getBeginLoc() const {
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 9e235a46707cd4..5895da9daaf22d 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -204,22 +204,28 @@ static bool DiagnoseUnusedComparison(Sema &S, const Expr 
*E) {
   return true;
 }
 

[clang] [clang] Improve diagnostic on [[nodiscard]] attribute (PR #112521)

[Yihe Li via cfe-commits]

@@ -17,10 +17,10 @@ E get_e();
 // cxx11-warning@-1 {{use of the 'nodiscard' attribute is a C++17 extension}}
 
 void f() {
-  get_s(); // expected-warning {{ignoring return value of function declared 
with 'nodiscard' attribute}}
+  get_s(); // expected-warning {{ignoring return value of type 'S' declared 
with 'nodiscard' attribute}}

Mick235711 wrote:

Umm... the error message does say "ignoring return value of type 'S'", do you 
want me to remove that of in general?

https://github.com/llvm/llvm-project/pull/112521
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Improve diagnostic on [[nodiscard]] attribute (PR #112521)

[Yihe Li via cfe-commits]

@@ -1616,22 +1616,24 @@ QualType CallExpr::getCallReturnType(const ASTContext 
&Ctx) const {
   return FnType->getReturnType();
 }
 
-const Attr *CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
+std::pair
+CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
+  // If the callee is marked nodiscard, return that attribute
+  const Decl *D = getCalleeDecl();

Mick235711 wrote:

Yes, that is definitely observable, as I presented in 
https://github.com/llvm/llvm-project/pull/112521#issuecomment-2417847257. This 
actually fixed an inconsistency bug between Clang and GCC/MSVC, with the latter 
two already always preferring the callee decl.

I will add a release note for this.

https://github.com/llvm/llvm-project/pull/112521
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Improve diagnostic on [[nodiscard]] attribute (PR #112521)

[Yihe Li via cfe-commits]

Mick235711 wrote:

Gentle ping @erichkeane

https://github.com/llvm/llvm-project/pull/112521
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Improve diagnostic on [[nodiscard]] attribute (PR #112521)

[Yihe Li via cfe-commits]

@@ -1615,22 +1615,24 @@ QualType CallExpr::getCallReturnType(const ASTContext 
&Ctx) const {
   return FnType->getReturnType();
 }
 
-const Attr *CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
+std::pair
+CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
+  // If the callee is marked nodiscard, return that attribute
+  const Decl *D = getCalleeDecl();
+  if (const auto *A = D->getAttr())

Mick235711 wrote:

Indeed, it seems I forgot to add a check for `nullptr`.
Will open a PR to fix this later today.

https://github.com/llvm/llvm-project/pull/112521
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Fix missing check for nullptr in CallExpr::getUnusedResultAttr (PR #118636)

[Yihe Li via cfe-commits]

https://github.com/Mick235711 created 
https://github.com/llvm/llvm-project/pull/118636

Fixes #117975, a regression introduced by #112521 due to me forgetting to check 
for `nullptr` before dereferencing in `CallExpr::getUnusedResultAttr`.
A regression test has been added as per the comments on the fixed issue.

>From e81792ade5cda7afbba6ba161a3a9b9184065d82 Mon Sep 17 00:00:00 2001
From: Yihe Li 
Date: Wed, 4 Dec 2024 21:45:19 +0800
Subject: [PATCH] [clang] Fix missing check for nullptr in
 CallExpr::getUnusedResultAttr

---
 clang/lib/AST/Expr.cpp| 5 +++--
 clang/test/SemaCXX/warn-unused-result.cpp | 9 +
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index a4fb4d5a1f2ec4..286f02ded27196 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -1619,8 +1619,9 @@ std::pair
 CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
   // If the callee is marked nodiscard, return that attribute
   const Decl *D = getCalleeDecl();
-  if (const auto *A = D->getAttr())
-return {nullptr, A};
+  if (D != nullptr)
+if (const auto *A = D->getAttr())
+  return {nullptr, A};
 
   // If the return type is a struct, union, or enum that is marked nodiscard,
   // then return the return type attribute.
diff --git a/clang/test/SemaCXX/warn-unused-result.cpp 
b/clang/test/SemaCXX/warn-unused-result.cpp
index 682c500dc1d96d..5105f347db8b53 100644
--- a/clang/test/SemaCXX/warn-unused-result.cpp
+++ b/clang/test/SemaCXX/warn-unused-result.cpp
@@ -355,3 +355,12 @@ void use2() {
   (void)G{"Hello"};
 }
 } // namespace nodiscard_specialization
+
+namespace GH117975 {
+// Test for a regression for ICE in CallExpr::getUnusedResultAttr
+int f() { return 0; }
+void id_print_name() {
+  (int) // expected-warning {{expression result unused}}
+((int(*)())f)();
+}
+} // namespace GH117975

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Fix missing check for nullptr in CallExpr::getUnusedResultAttr (PR #118636)

[Yihe Li via cfe-commits]

https://github.com/Mick235711 updated 
https://github.com/llvm/llvm-project/pull/118636

>From e81792ade5cda7afbba6ba161a3a9b9184065d82 Mon Sep 17 00:00:00 2001
From: Yihe Li 
Date: Wed, 4 Dec 2024 21:45:19 +0800
Subject: [PATCH 1/2] [clang] Fix missing check for nullptr in
 CallExpr::getUnusedResultAttr

---
 clang/lib/AST/Expr.cpp| 5 +++--
 clang/test/SemaCXX/warn-unused-result.cpp | 9 +
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index a4fb4d5a1f2ec4..286f02ded27196 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -1619,8 +1619,9 @@ std::pair
 CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
   // If the callee is marked nodiscard, return that attribute
   const Decl *D = getCalleeDecl();
-  if (const auto *A = D->getAttr())
-return {nullptr, A};
+  if (D != nullptr)
+if (const auto *A = D->getAttr())
+  return {nullptr, A};
 
   // If the return type is a struct, union, or enum that is marked nodiscard,
   // then return the return type attribute.
diff --git a/clang/test/SemaCXX/warn-unused-result.cpp 
b/clang/test/SemaCXX/warn-unused-result.cpp
index 682c500dc1d96d..5105f347db8b53 100644
--- a/clang/test/SemaCXX/warn-unused-result.cpp
+++ b/clang/test/SemaCXX/warn-unused-result.cpp
@@ -355,3 +355,12 @@ void use2() {
   (void)G{"Hello"};
 }
 } // namespace nodiscard_specialization
+
+namespace GH117975 {
+// Test for a regression for ICE in CallExpr::getUnusedResultAttr
+int f() { return 0; }
+void id_print_name() {
+  (int) // expected-warning {{expression result unused}}
+((int(*)())f)();
+}
+} // namespace GH117975

>From 9e8950a1f7cf9a3822b7954618085813a5e8ffdc Mon Sep 17 00:00:00 2001
From: Yihe Li 
Date: Thu, 5 Dec 2024 00:27:34 +0800
Subject: [PATCH 2/2] Use inline decl in if to test nullptr

---
 clang/lib/AST/Expr.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 286f02ded27196..d8119543e9f8f8 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -1618,8 +1618,7 @@ QualType CallExpr::getCallReturnType(const ASTContext 
&Ctx) const {
 std::pair
 CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
   // If the callee is marked nodiscard, return that attribute
-  const Decl *D = getCalleeDecl();
-  if (D != nullptr)
+  if (const Decl *D = getCalleeDecl())
 if (const auto *A = D->getAttr())
   return {nullptr, A};
 

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Fix missing check for nullptr in CallExpr::getUnusedResultAttr (PR #118636)

[Yihe Li via cfe-commits]

Mick235711 wrote:

Gentle ping @cor3ntin @Sirraide

https://github.com/llvm/llvm-project/pull/118636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Fix missing check for nullptr in CallExpr::getUnusedResultAttr (PR #118636)

[Yihe Li via cfe-commits]

Mick235711 wrote:

> This was approved, do you need someone to merge this for you?

Yes, I do; I don't really have merge permissions. Thanks a lot for the helping 
hand.

https://github.com/llvm/llvm-project/pull/118636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Generate note on declaration for nodiscard-related attributes (PR #112289)

[Yihe Li via cfe-commits]

Mick235711 wrote:

Closed in favor of #112521.

https://github.com/llvm/llvm-project/pull/112289
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits