https://github.com/NagyDonat created https://github.com/llvm/llvm-project/pull/113906
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. From 003236dd68aee99a0b53b93a4c3406a44fec0085 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.n...@ericsson.com> Date: Mon, 28 Oct 2024 13:54:57 +0100 Subject: [PATCH] [analyzer][NFC] Remove check::BranchCondition from debug.DumpTraversal 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. --- .../Checkers/TraversalChecker.cpp | 28 +-- clang/test/Analysis/traversal-algorithm.mm | 213 ------------------ 2 files changed, 7 insertions(+), 234 deletions(-) delete mode 100644 clang/test/Analysis/traversal-algorithm.mm 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-- - _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
