barcisz updated this revision to Diff 461721.
barcisz added a comment.
Tests for the check
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133801/new/
https://reviews.llvm.org/D133801
Files:
clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
clang-tools-extra/clang-tidy/utils/Matchers.h
clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
clang-tools-extra/unittests/clang-tidy/MatchersTest.cpp
Index: clang-tools-extra/unittests/clang-tidy/MatchersTest.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/unittests/clang-tidy/MatchersTest.cpp
@@ -0,0 +1,37 @@
+#include "utils/Matchers.h"
+#include "../../clang/unittests/ASTMatchers/ASTMatchersTest.h"
+
+namespace clang {
+namespace tidy {
+namespace test {
+
+using namespace ast_matchers;
+
+TEST_P(ASTMatchersTest, isValueUnused) {
+ auto Matcher = matchers::isValueUnused(integerLiteral(equals(42)));
+ std::string CodePrefix = "void bar()";
+
+ matches(CodePrefix + "{42;}", Matcher);
+ matches(CodePrefix + "{int x = ({42; 0;});}", Matcher);
+ matches(CodePrefix + "{if (true) 42;}", Matcher);
+ matches(CodePrefix + "{while(true) 42;}", Matcher);
+ matches(CodePrefix + "{do 42; while(true);}", Matcher);
+ matches(CodePrefix + "{for(;;) 42;}", Matcher);
+ matches(CodePrefix + "{for(42;;) bar();}", Matcher);
+ matches(CodePrefix + "{for(;;42) bar();}", Matcher);
+ matches(CodePrefix + "{int t[] = {1, 2, 3}; for(int x : t) 42;}", Matcher);
+ matches(CodePrefix + "{switch(1) {case 42:}", Matcher);
+
+ notMatches(CodePrefix + "{bar();}", Matcher);
+ notMatches(CodePrefix + "{int x = 42;}", Matcher);
+ notMatches(CodePrefix + "{int x = ({42;});}", Matcher);
+ notMatches(CodePrefix + "{if (42) bar();}", Matcher);
+ notMatches(CodePrefix + "{while(42) bar();}", Matcher);
+ notMatches(CodePrefix + "{do bar(); while(42);}", Matcher);
+ notMatches(CodePrefix + "{for(; 42; )} bar();", Matcher);
+ notMatches(CodePrefix + "switch(1) {default: bar();}", Matcher);
+}
+
+} // namespace test
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
===================================================================
--- clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
+++ clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
@@ -25,6 +25,7 @@
GlobListTest.cpp
GoogleModuleTest.cpp
LLVMModuleTest.cpp
+ MatchersTest.cpp
ModernizeModuleTest.cpp
NamespaceAliaserTest.cpp
ObjCModuleTest.cpp
@@ -43,6 +44,7 @@
clangFrontend
clangLex
clangSerialization
+ clangTesting
clangTooling
clangToolingCore
clangTransformer
Index: clang-tools-extra/clang-tidy/utils/Matchers.h
===================================================================
--- clang-tools-extra/clang-tidy/utils/Matchers.h
+++ clang-tools-extra/clang-tidy/utils/Matchers.h
@@ -49,6 +49,51 @@
return pointerType(pointee(qualType(isConstQualified())));
}
+// Matches the statements in a GNU statement-expression that are not returned
+// from it.
+AST_MATCHER_P(StmtExpr, hasUnreturning,
+ clang::ast_matchers::internal::Matcher<Stmt>, matcher) {
+ const auto compoundStmt = Node.getSubStmt();
+ assert(compoundStmt);
+
+ clang::ast_matchers::internal::BoundNodesTreeBuilder result;
+ bool matched = false;
+ for (auto stmt = compoundStmt->body_begin();
+ stmt + 1 < compoundStmt->body_end(); ++stmt) {
+ clang::ast_matchers::internal::BoundNodesTreeBuilder builderInner(*Builder);
+ assert(stmt && *stmt);
+ if (matcher.matches(**stmt, Finder, &builderInner)) {
+ result.addMatch(builderInner);
+ matched = true;
+ }
+ }
+ *Builder = result;
+ return matched;
+}
+
+// Matches all of the nodes (simmilar to forEach) that match the matcher
+// and have return values not used in any statement.
+AST_MATCHER_FUNCTION_P(ast_matchers::StatementMatcher, isValueUnused,
+ ast_matchers::StatementMatcher, Matcher) {
+ using namespace ast_matchers;
+ const auto UnusedInCompoundStmt =
+ compoundStmt(forEach(Matcher), unless(hasParent(stmtExpr())));
+ const auto UnusedInGnuExprStmt = stmtExpr(hasUnreturning(Matcher));
+ const auto UnusedInIfStmt =
+ ifStmt(eachOf(hasThen(Matcher), hasElse(Matcher)));
+ const auto UnusedInWhileStmt = whileStmt(hasBody(Matcher));
+ const auto UnusedInDoStmt = doStmt(hasBody(Matcher));
+ const auto UnusedInForStmt = forStmt(
+ eachOf(hasLoopInit(Matcher), hasIncrement(Matcher), hasBody(Matcher)));
+ const auto UnusedInRangeForStmt = cxxForRangeStmt(hasBody(Matcher));
+ const auto UnusedInCaseStmt = switchCase(forEach(Matcher));
+ const auto Unused =
+ stmt(anyOf(UnusedInCompoundStmt, UnusedInGnuExprStmt, UnusedInIfStmt,
+ UnusedInWhileStmt, UnusedInDoStmt, UnusedInForStmt,
+ UnusedInRangeForStmt, UnusedInCaseStmt));
+ return stmt(eachOf(Unused, forEachDescendant(Unused)));
+}
+
// A matcher implementation that matches a list of type name regular expressions
// against a NamedDecl. If a regular expression contains the substring "::"
// matching will occur against the qualified name, otherwise only the typename.
Index: clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
+++ 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"
@@ -140,29 +141,8 @@
unless(returns(voidType())),
isInstantiatedFrom(hasAnyName(FunVec)))))
.bind("match"))));
-
- auto UnusedInCompoundStmt =
- compoundStmt(forEach(MatchedCallExpr),
- // The checker can't currently differentiate between the
- // return statement and other statements inside GNU statement
- // expressions, so disable the checker inside them to avoid
- // false positives.
- unless(hasParent(stmtExpr())));
- auto UnusedInIfStmt =
- ifStmt(eachOf(hasThen(MatchedCallExpr), hasElse(MatchedCallExpr)));
- auto UnusedInWhileStmt = whileStmt(hasBody(MatchedCallExpr));
- auto UnusedInDoStmt = doStmt(hasBody(MatchedCallExpr));
- auto UnusedInForStmt =
- forStmt(eachOf(hasLoopInit(MatchedCallExpr),
- hasIncrement(MatchedCallExpr), hasBody(MatchedCallExpr)));
- auto UnusedInRangeForStmt = cxxForRangeStmt(hasBody(MatchedCallExpr));
- auto UnusedInCaseStmt = switchCase(forEach(MatchedCallExpr));
-
Finder->addMatcher(
- stmt(anyOf(UnusedInCompoundStmt, UnusedInIfStmt, UnusedInWhileStmt,
- UnusedInDoStmt, UnusedInForStmt, UnusedInRangeForStmt,
- UnusedInCaseStmt)),
- this);
+ functionDecl(hasBody(matchers::isValueUnused(MatchedCallExpr))), this);
}
void UnusedReturnValueCheck::check(const MatchFinder::MatchResult &Result) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
