[clang-tidy] readability-function-size: fix nesting level calculation
Summary:
A followup for D32942.
Malcolm Parsons has provided a valid testcase that the initial version of the check complained about nested `if`'s.
As it turns out, the culprit is the **partially** un-intentional `switch` fallthrough.
So rewrite the NestingThreshold logic without ab-using+mis-using that switch with fallthrough, and add testcases with nested `if`' where there should be a warning and shouldn't be a warning. This results in a cleaner, simpler code, too.
I guess, now it would be actually possible to pick some reasonable default for `NestingThreshold` setting.
Fixes PR33454.
Reviewers: malcolm.parsons, alexfh
Reviewed By: malcolm.parsons
Subscribers: sbenza, xazax.hun, cfe-commits, aaron.ballman
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D34202
llvm-svn: 305554
diff --git a/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp b/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp
index 51b1684..ea5f750 100644
--- a/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp
@@ -36,15 +36,8 @@
case Stmt::ForStmtClass:
case Stmt::SwitchStmtClass:
++Info.Branches;
- // fallthrough
+ LLVM_FALLTHROUGH;
case Stmt::CompoundStmtClass:
- // If this new compound statement is located in a compound statement,
- // which is already nested NestingThreshold levels deep, record the start
- // location of this new compound statement
- if (CurrentNestingLevel == Info.NestingThreshold)
- Info.NestingThresholders.push_back(Node->getLocStart());
-
- ++CurrentNestingLevel;
TrackedParent.push_back(true);
break;
default:
@@ -54,13 +47,25 @@
Base::TraverseStmt(Node);
- if (TrackedParent.back())
- --CurrentNestingLevel;
TrackedParent.pop_back();
return true;
}
+ bool TraverseCompoundStmt(CompoundStmt *Node) {
+ // If this new compound statement is located in a compound statement, which
+ // is already nested NestingThreshold levels deep, record the start location
+ // of this new compound statement.
+ if (CurrentNestingLevel == Info.NestingThreshold)
+ Info.NestingThresholders.push_back(Node->getLocStart());
+
+ ++CurrentNestingLevel;
+ Base::TraverseCompoundStmt(Node);
+ --CurrentNestingLevel;
+
+ return true;
+ }
+
bool TraverseDecl(Decl *Node) {
TrackedParent.push_back(false);
Base::TraverseDecl(Node);