lebedev.ri created this revision.
lebedev.ri added a project: clang-tools-extra.
A followup for https://reviews.llvm.org/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 fix the unintentional part of the fallthrough, and add testcases with nested
`if`' where there should be a warning and shouldn't be a warning.
I guess, now it would be actually possible to pick some reasonable default for
`NestingThreshold` setting.
Repository:
rL LLVM
https://reviews.llvm.org/D34202
Files:
clang-tidy/readability/FunctionSizeCheck.cpp
test/clang-tidy/readability-function-size.cpp
Index: test/clang-tidy/readability-function-size.cpp
===================================================================
--- test/clang-tidy/readability-function-size.cpp
+++ test/clang-tidy/readability-function-size.cpp
@@ -63,8 +63,9 @@
#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,51 @@
}
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: 18 lines including whitespace and
comments (threshold 0)
+ // CHECK-MESSAGES: :[[@LINE-3]]:6: note: 15 statements (threshold 0)
+ // CHECK-MESSAGES: :[[@LINE-4]]:6: note: 5 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;
+ } 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;
+ }
+ }
+ }
+ }
}
Index: clang-tidy/readability/FunctionSizeCheck.cpp
===================================================================
--- clang-tidy/readability/FunctionSizeCheck.cpp
+++ clang-tidy/readability/FunctionSizeCheck.cpp
@@ -36,15 +36,18 @@
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());
+ if(Node->getStmtClass() == 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;
+ }
- ++CurrentNestingLevel;
TrackedParent.push_back(true);
break;
default:
Index: test/clang-tidy/readability-function-size.cpp
===================================================================
--- test/clang-tidy/readability-function-size.cpp
+++ test/clang-tidy/readability-function-size.cpp
@@ -63,8 +63,9 @@
#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,51 @@
}
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: 18 lines including whitespace and comments (threshold 0)
+ // CHECK-MESSAGES: :[[@LINE-3]]:6: note: 15 statements (threshold 0)
+ // CHECK-MESSAGES: :[[@LINE-4]]:6: note: 5 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;
+ } 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;
+ }
+ }
+ }
+ }
}
Index: clang-tidy/readability/FunctionSizeCheck.cpp
===================================================================
--- clang-tidy/readability/FunctionSizeCheck.cpp
+++ clang-tidy/readability/FunctionSizeCheck.cpp
@@ -36,15 +36,18 @@
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());
+ if(Node->getStmtClass() == 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;
+ }
- ++CurrentNestingLevel;
TrackedParent.push_back(true);
break;
default:
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
