lebedev.ri updated this revision to Diff 141997.
lebedev.ri added a reviewer: alexfh.
lebedev.ri added a comment.
After a quick IRC chat with @aaron.ballman, it was discovered that there has
been a big misunderstanding:
we don't need to ignore/exempt *all* the variables declared in macros, we
just need to ignore *all* GNU Statement Expressions.
Yay!
I think this is ready now, please review.
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,161 @@
}
}
}
+ // 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() {
+ if (int a = 1)
+ if (int b = 2)
+ ;
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:6: warning: function 'variables_7' 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: 7 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_8() {
+ int a[2];
+ for (auto i : a)
+ for (auto j : a)
+ ;
+}
+// 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: 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_9() {
+ int a, b;
+ struct A {
+ A(int c, int d) {
+ int e, f;
+ }
+ };
+}
+// CHECK-MESSAGES: :[[@LINE-8]]:6: warning: function 'variables_9' exceeds recommended size/complexity thresholds [readability-function-size]
+// CHECK-MESSAGES: :[[@LINE-9]]:6: note: 7 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-10]]:6: note: 3 statements (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-11]]:6: note: 2 variables (threshold 1)
+// CHECK-MESSAGES: :[[@LINE-9]]:5: warning: function 'A' exceeds recommended size/complexity thresholds [readability-function-size]
+// CHECK-MESSAGES: :[[@LINE-10]]:5: note: 2 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-11]]:5: note: 1 statements (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-12]]:5: note: 2 variables (threshold 1)
+void variables_10() {
+ int a, b;
+ struct A {
+ int c;
+ int d;
+ };
+}
+// CHECK-MESSAGES: :[[@LINE-7]]:6: warning: function 'variables_10' exceeds recommended size/complexity thresholds [readability-function-size]
+// CHECK-MESSAGES: :[[@LINE-8]]:6: note: 6 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-9]]:6: note: 2 statements (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-10]]:6: note: 2 variables (threshold 1)
+void variables_11() {
+ struct S {
+ void bar() {
+ int a, b;
+ }
+ };
+}
+// CHECK-MESSAGES: :[[@LINE-7]]:6: warning: function 'variables_11' exceeds recommended size/complexity thresholds [readability-function-size]
+// CHECK-MESSAGES: :[[@LINE-8]]:6: note: 6 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-7]]:10: warning: function 'bar' exceeds recommended size/complexity thresholds [readability-function-size]
+// CHECK-MESSAGES: :[[@LINE-8]]:10: note: 2 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-9]]:10: note: 2 variables (threshold 1)
+void variables_12() {
+ int v;
+ auto test = [](int a, int b) -> void {};
+ test({}, {});
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:6: warning: function 'variables_12' 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: 3 statements (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-8]]:6: note: 2 variables (threshold 1)
+void variables_13() {
+ int v;
+ auto test = []() -> void {
+ int a;
+ int b;
+ };
+ test();
+}
+// CHECK-MESSAGES: :[[@LINE-8]]:6: warning: function 'variables_13' exceeds recommended size/complexity thresholds [readability-function-size]
+// CHECK-MESSAGES: :[[@LINE-9]]:6: note: 7 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-10]]:6: note: 5 statements (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-11]]:6: note: 2 variables (threshold 1)
+void variables_14() {
+ (void)({int a = 12; a; });
+ (void)({int a = 12; a; });
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:6: warning: function 'variables_14' 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: 6 statements (threshold 0)
+#define SWAP(x, y) ({__typeof__(x) temp = x; x = y; y = temp; })
+void variables_15() {
+ int a = 10, b = 12;
+ SWAP(a, b);
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:6: warning: function 'variables_15' 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: 5 statements (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-7]]:6: note: 2 variables (threshold 1)
+#define vardecl(type, name) type name;
+void variables_16() {
+ vardecl(int, a);
+ vardecl(int, b);
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:6: warning: function 'variables_16' 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: 4 statements (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-7]]: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,21 @@
+// 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)
+
+#define SWAP(x, y) ({auto& [x0, x1] = x; __typeof__(x) t = {x0, x1}; auto& [y0, y1] = y; auto& [t0, t1] = t; x0 = y0; x1 = y1; y0 = t0; y1 = t1; })
+void variables_13() {
+ int a[2] = {1, 2};
+ int b[2] = {3, 4};
+ SWAP(a, b);
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:6: warning: function 'variables_13' 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: 11 statements (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-8]]:6: note: 2 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,10 @@
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 note that function parameters and variables declared in lambdas,
+ GNU Statement Expressions, and nested class inline functions are not counted.
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -156,6 +156,11 @@
<clang-tidy/checks/cppcoreguidelines-avoid-goto>`
added.
+- Added `VariableThreshold` option to :doc:`readability-function-size
+ <clang-tidy/checks/readability-function-size>` check
+
+ Flags functions that have more than a specified number of variables declared in the body.
+
- Adding the missing bitwise assignment operations to
:doc:`hicpp-signed-bitwise <clang-tidy/checks/hicpp-signed-bitwise>`.
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,22 @@
using Base = RecursiveASTVisitor<FunctionASTVisitor>;
public:
+ bool VisitVarDecl(VarDecl *VD) {
+ // Do not count function params.
+ // Do not count decomposition declarations (C++17's structured bindings).
+ // Do not count variables declared in macros.
+ if (StructNesting == 0 &&
+ !(isa<ParmVarDecl>(VD) || isa<DecompositionDecl>(VD)))
+ Info.Variables++;
+ return true;
+ }
+ bool VisitBindingDecl(BindingDecl *BD) {
+ // Do count each of the bindings (in the decomposition declaration).
+ if (StructNesting == 0)
+ Info.Variables++;
+ return true;
+ }
+
bool TraverseStmt(Stmt *Node) {
if (!Node)
return Base::TraverseStmt(Node);
@@ -74,15 +90,38 @@
return true;
}
+ bool TraverseLambdaExpr(LambdaExpr *Node) {
+ StructNesting++;
+ Base::TraverseLambdaExpr(Node);
+ StructNesting--;
+ return true;
+ }
+
+ bool TraverseCXXRecordDecl(CXXRecordDecl *Node) {
+ StructNesting++;
+ Base::TraverseCXXRecordDecl(Node);
+ StructNesting--;
+ return true;
+ }
+
+ bool TraverseStmtExpr(StmtExpr *SE) {
+ ++StructNesting;
+ Base::TraverseStmtExpr(SE);
+ --StructNesting;
+ return true;
+ }
+
struct FunctionInfo {
unsigned Lines = 0;
unsigned Statements = 0;
unsigned Branches = 0;
unsigned NestingThreshold = 0;
+ unsigned Variables = 0;
std::vector<SourceLocation> NestingThresholders;
};
FunctionInfo Info;
std::vector<bool> TrackedParent;
+ unsigned StructNesting = 0;
unsigned CurrentNestingLevel = 0;
};
@@ -94,14 +133,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 +174,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 +209,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
