[clang] [analyzer][NFC] Remove check::BranchCondition from debug.DumpTraversal (PR #113906)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 28 06:10:07 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-static-analyzer-1
Author: Donát Nagy (NagyDonat)
<details>
<summary>Changes</summary>
This commit removes the `check::BranchCondition` callback of the debug checker `debug.DumpTraversal` (in `TraversalChecker.cpp`) and the single broken testcase that was referring to it.
The testcase `traversal-algorithm.mm` was added in 2012 to verify that we're using DFS traversal -- however it failed to detect that we're no longer using DFS traversal and in fact it continues to pass even if I remove large random portions of its code.
This change is motivated by the plan discussed at
https://discourse.llvm.org/t/fixing-or-removing-check-branchcondition/82738
I also added some TODO notes to mark the rest of `TraversalChecker.cpp` for removal in follow-up commits.
---
Full diff: https://github.com/llvm/llvm-project/pull/113906.diff
2 Files Affected:
- (modified) clang/lib/StaticAnalyzer/Checkers/TraversalChecker.cpp (+7-21)
- (removed) clang/test/Analysis/traversal-algorithm.mm (-213)
``````````diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/TraversalChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/TraversalChecker.cpp
index 2f316bd3b20dbe..9027fa3f4dba8e 100644
--- a/clang/lib/StaticAnalyzer/Checkers/TraversalChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/TraversalChecker.cpp
@@ -23,34 +23,17 @@ using namespace clang;
using namespace ento;
namespace {
-class TraversalDumper : public Checker< check::BranchCondition,
- check::BeginFunction,
+// TODO: This checker is only referenced from two small test files and it
+// doesn't seem to be useful for manual debugging, so consider reimplementing
+// those tests with more modern tools and removing this checker.
+class TraversalDumper : public Checker< check::BeginFunction,
check::EndFunction > {
public:
- void checkBranchCondition(const Stmt *Condition, CheckerContext &C) const;
void checkBeginFunction(CheckerContext &C) const;
void checkEndFunction(const ReturnStmt *RS, CheckerContext &C) const;
};
}
-void TraversalDumper::checkBranchCondition(const Stmt *Condition,
- CheckerContext &C) const {
- // Special-case Objective-C's for-in loop, which uses the entire loop as its
- // condition. We just print the collection expression.
- const Stmt *Parent = dyn_cast<ObjCForCollectionStmt>(Condition);
- if (!Parent) {
- const ParentMap &Parents = C.getLocationContext()->getParentMap();
- Parent = Parents.getParent(Condition);
- }
-
- // It is mildly evil to print directly to llvm::outs() rather than emitting
- // warnings, but this ensures things do not get filtered out by the rest of
- // the static analyzer machinery.
- SourceLocation Loc = Parent->getBeginLoc();
- llvm::outs() << C.getSourceManager().getSpellingLineNumber(Loc) << " "
- << Parent->getStmtClassName() << "\n";
-}
-
void TraversalDumper::checkBeginFunction(CheckerContext &C) const {
llvm::outs() << "--BEGIN FUNCTION--\n";
}
@@ -71,6 +54,9 @@ bool ento::shouldRegisterTraversalDumper(const CheckerManager &mgr) {
//------------------------------------------------------------------------------
namespace {
+// TODO: This checker appears to be a utility for creating `FileCheck` tests
+// verifying its stdout output, but there are no tests that rely on it, so
+// perhaps it should be removed.
class CallDumper : public Checker< check::PreCall,
check::PostCall > {
public:
diff --git a/clang/test/Analysis/traversal-algorithm.mm b/clang/test/Analysis/traversal-algorithm.mm
deleted file mode 100644
index bdf576063d656c..00000000000000
--- a/clang/test/Analysis/traversal-algorithm.mm
+++ /dev/null
@@ -1,213 +0,0 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpTraversal -analyzer-max-loop 4 -std=c++11 %s | FileCheck -check-prefix=DFS %s
-
-int a();
-int b();
-int c();
-
-int work();
-
-void test(id input) {
- if (a()) {
- if (a())
- b();
- else
- c();
- } else {
- if (b())
- a();
- else
- c();
- }
-
- if (a())
- work();
-}
-
-void testLoops(id input) {
- while (a()) {
- work();
- work();
- work();
- }
-
- for (int i = 0; i != b(); ++i) {
- work();
- }
-
- for (id x in input) {
- work();
- work();
- work();
- }
-
- int z[] = {1,2,3};
- for (int y : z) {
- work();
- work();
- work();
- }
-}
-
-// This ordering assumes that false cases happen before the true cases.
-
-// DFS:27 WhileStmt
-// DFS-next:33 ForStmt
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:--END PATH--
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:--END PATH--
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:33 ForStmt
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:--END PATH--
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:33 ForStmt
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:--END PATH--
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:33 ForStmt
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:--END PATH--
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:27 WhileStmt
-// DFS-next:33 ForStmt
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:--END PATH--
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:33 ForStmt
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:--END PATH--
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:33 ForStmt
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:--END PATH--
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:33 ForStmt
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:--END PATH--
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:27 WhileStmt
-// DFS-next:33 ForStmt
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:--END PATH--
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:33 ForStmt
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:--END PATH--
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:33 ForStmt
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:--END PATH--
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:33 ForStmt
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:--END PATH--
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:27 WhileStmt
-// DFS-next:33 ForStmt
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:--END PATH--
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:33 ForStmt
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:--END PATH--
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:33 ForStmt
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:--END PATH--
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:33 ForStmt
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:44 CXXForRangeStmt
-// DFS-next:--END PATH--
-// DFS-next:37 ObjCForCollectionStmt
-// DFS-next:10 IfStmt
-// DFS-next:16 IfStmt
-// DFS-next:22 IfStmt
-// DFS-next:--END PATH--
-// DFS-next:--END PATH--
-// DFS-next:22 IfStmt
-// DFS-next:--END PATH--
-// DFS-next:--END PATH--
-// DFS-next:11 IfStmt
-// DFS-next:22 IfStmt
-// DFS-next:--END PATH--
-// DFS-next:--END PATH--
-// DFS-next:22 IfStmt
-// DFS-next:--END PATH--
-// DFS-next:--END PATH--
-
``````````
</details>
https://github.com/llvm/llvm-project/pull/113906
More information about the cfe-commits
mailing list