diff options
6 files changed, 249 insertions, 8 deletions
diff --git a/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp b/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp index e6410ce2134..8ed6b90773a 100644 --- a/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp @@ -22,6 +22,21 @@ class FunctionASTVisitor : public RecursiveASTVisitor<FunctionASTVisitor> { 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 (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 +89,38 @@ public: 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,7 +132,8 @@ FunctionSizeCheck::FunctionSizeCheck(StringRef Name, ClangTidyContext *Context) 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); @@ -102,6 +141,7 @@ void FunctionSizeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { 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 +173,7 @@ void FunctionSizeCheck::check(const MatchFinder::MatchResult &Result) { 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 +208,12 @@ void FunctionSizeCheck::check(const MatchFinder::MatchResult &Result) { DiagnosticIDs::Note) << NestingThreshold + 1 << NestingThreshold; } + + if (FI.Variables > VariableThreshold) { + diag(Func->getLocation(), "%0 variables (threshold %1)", + DiagnosticIDs::Note) + << FI.Variables << VariableThreshold; + } } } // namespace readability diff --git a/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h b/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h index 3986b95a144..7defccdb0b1 100644 --- a/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h +++ b/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h @@ -33,6 +33,8 @@ namespace readability { /// 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 @@ private: const unsigned BranchThreshold; const unsigned ParameterThreshold; const unsigned NestingThreshold; + const unsigned VariableThreshold; }; } // namespace readability diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 1334625e06f..2a00c401351 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -158,6 +158,11 @@ Improvements to clang-tidy <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>`. diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability-function-size.rst b/clang-tools-extra/docs/clang-tidy/checks/readability-function-size.rst index f903dd7aa95..3360fbd5f1e 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/readability-function-size.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/readability-function-size.rst @@ -36,3 +36,10 @@ Options 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. diff --git a/clang-tools-extra/test/clang-tidy/readability-function-size-variables-c++17.cpp b/clang-tools-extra/test/clang-tidy/readability-function-size-variables-c++17.cpp new file mode 100644 index 00000000000..8af0d9668b7 --- /dev/null +++ b/clang-tools-extra/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) diff --git a/clang-tools-extra/test/clang-tidy/readability-function-size.cpp b/clang-tools-extra/test/clang-tidy/readability-function-size.cpp index c75f5b6b70f..efb41a19cbb 100644 --- a/clang-tools-extra/test/clang-tidy/readability-function-size.cpp +++ b/clang-tools-extra/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 bar2() { class A { void barx() {;;} }; } 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 @@ void baz0() { // 1 } } 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 @@ void nesting_if() { // 1 } 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 @@ void bad_if_nesting() { // 1 } } } + // 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) |
