[clang-tools-extra] [clang-tidy] Improve bugprone-unused-return-value check (PR #66573)

Piotr Zegar via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 26 10:35:05 PDT 2023


https://github.com/PiotrZSL updated https://github.com/llvm/llvm-project/pull/66573

>From 2068098bb3a3e93150459f97f278209488d18943 Mon Sep 17 00:00:00 2001
From: Piotr Zegar <piotr.zegar at nokia.com>
Date: Sat, 16 Sep 2023 08:46:02 +0000
Subject: [PATCH 1/3] [clang-tidy] Improve bugprone-unused-return-value
 diagnostic

Improve diagnostic message to be more straight forward.
---
 .../bugprone/UnusedReturnValueCheck.cpp       |  3 +-
 clang-tools-extra/docs/ReleaseNotes.rst       |  3 +
 .../bugprone/unused-return-value-custom.cpp   | 32 +++----
 .../checkers/bugprone/unused-return-value.cpp | 96 +++++++++----------
 .../test/clang-tidy/checkers/cert/err33-c.c   | 12 +--
 5 files changed, 75 insertions(+), 71 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
index bdc601c2445f5e6..791385fbb18b493 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
@@ -178,7 +178,8 @@ void UnusedReturnValueCheck::registerMatchers(MatchFinder *Finder) {
 void UnusedReturnValueCheck::check(const MatchFinder::MatchResult &Result) {
   if (const auto *Matched = Result.Nodes.getNodeAs<CallExpr>("match")) {
     diag(Matched->getBeginLoc(),
-         "the value returned by this function should be used")
+         "the value returned by this function should not be disregarded; "
+         "neglecting it may lead to errors")
         << Matched->getSourceRange();
     diag(Matched->getBeginLoc(),
          "cast the expression to void to silence this warning",
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index c93775beb8aeaf5..e94f03a7ea5e000 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -232,6 +232,9 @@ Changes in existing checks
   <clang-tidy/checks/bugprone/undefined-memory-manipulation>` check to support
   fixed-size arrays of non-trivial types.
 
+- Improved :doc:`bugprone-unused-return-value
+  <clang-tidy/checks/bugprone/unused-return-value>` check diagnostic message.
+
 - Improved :doc:`cppcoreguidelines-avoid-non-const-global-variables
   <clang-tidy/checks/cppcoreguidelines/avoid-non-const-global-variables>` check
   to ignore ``static`` variables declared within the scope of
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-custom.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-custom.cpp
index fb5f77031a3276f..a949fb37884b880 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-custom.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-custom.cpp
@@ -47,40 +47,40 @@ void fun(int);
 
 void warning() {
   fun();
-  // CHECK-NOTES: [[@LINE-1]]:3: warning: the value returned by this function should be used
-  // CHECK-NOTES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+  // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
 
   (fun());
-  // CHECK-NOTES: [[@LINE-1]]:4: warning: the value {{.*}} should be used
-  // CHECK-NOTES: [[@LINE-2]]:4: note: cast {{.*}} this warning
+  // CHECK-MESSAGES: [[@LINE-1]]:4: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+  // CHECK-MESSAGES: [[@LINE-2]]:4: note: cast the expression to void to silence this warning
 
   ns::Outer::Inner ObjA1;
   ObjA1.memFun();
-  // CHECK-NOTES: [[@LINE-1]]:3: warning: the value {{.*}} should be used
-  // CHECK-NOTES: [[@LINE-2]]:3: note: cast {{.*}} this warning
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+  // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
 
   ns::AliasName::Inner ObjA2;
   ObjA2.memFun();
-  // CHECK-NOTES: [[@LINE-1]]:3: warning: the value {{.*}} should be used
-  // CHECK-NOTES: [[@LINE-2]]:3: note: cast {{.*}} this warning
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+  // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
 
   ns::Derived ObjA3;
   ObjA3.memFun();
-  // CHECK-NOTES: [[@LINE-1]]:3: warning: the value {{.*}} should be used
-  // CHECK-NOTES: [[@LINE-2]]:3: note: cast {{.*}} this warning
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+  // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
 
   ns::Type::staticFun();
-  // CHECK-NOTES: [[@LINE-1]]:3: warning: the value {{.*}} should be used
-  // CHECK-NOTES: [[@LINE-2]]:3: note: cast {{.*}} this warning
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+  // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
 
   ns::ClassTemplate<int> ObjA4;
   ObjA4.memFun();
-  // CHECK-NOTES: [[@LINE-1]]:3: warning: the value {{.*}} should be used
-  // CHECK-NOTES: [[@LINE-2]]:3: note: cast {{.*}} this warning
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+  // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
 
   ns::ClassTemplate<int>::staticFun();
-  // CHECK-NOTES: [[@LINE-1]]:3: warning: the value {{.*}} should be used
-  // CHECK-NOTES: [[@LINE-2]]:3: note: cast {{.*}} this warning
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+  // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
 }
 
 void noWarning() {
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value.cpp
index 6da66ca42d90fa8..db33edc370a2309 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value.cpp
@@ -81,121 +81,121 @@ std::error_code errorFunc() {
 
 void warning() {
   std::async(increment, 42);
-  // CHECK-NOTES: [[@LINE-1]]:3: warning: the value returned by this function should be used
-  // CHECK-NOTES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+  // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
 
   std::async(std::launch::async, increment, 42);
-  // CHECK-NOTES: [[@LINE-1]]:3: warning: the value {{.*}} should be used
-  // CHECK-NOTES: [[@LINE-2]]:3: note: cast {{.*}} this warning
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+  // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
 
   Foo F;
   std::launder(&F);
-  // CHECK-NOTES: [[@LINE-1]]:3: warning: the value {{.*}} should be used
-  // CHECK-NOTES: [[@LINE-2]]:3: note: cast {{.*}} this warning
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+  // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
 
   std::remove(nullptr, nullptr, 1);
-  // CHECK-NOTES: [[@LINE-1]]:3: warning: the value {{.*}} should be used
-  // CHECK-NOTES: [[@LINE-2]]:3: note: cast {{.*}} this warning
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+  // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
 
   std::remove_if(nullptr, nullptr, nullptr);
-  // CHECK-NOTES: [[@LINE-1]]:3: warning: the value {{.*}} should be used
-  // CHECK-NOTES: [[@LINE-2]]:3: note: cast {{.*}} this warning
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+  // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
 
   std::unique(nullptr, nullptr);
-  // CHECK-NOTES: [[@LINE-1]]:3: warning: the value {{.*}} should be used
-  // CHECK-NOTES: [[@LINE-2]]:3: note: cast {{.*}} this warning
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+  // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
 
   std::unique_ptr<Foo> UPtr;
   UPtr.release();
-  // CHECK-NOTES: [[@LINE-1]]:3: warning: the value {{.*}} should be used
-  // CHECK-NOTES: [[@LINE-2]]:3: note: cast {{.*}} this warning
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+  // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
 
   std::string Str;
   Str.empty();
-  // CHECK-NOTES: [[@LINE-1]]:3: warning: the value {{.*}} should be used
-  // CHECK-NOTES: [[@LINE-2]]:3: note: cast {{.*}} this warning
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+  // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
 
   std::vector<Foo> Vec;
   Vec.empty();
-  // CHECK-NOTES: [[@LINE-1]]:3: warning: the value {{.*}} should be used
-  // CHECK-NOTES: [[@LINE-2]]:3: note: cast {{.*}} this warning
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+  // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
 
   // test discarding return values inside different kinds of statements
 
   auto Lambda = [] { std::remove(nullptr, nullptr, 1); };
-  // CHECK-NOTES: [[@LINE-1]]:22: warning: the value {{.*}} should be used
-  // CHECK-NOTES: [[@LINE-2]]:22: note: cast {{.*}} this warning
+  // CHECK-MESSAGES: [[@LINE-1]]:22: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+  // CHECK-MESSAGES: [[@LINE-2]]:22: note: cast the expression to void to silence this warning
 
   if (true)
     std::remove(nullptr, nullptr, 1);
-  // CHECK-NOTES: [[@LINE-1]]:5: warning: the value {{.*}} should be used
-  // CHECK-NOTES: [[@LINE-2]]:5: note: cast {{.*}} this warning
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+  // CHECK-MESSAGES: [[@LINE-2]]:5: note: cast the expression to void to silence this warning
   else if (true)
     std::remove(nullptr, nullptr, 1);
-  // CHECK-NOTES: [[@LINE-1]]:5: warning: the value {{.*}} should be used
-  // CHECK-NOTES: [[@LINE-2]]:5: note: cast {{.*}} this warning
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+  // CHECK-MESSAGES: [[@LINE-2]]:5: note: cast the expression to void to silence this warning
   else
     std::remove(nullptr, nullptr, 1);
-  // CHECK-NOTES: [[@LINE-1]]:5: warning: the value {{.*}} should be used
-  // CHECK-NOTES: [[@LINE-2]]:5: note: cast {{.*}} this warning
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+  // CHECK-MESSAGES: [[@LINE-2]]:5: note: cast the expression to void to silence this warning
 
   while (true)
     std::remove(nullptr, nullptr, 1);
-  // CHECK-NOTES: [[@LINE-1]]:5: warning: the value {{.*}} should be used
-  // CHECK-NOTES: [[@LINE-2]]:5: note: cast {{.*}} this warning
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+  // CHECK-MESSAGES: [[@LINE-2]]:5: note: cast the expression to void to silence this warning
 
   do
     std::remove(nullptr, nullptr, 1);
-  // CHECK-NOTES: [[@LINE-1]]:5: warning: the value {{.*}} should be used
-  // CHECK-NOTES: [[@LINE-2]]:5: note: cast {{.*}} this warning
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+  // CHECK-MESSAGES: [[@LINE-2]]:5: note: cast the expression to void to silence this warning
   while (true);
 
   for (;;)
     std::remove(nullptr, nullptr, 1);
-  // CHECK-NOTES: [[@LINE-1]]:5: warning: the value {{.*}} should be used
-  // CHECK-NOTES: [[@LINE-2]]:5: note: cast {{.*}} this warning
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+  // CHECK-MESSAGES: [[@LINE-2]]:5: note: cast the expression to void to silence this warning
 
   for (std::remove(nullptr, nullptr, 1);;)
-    // CHECK-NOTES: [[@LINE-1]]:8: warning: the value {{.*}} should be used
-    // CHECK-NOTES: [[@LINE-2]]:8: note: cast {{.*}} this warning
+    // CHECK-MESSAGES: [[@LINE-1]]:8: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+    // CHECK-MESSAGES: [[@LINE-2]]:8: note: cast the expression to void to silence this warning
     ;
 
   for (;; std::remove(nullptr, nullptr, 1))
-    // CHECK-NOTES: [[@LINE-1]]:11: warning: the value {{.*}} should be used
-    // CHECK-NOTES: [[@LINE-2]]:11: note: cast {{.*}} this warning
+    // CHECK-MESSAGES: [[@LINE-1]]:11: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+    // CHECK-MESSAGES: [[@LINE-2]]:11: note: cast the expression to void to silence this warning
     ;
 
   for (auto C : "foo")
     std::remove(nullptr, nullptr, 1);
-  // CHECK-NOTES: [[@LINE-1]]:5: warning: the value {{.*}} should be used
-  // CHECK-NOTES: [[@LINE-2]]:5: note: cast {{.*}} this warning
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+  // CHECK-MESSAGES: [[@LINE-2]]:5: note: cast the expression to void to silence this warning
 
   switch (1) {
   case 1:
     std::remove(nullptr, nullptr, 1);
-    // CHECK-NOTES: [[@LINE-1]]:5: warning: the value {{.*}} should be used
-    // CHECK-NOTES: [[@LINE-2]]:5: note: cast {{.*}} this warning
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+    // CHECK-MESSAGES: [[@LINE-2]]:5: note: cast the expression to void to silence this warning
     break;
   default:
     std::remove(nullptr, nullptr, 1);
-    // CHECK-NOTES: [[@LINE-1]]:5: warning: the value {{.*}} should be used
-    // CHECK-NOTES: [[@LINE-2]]:5: note: cast {{.*}} this warning
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+    // CHECK-MESSAGES: [[@LINE-2]]:5: note: cast the expression to void to silence this warning
     break;
   }
 
   try {
     std::remove(nullptr, nullptr, 1);
-    // CHECK-NOTES: [[@LINE-1]]:5: warning: the value {{.*}} should be used
-    // CHECK-NOTES: [[@LINE-2]]:5: note: cast {{.*}} this warning
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+    // CHECK-MESSAGES: [[@LINE-2]]:5: note: cast the expression to void to silence this warning
   } catch (...) {
     std::remove(nullptr, nullptr, 1);
-    // CHECK-NOTES: [[@LINE-1]]:5: warning: the value {{.*}} should be used
-    // CHECK-NOTES: [[@LINE-2]]:5: note: cast {{.*}} this warning
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+    // CHECK-MESSAGES: [[@LINE-2]]:5: note: cast the expression to void to silence this warning
   }
 
   errorFunc();
-  // CHECK-NOTES: [[@LINE-1]]:3: warning: the value {{.*}} should be used
-  // CHECK-NOTES: [[@LINE-2]]:3: note: cast {{.*}} this warning
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+  // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
 }
 
 void noWarning() {
diff --git a/clang-tools-extra/test/clang-tidy/checkers/cert/err33-c.c b/clang-tools-extra/test/clang-tidy/checkers/cert/err33-c.c
index 520ff17bd890eae..87ce0acf664e6d4 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cert/err33-c.c
+++ b/clang-tools-extra/test/clang-tidy/checkers/cert/err33-c.c
@@ -4,15 +4,15 @@ typedef __SIZE_TYPE__ size_t;
 void *aligned_alloc(size_t alignment, size_t size);
 void test_aligned_alloc(void) {
   aligned_alloc(2, 10);
-  // CHECK-NOTES: [[@LINE-1]]:3: warning: the value returned by this function should be used
-  // CHECK-NOTES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+  // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
 }
 
 long strtol(const char *restrict nptr, char **restrict endptr, int base);
 void test_strtol(void) {
   strtol("123", 0, 10);
-  // CHECK-NOTES: [[@LINE-1]]:3: warning: the value returned by this function should be used
-  // CHECK-NOTES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+  // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
 }
 
 typedef char wchar_t;
@@ -20,6 +20,6 @@ int wscanf_s(const wchar_t *restrict format, ...);
 void test_wscanf_s(void) {
   int Val;
   wscanf_s("%i", &Val);
-  // CHECK-NOTES: [[@LINE-1]]:3: warning: the value returned by this function should be used
-  // CHECK-NOTES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+  // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
 }

>From 028dbdb111839b82ca946190683e517b8d282e19 Mon Sep 17 00:00:00 2001
From: Piotr Zegar <piotr.zegar at nokia.com>
Date: Sat, 16 Sep 2023 11:26:45 +0000
Subject: [PATCH 2/3] [clang-tidy] Add support for explicit casts in
 bugprone-unused-return-value

Issues could be silenced by for example casting
bool result value to int, now it won't work, and
problem will be detected.
---
 .../bugprone/UnusedReturnValueCheck.cpp       | 24 ++++++++++++-------
 .../bugprone/UnusedReturnValueCheck.h         |  3 +++
 clang-tools-extra/docs/ReleaseNotes.rst       |  3 ++-
 .../checkers/bugprone/unused-return-value.cpp |  4 ++++
 4 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
index 791385fbb18b493..4a499c69c294e37 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
@@ -141,15 +141,21 @@ void UnusedReturnValueCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
 void UnusedReturnValueCheck::registerMatchers(MatchFinder *Finder) {
   auto FunVec = utils::options::parseStringList(CheckedFunctions);
 
-  auto MatchedCallExpr = expr(ignoringImplicit(ignoringParenImpCasts(
-      callExpr(callee(functionDecl(
-                   // Don't match void overloads of checked functions.
-                   unless(returns(voidType())),
-                   anyOf(isInstantiatedFrom(hasAnyName(FunVec)),
-                         returns(hasCanonicalType(hasDeclaration(
-                             namedDecl(matchers::matchesAnyListedName(
-                                 CheckedReturnTypes)))))))))
-          .bind("match"))));
+  auto MatchedDirectCallExpr =
+      expr(callExpr(callee(functionDecl(
+                        // Don't match void overloads of checked functions.
+                        unless(returns(voidType())),
+                        anyOf(isInstantiatedFrom(hasAnyName(FunVec)),
+                              returns(hasCanonicalType(hasDeclaration(
+                                  namedDecl(matchers::matchesAnyListedName(
+                                      CheckedReturnTypes)))))))))
+               .bind("match"));
+
+  auto MatchedCallExpr =
+      expr(anyOf(MatchedDirectCallExpr,
+                 explicitCastExpr(unless(cxxFunctionalCastExpr()),
+                                  unless(hasCastKind(CK_ToVoid)),
+                                  hasSourceExpression(MatchedDirectCallExpr))));
 
   auto UnusedInCompoundStmt =
       compoundStmt(forEach(MatchedCallExpr),
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h
index 15881e12ca0c36a..f4288c0e72f2db4 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h
@@ -24,6 +24,9 @@ class UnusedReturnValueCheck : public ClangTidyCheck {
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  std::optional<TraversalKind> getCheckTraversalKind() const override {
+    return TK_IgnoreUnlessSpelledInSource;
+  }
 
 private:
   std::string CheckedFunctions;
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index e94f03a7ea5e000..f76cef03874d781 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -233,7 +233,8 @@ Changes in existing checks
   fixed-size arrays of non-trivial types.
 
 - Improved :doc:`bugprone-unused-return-value
-  <clang-tidy/checks/bugprone/unused-return-value>` check diagnostic message.
+  <clang-tidy/checks/bugprone/unused-return-value>` check diagnostic message,
+  added support for detection of unused results when cast to non-``void`` type.
 
 - Improved :doc:`cppcoreguidelines-avoid-non-const-global-variables
   <clang-tidy/checks/cppcoreguidelines/avoid-non-const-global-variables>` check
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value.cpp
index db33edc370a2309..1e2f67367f448bc 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value.cpp
@@ -115,6 +115,10 @@ void warning() {
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
   // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
 
+  (int)Str.empty();
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+  // CHECK-MESSAGES: [[@LINE-2]]:8: note: cast the expression to void to silence this warning
+
   std::vector<Foo> Vec;
   Vec.empty();
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors

>From c2ac1a6cfffa22f094182a51755a4c94f1c62ff9 Mon Sep 17 00:00:00 2001
From: Piotr Zegar <piotr.zegar at nokia.com>
Date: Sat, 16 Sep 2023 11:56:26 +0000
Subject: [PATCH 3/3] [clang-tidy] Add AllowCastToVoid option to
 bugprone-unused-return-value

Add option that control casting to void behavior,
change default issue suppression to disallow casting
to void.
---
 .../bugprone/UnusedReturnValueCheck.cpp       | 19 +++++++++++++------
 .../bugprone/UnusedReturnValueCheck.h         |  1 +
 .../clang-tidy/cert/CERTTidyModule.cpp        |  1 +
 clang-tools-extra/docs/ReleaseNotes.rst       |  2 ++
 .../checks/bugprone/unused-return-value.rst   |  4 ++++
 .../docs/clang-tidy/checks/cert/err33-c.rst   |  3 +++
 .../bugprone/unused-return-value-custom.cpp   | 11 +++--------
 .../checkers/bugprone/unused-return-value.cpp |  3 ++-
 8 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
index 4a499c69c294e37..6bc9f2dd367dcbd 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
@@ -130,12 +130,14 @@ UnusedReturnValueCheck::UnusedReturnValueCheck(llvm::StringRef Name,
                                             "::std::error_condition;"
                                             "::std::errc;"
                                             "::std::expected;"
-                                            "::boost::system::error_code"))) {}
+                                            "::boost::system::error_code"))),
+      AllowCastToVoid(Options.get("AllowCastToVoid", false)) {}
 
 void UnusedReturnValueCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "CheckedFunctions", CheckedFunctions);
   Options.store(Opts, "CheckedReturnTypes",
                 utils::options::serializeStringList(CheckedReturnTypes));
+  Options.store(Opts, "AllowCastToVoid", AllowCastToVoid);
 }
 
 void UnusedReturnValueCheck::registerMatchers(MatchFinder *Finder) {
@@ -151,11 +153,12 @@ void UnusedReturnValueCheck::registerMatchers(MatchFinder *Finder) {
                                       CheckedReturnTypes)))))))))
                .bind("match"));
 
-  auto MatchedCallExpr =
-      expr(anyOf(MatchedDirectCallExpr,
-                 explicitCastExpr(unless(cxxFunctionalCastExpr()),
-                                  unless(hasCastKind(CK_ToVoid)),
-                                  hasSourceExpression(MatchedDirectCallExpr))));
+  auto CheckCastToVoid =
+      AllowCastToVoid ? castExpr(unless(hasCastKind(CK_ToVoid))) : castExpr();
+  auto MatchedCallExpr = expr(
+      anyOf(MatchedDirectCallExpr,
+            explicitCastExpr(unless(cxxFunctionalCastExpr()), CheckCastToVoid,
+                             hasSourceExpression(MatchedDirectCallExpr))));
 
   auto UnusedInCompoundStmt =
       compoundStmt(forEach(MatchedCallExpr),
@@ -187,6 +190,10 @@ void UnusedReturnValueCheck::check(const MatchFinder::MatchResult &Result) {
          "the value returned by this function should not be disregarded; "
          "neglecting it may lead to errors")
         << Matched->getSourceRange();
+
+    if (!AllowCastToVoid)
+      return;
+
     diag(Matched->getBeginLoc(),
          "cast the expression to void to silence this warning",
          DiagnosticIDs::Note);
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h
index f4288c0e72f2db4..b4356f8379fdc85 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h
@@ -31,6 +31,7 @@ class UnusedReturnValueCheck : public ClangTidyCheck {
 private:
   std::string CheckedFunctions;
   const std::vector<StringRef> CheckedReturnTypes;
+  const bool AllowCastToVoid;
 };
 
 } // namespace clang::tidy::bugprone
diff --git a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
index d448d9ba6145481..d334ab8c437d32a 100644
--- a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
@@ -328,6 +328,7 @@ class CERTModule : public ClangTidyModule {
     ClangTidyOptions::OptionMap &Opts = Options.CheckOptions;
     Opts["cert-dcl16-c.NewSuffixes"] = "L;LL;LU;LLU";
     Opts["cert-err33-c.CheckedFunctions"] = CertErr33CCheckedFunctions;
+    Opts["cert-err33-c.AllowCastToVoid"] = "true";
     Opts["cert-oop54-cpp.WarnOnlyIfThisHasSuspiciousField"] = "false";
     Opts["cert-str34-c.DiagnoseSignedUnsignedCharComparisons"] = "false";
     return Options;
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index f76cef03874d781..f9eff2378439ac1 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -235,6 +235,8 @@ Changes in existing checks
 - Improved :doc:`bugprone-unused-return-value
   <clang-tidy/checks/bugprone/unused-return-value>` check diagnostic message,
   added support for detection of unused results when cast to non-``void`` type.
+  Casting to ``void`` no longer suppresses issues by default, control this
+  behavior with the new `AllowCastToVoid` option.
 
 - Improved :doc:`cppcoreguidelines-avoid-non-const-global-variables
   <clang-tidy/checks/cppcoreguidelines/avoid-non-const-global-variables>` check
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unused-return-value.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unused-return-value.rst
index 5ca96216a10043e..1e3c8a3268222ed 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unused-return-value.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unused-return-value.rst
@@ -52,5 +52,9 @@ Options
    By default the following function return types are checked:
    `::std::error_code`, `::std::error_condition`, `::std::errc`, `::std::expected`, `::boost::system::error_code`
 
+.. option:: AllowCastToVoid
+
+   Controls whether casting return values to ``void`` is permitted. Default: `false`.
+
 :doc:`cert-err33-c <../cert/err33-c>` is an alias of this check that checks a
 fixed and large set of standard library functions.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/cert/err33-c.rst b/clang-tools-extra/docs/clang-tidy/checks/cert/err33-c.rst
index f3211e50b7e6cad..c1414ec5e086db1 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/cert/err33-c.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/cert/err33-c.rst
@@ -189,6 +189,9 @@ functions are checked:
 This check is an alias of check :doc:`bugprone-unused-return-value <../bugprone/unused-return-value>`
 with a fixed set of functions.
 
+Suppressing issues by casting to ``void`` is enabled by default and can be
+disabled by setting `AllowCastToVoid` option to ``false``.
+
 The check corresponds to a part of CERT C Coding Standard rule `ERR33-C.
 Detect and handle standard library errors
 <https://wiki.sei.cmu.edu/confluence/display/c/ERR33-C.+Detect+and+handle+standard+library+errors>`_.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-custom.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-custom.cpp
index a949fb37884b880..d3650b210ab02b5 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-custom.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-custom.cpp
@@ -48,39 +48,34 @@ void fun(int);
 void warning() {
   fun();
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
-  // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
 
   (fun());
   // CHECK-MESSAGES: [[@LINE-1]]:4: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
-  // CHECK-MESSAGES: [[@LINE-2]]:4: note: cast the expression to void to silence this warning
+
+  (void)fun();
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
 
   ns::Outer::Inner ObjA1;
   ObjA1.memFun();
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
-  // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
 
   ns::AliasName::Inner ObjA2;
   ObjA2.memFun();
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
-  // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
 
   ns::Derived ObjA3;
   ObjA3.memFun();
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
-  // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
 
   ns::Type::staticFun();
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
-  // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
 
   ns::ClassTemplate<int> ObjA4;
   ObjA4.memFun();
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
-  // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
 
   ns::ClassTemplate<int>::staticFun();
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
-  // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
 }
 
 void noWarning() {
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value.cpp
index 1e2f67367f448bc..5c6ce1e4bf1fd21 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value.cpp
@@ -1,4 +1,5 @@
-// RUN: %check_clang_tidy %s bugprone-unused-return-value %t -- -- -fexceptions
+// RUN: %check_clang_tidy %s bugprone-unused-return-value %t -- \
+// RUN:   --config="{CheckOptions: {bugprone-unused-return-value.AllowCastToVoid: true}}" -- -fexceptions
 
 namespace std {
 



More information about the cfe-commits mailing list