maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[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
This commit is contained in:
Roman Lebedev 2017-06-16 13:07:47 +00:00
parent 3a40b34123
commit eb6d821cda
2 changed files with 69 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp
View File

@ -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,13 +47,25 @@ 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);
Base::TraverseDecl(Node);

57
clang-tools-extra/test/clang-tidy/readability-function-size.cpp
View File

@ -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;
}
}
}
}
}