[clang-tools-extra] 785b30b - [clang-tidy] Check for specific return types on all functions
NagaChaitanya Vellanki via cfe-commits
cfe-commits at lists.llvm.org
Fri May 26 14:10:08 PDT 2023
Author: NagaChaitanya Vellanki
Date: 2023-05-26T13:59:18-07:00
New Revision: 785b30b8a33a394a677b1b8ce35c66ba482db169
URL: https://github.com/llvm/llvm-project/commit/785b30b8a33a394a677b1b8ce35c66ba482db169
DIFF: https://github.com/llvm/llvm-project/commit/785b30b8a33a394a677b1b8ce35c66ba482db169.diff
LOG: [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
Added:
Modified:
clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/bugprone/unused-return-value.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
index 6049569b5f581..f8139381d7e01 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<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 f16815b0f839e..15881e12ca0c3 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h
@@ -27,6 +27,7 @@ class UnusedReturnValueCheck : public ClangTidyCheck {
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 980be4869ead8..1eb8c5ba4b71b 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
+ <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 ffa4602ef049e..89c781b0fe714 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 797f56d9f4f5a..6da66ca42d90f 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<Foo> VecNoWarning;
auto VecEmptyRetval = VecNoWarning.empty();
+ (void) errorFunc();
+
// test using the return value in
diff erent kinds of expressions
useFuture(std::async(increment, 42));
std::launder(&FNoWarning)->f();
More information about the cfe-commits
mailing list