From: NagaChaitanya Vellanki Date: Fri, 26 May 2023 20:57:17 +0000 (-0700) Subject: [clang-tidy] Check for specific return types on all functions X-Git-Tag: upstream/17.0.6~7013 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=785b30b8a33a394a677b1b8ce35c66ba482db169;p=platform%2Fupstream%2Fllvm.git [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..f8139381 100644 --- 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, 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 --- 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 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 --- 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 + ` check to check for all functions + with specified return types using the ``CheckedReturnTypes`` option. + - Deprecated :doc:`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 --- 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 --- 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 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();