[clang-tidy] Check for specific return types on all functions

Extend the check to all functions with return types like
         std::error_code, std::expected, boost::system::error_code, abseil::Status...

         Resolves issue https://github.com/llvm/llvm-project/issues/62884

Reviewed By: PiotrZSL

Differential Revision: https://reviews.llvm.org/D151383

diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
index 6049569..f813938 100644 (file)
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "UnusedReturnValueCheck.h"
+#include "../utils/Matchers.h"
 #include "../utils/OptionsUtils.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
@@ -27,7 +28,6 @@ AST_MATCHER_P(FunctionDecl, isInstantiatedFrom, Matcher<FunctionDecl>,
   return InnerMatcher.matches(InstantiatedFrom ? *InstantiatedFrom : Node,
                               Finder, Builder);
 }
-
 } // namespace
 
 UnusedReturnValueCheck::UnusedReturnValueCheck(llvm::StringRef Name,
@@ -124,19 +124,30 @@ UnusedReturnValueCheck::UnusedReturnValueCheck(llvm::StringRef Name,
                                    "::sigismember;"
                                    "::strcasecmp;"
                                    "::strsignal;"
-                                   "::ttyname")) {}
+                                   "::ttyname")),
+      CheckedReturnTypes(utils::options::parseStringList(
+          Options.get("CheckedReturnTypes", "::std::error_code;"
+                                            "::std::expected;"
+                                            "::boost::system::error_code;"
+                                            "::abseil::Status"))) {}
 
 void UnusedReturnValueCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "CheckedFunctions", CheckedFunctions);
+  Options.store(Opts, "CheckedReturnTypes",
+                utils::options::serializeStringList(CheckedReturnTypes));
 }
 
 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())),
-                   isInstantiatedFrom(hasAnyName(FunVec)))))
+                   anyOf(isInstantiatedFrom(hasAnyName(FunVec)),
+                         returns(hasCanonicalType(hasDeclaration(
+                             namedDecl(matchers::matchesAnyListedName(
+                                 CheckedReturnTypes)))))))))
           .bind("match"))));
 
   auto UnusedInCompoundStmt =

diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h
index f16815b..15881e1 100644 (file)
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h
@@ -27,6 +27,7 @@ public:
 
 private:
   std::string CheckedFunctions;
+  const std::vector<StringRef> CheckedReturnTypes;
 };
 
 } // namespace clang::tidy::bugprone

diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 980be48..1eb8c5b 100644 (file)
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -248,6 +248,10 @@ Changes in existing checks
   constructor initializers. Correctly handle constructor arguments as being
   sequenced when constructor call is written as list-initialization.
 
+- Extend :doc:`bugprone-unused-return-value
+  <clang-tidy/checks/bugprone/unused-return-value>` check to check for all functions
+  with specified return types using the ``CheckedReturnTypes`` option.
+
 - Deprecated :doc:`cert-dcl21-cpp
   <clang-tidy/checks/cert/dcl21-cpp>` 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 ffa4602..89c781b 100644 (file)
--- 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
@@ -46,5 +46,11 @@ Options
      return value often indicates that the programmer confused the function with
      ``clear()``.
 
+.. option:: CheckedReturnTypes
+
+   Semicolon-separated list of function return types to check.
+   By default the following function return types are checked:
+   `::std::error_code`, `::std::expected`, `::boost::system::error_code`, `::abseil::Status`
+
 `cert-err33-c <../cert/err33-c.html>`_ is an alias of this check that checks a
 fixed and large set of standard library functions.

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 797f56d..6da66ca 100644 (file)
--- 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
@@ -52,6 +52,9 @@ struct vector {
   bool empty() const noexcept;
 };
 
+class error_code {
+};
+
 // the check should be able to match std lib calls even if the functions are
 // declared inside inline namespaces
 inline namespace v1 {
@@ -72,6 +75,10 @@ int increment(int i) {
 
 void useFuture(const std::future &fut);
 
+std::error_code errorFunc() {
+    return std::error_code();
+}
+
 void warning() {
   std::async(increment, 42);
   // CHECK-NOTES: [[@LINE-1]]:3: warning: the value returned by this function should be used
@@ -185,6 +192,10 @@ void warning() {
     // CHECK-NOTES: [[@LINE-1]]:5: warning: the value {{.*}} should be used
     // CHECK-NOTES: [[@LINE-2]]:5: note: cast {{.*}} this warning
   }
+
+  errorFunc();
+  // CHECK-NOTES: [[@LINE-1]]:3: warning: the value {{.*}} should be used
+  // CHECK-NOTES: [[@LINE-2]]:3: note: cast {{.*}} this warning
 }
 
 void noWarning() {
@@ -209,6 +220,8 @@ void noWarning() {
   std::vector<Foo> VecNoWarning;
   auto VecEmptyRetval = VecNoWarning.empty();
 
+  (void) errorFunc();
+
   // test using the return value in different kinds of expressions
   useFuture(std::async(increment, 42));
   std::launder(&FNoWarning)->f();