lebedev.ri updated this revision to Diff 139154.
lebedev.ri marked 5 inline comments as done.
lebedev.ri added a comment.
Address @aaron.ballman's review notes.
I'm not sure the change re `ParmVarDecl` handling is actually correct, see
testcase.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44602
Files:
clang-tidy/readability/FunctionSizeCheck.cpp
clang-tidy/readability/FunctionSizeCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/readability-function-size.rst
test/clang-tidy/readability-function-size-variables-c++17.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
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s readability-function-size %t -- -config='{CheckOptions: [{key: readability-function-size.LineThreshold, value: 0}, {key: readability-function-size.StatementThreshold, value: 0}, {key: readability-function-size.BranchThreshold, value: 0}, {key: readability-function-size.ParameterThreshold, value: 5}, {key: readability-function-size.NestingThreshold, value: 2}]}' -- -std=c++11
+// RUN: %check_clang_tidy %s readability-function-size %t -- -config='{CheckOptions: [{key: readability-function-size.LineThreshold, value: 0}, {key: readability-function-size.StatementThreshold, value: 0}, {key: readability-function-size.BranchThreshold, value: 0}, {key: readability-function-size.ParameterThreshold, value: 5}, {key: readability-function-size.NestingThreshold, value: 2}, {key: readability-function-size.VariableThreshold, value: 1}]}' -- -std=c++11
// Bad formatting is intentional, don't run clang-format over the whole file!
@@ -64,7 +64,7 @@
void baz0() { // 1
// 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-2]]:6: note: 28 lines including whitespace and comments (threshold 0)
// CHECK-MESSAGES: :[[@LINE-3]]:6: note: 9 statements (threshold 0)
int a;
{ // 2
@@ -87,14 +87,15 @@
}
}
macro()
-// CHECK-MESSAGES: :[[@LINE-1]]:3: note: nesting level 3 starts here (threshold 2)
-// CHECK-MESSAGES: :[[@LINE-28]]:25: note: expanded from macro 'macro'
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: note: nesting level 3 starts here (threshold 2)
+ // CHECK-MESSAGES: :[[@LINE-28]]:25: note: expanded from macro 'macro'
+ // CHECK-MESSAGES: :[[@LINE-27]]:6: note: 9 variables (threshold 1)
}
// 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-2]]:6: note: 23 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
@@ -114,12 +115,13 @@
} else if (true) { // 2
int j;
}
+ // CHECK-MESSAGES: :[[@LINE-22]]:6: note: 6 variables (threshold 1)
}
// 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-2]]:6: note: 23 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
@@ -139,4 +141,76 @@
}
}
}
+ // CHECK-MESSAGES: :[[@LINE-22]]:6: note: 4 variables (threshold 1)
}
+
+void variables_0() {
+ int i;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:6: warning: function 'variables_0' exceeds recommended size/complexity thresholds [readability-function-size]
+// CHECK-MESSAGES: :[[@LINE-4]]:6: note: 2 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-5]]:6: note: 1 statements (threshold 0)
+void variables_1(int i) {
+ int j;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:6: warning: function 'variables_1' exceeds recommended size/complexity thresholds [readability-function-size]
+// CHECK-MESSAGES: :[[@LINE-4]]:6: note: 2 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-5]]:6: note: 1 statements (threshold 0)
+void variables_2(int i, int j) {
+ ;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:6: warning: function 'variables_2' exceeds recommended size/complexity thresholds [readability-function-size]
+// CHECK-MESSAGES: :[[@LINE-4]]:6: note: 2 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-5]]:6: note: 1 statements (threshold 0)
+void variables_3() {
+ int i[2];
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:6: warning: function 'variables_3' exceeds recommended size/complexity thresholds [readability-function-size]
+// CHECK-MESSAGES: :[[@LINE-4]]:6: note: 2 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-5]]:6: note: 1 statements (threshold 0)
+void variables_4() {
+ int i;
+ int j;
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:6: warning: function 'variables_4' exceeds recommended size/complexity thresholds [readability-function-size]
+// CHECK-MESSAGES: :[[@LINE-5]]:6: note: 3 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-6]]:6: note: 2 statements (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-7]]:6: note: 2 variables (threshold 1)
+void variables_5() {
+ int i, j;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:6: warning: function 'variables_5' exceeds recommended size/complexity thresholds [readability-function-size]
+// CHECK-MESSAGES: :[[@LINE-4]]:6: note: 2 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-5]]:6: note: 1 statements (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-6]]:6: note: 2 variables (threshold 1)
+void variables_6() {
+ for (int i;;)
+ for (int j;;)
+ ;
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:6: warning: function 'variables_6' exceeds recommended size/complexity thresholds [readability-function-size]
+// CHECK-MESSAGES: :[[@LINE-6]]:6: note: 4 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-7]]:6: note: 5 statements (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-8]]:6: note: 2 branches (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-9]]:6: note: 2 variables (threshold 1)
+void variables_7() {
+ int a[2];
+ for (auto i : a)
+ for (auto j : a)
+ ;
+}
+// CHECK-MESSAGES: :[[@LINE-6]]:6: warning: function 'variables_7' exceeds recommended size/complexity thresholds [readability-function-size]
+// CHECK-MESSAGES: :[[@LINE-7]]:6: note: 5 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-8]]:6: note: 8 statements (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-9]]:6: note: 2 branches (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-10]]:6: note: 3 variables (threshold 1)
+void variables_8() {
+ int a, b;
+ struct A {
+ A(int c, int d);
+ };
+}
+// CHECK-MESSAGES: :[[@LINE-6]]:6: warning: function 'variables_8' exceeds recommended size/complexity thresholds [readability-function-size]
+// CHECK-MESSAGES: :[[@LINE-7]]:6: note: 5 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-8]]:6: note: 2 statements (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-9]]:6: note: 2 variables (threshold 1)
Index: test/clang-tidy/readability-function-size-variables-c++17.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/readability-function-size-variables-c++17.cpp
@@ -0,0 +1,10 @@
+// RUN: %check_clang_tidy %s readability-function-size %t -- -config='{CheckOptions: [{key: readability-function-size.LineThreshold, value: 0}, {key: readability-function-size.StatementThreshold, value: 0}, {key: readability-function-size.BranchThreshold, value: 0}, {key: readability-function-size.ParameterThreshold, value: 5}, {key: readability-function-size.NestingThreshold, value: 2}, {key: readability-function-size.VariableThreshold, value: 1}]}' -- -std=c++17
+
+void structured_bindings() {
+ int a[2] = {1, 2};
+ auto [x, y] = a;
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:6: warning: function 'structured_bindings' exceeds recommended size/complexity thresholds [readability-function-size]
+// CHECK-MESSAGES: :[[@LINE-5]]:6: note: 3 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-6]]:6: note: 2 statements (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-7]]:6: note: 3 variables (threshold 1)
Index: docs/clang-tidy/checks/readability-function-size.rst
===================================================================
--- docs/clang-tidy/checks/readability-function-size.rst
+++ docs/clang-tidy/checks/readability-function-size.rst
@@ -36,3 +36,9 @@
Flag compound statements which create next nesting level after
`NestingThreshold`. This may differ significantly from the expected value
for macro-heavy code. The default is `-1` (ignore the nesting level).
+
+.. option:: VariableThreshold
+
+ Flag functions exceeding this number of variables declared in the body.
+ The default is `-1` (ignore the number of variables).
+ Please do note that function parameters are not counted here.
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -124,6 +124,11 @@
Warns on construction of specific temporary objects in the Zircon kernel.
+- Added `VariableThreshold` option to `readability-function-size
+ <http://clang.llvm.org/extra/clang-tidy/checks/readability-function-size.html>`_ check
+
+ Flags functions that have more than a specified number of variables declared in the body.
+
- New alias `hicpp-avoid-goto
<http://clang.llvm.org/extra/clang-tidy/checks/hicpp-avoid-goto.html>`_ to
`cppcoreguidelines-avoid-goto <http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-avoid-goto.html>`_
Index: clang-tidy/readability/FunctionSizeCheck.h
===================================================================
--- clang-tidy/readability/FunctionSizeCheck.h
+++ clang-tidy/readability/FunctionSizeCheck.h
@@ -33,6 +33,8 @@
/// level after `NestingThreshold`. This may differ significantly from the
/// expected value for macro-heavy code. The default is `-1` (ignore the
/// nesting level).
+/// * `VariableThreshold` - flag functions having a high number of variable
+/// declarations. The default is `-1` (ignore the number of variables).
class FunctionSizeCheck : public ClangTidyCheck {
public:
FunctionSizeCheck(StringRef Name, ClangTidyContext *Context);
@@ -47,6 +49,7 @@
const unsigned BranchThreshold;
const unsigned ParameterThreshold;
const unsigned NestingThreshold;
+ const unsigned VariableThreshold;
};
} // namespace readability
Index: clang-tidy/readability/FunctionSizeCheck.cpp
===================================================================
--- clang-tidy/readability/FunctionSizeCheck.cpp
+++ clang-tidy/readability/FunctionSizeCheck.cpp
@@ -22,6 +22,19 @@
using Base = RecursiveASTVisitor<FunctionASTVisitor>;
public:
+ bool VisitVarDecl(VarDecl *VD) {
+ // Do not count function params.
+ // Do not count decomposition declarations (C++17's structured bindings).
+ if (!(isa<ParmVarDecl>(VD) || isa<DecompositionDecl>(VD)))
+ Info.Variables++;
+ return true;
+ }
+ bool VisitBindingDecl(BindingDecl *) {
+ // Do count each of the bindings (in the decomposition declaration).
+ Info.Variables++;
+ return true;
+ }
+
bool TraverseStmt(Stmt *Node) {
if (!Node)
return Base::TraverseStmt(Node);
@@ -79,6 +92,7 @@
unsigned Statements = 0;
unsigned Branches = 0;
unsigned NestingThreshold = 0;
+ unsigned Variables = 0;
std::vector<SourceLocation> NestingThresholders;
};
FunctionInfo Info;
@@ -94,14 +108,16 @@
StatementThreshold(Options.get("StatementThreshold", 800U)),
BranchThreshold(Options.get("BranchThreshold", -1U)),
ParameterThreshold(Options.get("ParameterThreshold", -1U)),
- NestingThreshold(Options.get("NestingThreshold", -1U)) {}
+ NestingThreshold(Options.get("NestingThreshold", -1U)),
+ VariableThreshold(Options.get("VariableThreshold", -1U)) {}
void FunctionSizeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "LineThreshold", LineThreshold);
Options.store(Opts, "StatementThreshold", StatementThreshold);
Options.store(Opts, "BranchThreshold", BranchThreshold);
Options.store(Opts, "ParameterThreshold", ParameterThreshold);
Options.store(Opts, "NestingThreshold", NestingThreshold);
+ Options.store(Opts, "VariableThreshold", VariableThreshold);
}
void FunctionSizeCheck::registerMatchers(MatchFinder *Finder) {
@@ -133,7 +149,7 @@
if (FI.Lines > LineThreshold || FI.Statements > StatementThreshold ||
FI.Branches > BranchThreshold ||
ActualNumberParameters > ParameterThreshold ||
- !FI.NestingThresholders.empty()) {
+ !FI.NestingThresholders.empty() || FI.Variables > VariableThreshold) {
diag(Func->getLocation(),
"function %0 exceeds recommended size/complexity thresholds")
<< Func;
@@ -168,6 +184,12 @@
DiagnosticIDs::Note)
<< NestingThreshold + 1 << NestingThreshold;
}
+
+ if (FI.Variables > VariableThreshold) {
+ diag(Func->getLocation(), "%0 variables (threshold %1)",
+ DiagnosticIDs::Note)
+ << FI.Variables << VariableThreshold;
+ }
}
} // namespace readability
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
