[clang-tools-extra] r305554 - [clang-tidy] readability-function-size: fix nesting level calculation
Roman Lebedev via cfe-commits
cfe-commits at lists.llvm.org
Fri Jun 16 06:07:47 PDT 2017
Author: lebedevri
Date: Fri Jun 16 08:07:47 2017
New Revision: 305554
URL: http://llvm.org/viewvc/llvm-project?rev=305554&view=rev
Log:
[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
Modified:
clang-tools-extra/trunk/clang-tidy/readability/FunctionSizeCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-function-size.cpp
Modified: clang-tools-extra/trunk/clang-tidy/readability/FunctionSizeCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/FunctionSizeCheck.cpp?rev=305554&r1=305553&r2=305554&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/readability/FunctionSizeCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/readability/FunctionSizeCheck.cpp Fri Jun 16 08:07:47 2017
@@ -36,15 +36,8 @@ public:
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,12 +47,24 @@ public:
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);
Modified: clang-tools-extra/trunk/test/clang-tidy/readability-function-size.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-function-size.cpp?rev=305554&r1=305553&r2=305554&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/readability-function-size.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/readability-function-size.cpp Fri Jun 16 08:07:47 2017
@@ -63,8 +63,9 @@ void bar2() { class A { void barx() {;;}
#define macro() {int x; {int y; {int z;}}}
void baz0() { // 1
-// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'baz0' exceeds recommended size/complexity
-// CHECK-MESSAGES: :[[@LINE-2]]:6: note: 9 statements (threshold 0)
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'baz0' exceeds recommended size/complexity
+ // CHECK-MESSAGES: :[[@LINE-2]]:6: note: 27 lines including whitespace and comments (threshold 0)
+ // CHECK-MESSAGES: :[[@LINE-3]]:6: note: 9 statements (threshold 0)
int a;
{ // 2
int b;
@@ -87,5 +88,55 @@ void baz0() { // 1
}
macro()
// CHECK-MESSAGES: :[[@LINE-1]]:3: note: nesting level 3 starts here (threshold 2)
-// CHECK-MESSAGES: :[[@LINE-27]]:25: note: expanded from macro 'macro'
+// CHECK-MESSAGES: :[[@LINE-28]]:25: note: expanded from macro 'macro'
+}
+
+// check that nested if's are not reported. this was broken initially
+void nesting_if() { // 1
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'nesting_if' exceeds recommended size/complexity
+ // CHECK-MESSAGES: :[[@LINE-2]]:6: note: 22 lines including whitespace and comments (threshold 0)
+ // CHECK-MESSAGES: :[[@LINE-3]]:6: note: 18 statements (threshold 0)
+ // CHECK-MESSAGES: :[[@LINE-4]]:6: note: 6 branches (threshold 0)
+ if (true) { // 2
+ int j;
+ } else if (true) { // 2
+ int j;
+ if (true) { // 3
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: note: nesting level 3 starts here (threshold 2)
+ int j;
+ }
+ } else if (true) { // 2
+ int j;
+ if (true) { // 3
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: note: nesting level 3 starts here (threshold 2)
+ int j;
+ }
+ } else if (true) { // 2
+ int j;
+ }
+}
+
+// however this should warn
+void bad_if_nesting() { // 1
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_if_nesting' exceeds recommended size/complexity
+// CHECK-MESSAGES: :[[@LINE-2]]:6: note: 22 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-3]]:6: note: 12 statements (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-4]]:6: note: 4 branches (threshold 0)
+ if (true) { // 2
+ int j;
+ } else { // 2
+ if (true) { // 3
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: note: nesting level 3 starts here (threshold 2)
+ int j;
+ } else { // 3
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: note: nesting level 3 starts here (threshold 2)
+ if (true) { // 4
+ int j;
+ } else { // 4
+ if (true) { // 5
+ int j;
+ }
+ }
+ }
+ }
}
More information about the cfe-commits
mailing list