Author: lebedevri
Date: Fri Jun  9 09:22:10 2017
New Revision: 305082

URL: http://llvm.org/viewvc/llvm-project?rev=305082&view=rev
Log:
[clang-tidy] readability-function-size: add NestingThreshold param.

Summary:
Finds compound statements which create next nesting level after 
`NestingThreshold` and emits a warning.
Do note that it warns about each compound statement that breaches the 
threshold, but not any of it's sub-statements, to have readable warnings.

I was able to find only one coding style referencing nesting:
  - https://www.kernel.org/doc/html/v4.10/process/coding-style.html#indentation
     > In short, 8-char indents make things easier to read, and have the added 
benefit of warning you when you’re nesting your functions too deep.

This seems too basic, i'm not sure what else to test. Are more tests needed?

Reviewers: alexfh, aaron.ballman, sbenza

Reviewed By: alexfh, aaron.ballman

Subscribers: xazax.hun

Tags: #clang-tools-extra

Differential Revision: https://reviews.llvm.org/D32942

Modified:
    clang-tools-extra/trunk/clang-tidy/readability/FunctionSizeCheck.cpp
    clang-tools-extra/trunk/clang-tidy/readability/FunctionSizeCheck.h
    clang-tools-extra/trunk/docs/ReleaseNotes.rst
    clang-tools-extra/trunk/docs/clang-tidy/checks/readability-function-size.rst
    clang-tools-extra/trunk/test/clang-tidy/readability-function-size.cpp

Modified: clang-tools-extra/trunk/clang-tidy/readability/FunctionSizeCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/FunctionSizeCheck.cpp?rev=305082&r1=305081&r2=305082&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/readability/FunctionSizeCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/readability/FunctionSizeCheck.cpp Fri 
Jun  9 09:22:10 2017
@@ -38,6 +38,13 @@ public:
       ++Info.Branches;
     // 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:
@@ -47,7 +54,10 @@ public:
 
     Base::TraverseStmt(Node);
 
+    if (TrackedParent.back())
+      --CurrentNestingLevel;
     TrackedParent.pop_back();
+
     return true;
   }
 
@@ -59,13 +69,15 @@ public:
   }
 
   struct FunctionInfo {
-    FunctionInfo() : Lines(0), Statements(0), Branches(0) {}
-    unsigned Lines;
-    unsigned Statements;
-    unsigned Branches;
+    unsigned Lines = 0;
+    unsigned Statements = 0;
+    unsigned Branches = 0;
+    unsigned NestingThreshold = 0;
+    std::vector<SourceLocation> NestingThresholders;
   };
   FunctionInfo Info;
   std::vector<bool> TrackedParent;
+  unsigned CurrentNestingLevel = 0;
 };
 
 FunctionSizeCheck::FunctionSizeCheck(StringRef Name, ClangTidyContext *Context)
@@ -73,13 +85,15 @@ FunctionSizeCheck::FunctionSizeCheck(Str
       LineThreshold(Options.get("LineThreshold", -1U)),
       StatementThreshold(Options.get("StatementThreshold", 800U)),
       BranchThreshold(Options.get("BranchThreshold", -1U)),
-      ParameterThreshold(Options.get("ParameterThreshold", -1U)) {}
+      ParameterThreshold(Options.get("ParameterThreshold", -1U)),
+      NestingThreshold(Options.get("NestingThreshold", -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);
 }
 
 void FunctionSizeCheck::registerMatchers(MatchFinder *Finder) {
@@ -90,6 +104,7 @@ void FunctionSizeCheck::check(const Matc
   const auto *Func = Result.Nodes.getNodeAs<FunctionDecl>("func");
 
   FunctionASTVisitor Visitor;
+  Visitor.Info.NestingThreshold = NestingThreshold;
   Visitor.TraverseDecl(const_cast<FunctionDecl *>(Func));
   auto &FI = Visitor.Info;
 
@@ -109,7 +124,8 @@ void FunctionSizeCheck::check(const Matc
 
   if (FI.Lines > LineThreshold || FI.Statements > StatementThreshold ||
       FI.Branches > BranchThreshold ||
-      ActualNumberParameters > ParameterThreshold) {
+      ActualNumberParameters > ParameterThreshold ||
+      !FI.NestingThresholders.empty()) {
     diag(Func->getLocation(),
          "function %0 exceeds recommended size/complexity thresholds")
         << Func;
@@ -138,6 +154,12 @@ void FunctionSizeCheck::check(const Matc
          DiagnosticIDs::Note)
         << ActualNumberParameters << ParameterThreshold;
   }
+
+  for (const auto &CSPos : FI.NestingThresholders) {
+    diag(CSPos, "nesting level %0 starts here (threshold %1)",
+         DiagnosticIDs::Note)
+        << NestingThreshold + 1 << NestingThreshold;
+  }
 }
 
 } // namespace readability

Modified: clang-tools-extra/trunk/clang-tidy/readability/FunctionSizeCheck.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/FunctionSizeCheck.h?rev=305082&r1=305081&r2=305082&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/readability/FunctionSizeCheck.h 
(original)
+++ clang-tools-extra/trunk/clang-tidy/readability/FunctionSizeCheck.h Fri Jun  
9 09:22:10 2017
@@ -27,8 +27,12 @@ namespace readability {
 ///     macro-heavy code. The default is `800`.
 ///   * `BranchThreshold` - flag functions exceeding this number of control
 ///     statements. The default is `-1` (ignore the number of branches).
-///   * `ParameterThreshold` - flag functions having a high number of 
parameters.
-///     The default is `6`.
+///   * `ParameterThreshold` - flag functions having a high number of
+///     parameters. The default is `-1` (ignore the number of parameters).
+///   * `NestingThreshold` - 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).
 class FunctionSizeCheck : public ClangTidyCheck {
 public:
   FunctionSizeCheck(StringRef Name, ClangTidyContext *Context);
@@ -42,6 +46,7 @@ private:
   const unsigned StatementThreshold;
   const unsigned BranchThreshold;
   const unsigned ParameterThreshold;
+  const unsigned NestingThreshold;
 };
 
 } // namespace readability

Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=305082&r1=305081&r2=305082&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Fri Jun  9 09:22:10 2017
@@ -111,6 +111,11 @@ Improvements to clang-tidy
   Finds possible inefficient vector operations in for loops that may cause
   unnecessary memory reallocations.
 
+- Added `NestingThreshold` to `readability-function-size
+  
<http://clang.llvm.org/extra/clang-tidy/checks/readability-function-size.html>`_
 check
+
+  Finds compound statements which create next nesting level after 
`NestingThreshold` and emits a warning.
+
 - Added `ParameterThreshold` to `readability-function-size
   
<http://clang.llvm.org/extra/clang-tidy/checks/readability-function-size.html>`_
 check
 

Modified: 
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-function-size.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/readability-function-size.rst?rev=305082&r1=305081&r2=305082&view=diff
==============================================================================
--- 
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-function-size.rst 
(original)
+++ 
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-function-size.rst 
Fri Jun  9 09:22:10 2017
@@ -28,5 +28,11 @@ Options
 
 .. option:: ParameterThreshold
 
-   Flag functions that exceed a specified number of parameters. The default 
+   Flag functions that exceed a specified number of parameters. The default
    is `-1` (ignore the number of parameters).
+
+.. option:: NestingThreshold
+
+    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).

Modified: clang-tools-extra/trunk/test/clang-tidy/readability-function-size.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-function-size.cpp?rev=305082&r1=305081&r2=305082&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/readability-function-size.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/readability-function-size.cpp Fri 
Jun  9 09:22:10 2017
@@ -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}]}' -- -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}]}' -- -std=c++11
 
 // Bad formatting is intentional, don't run clang-format over the whole file!
 
@@ -59,3 +59,33 @@ void bar2() { class A { void barx() {;;}
 //
 // CHECK-MESSAGES: :[[@LINE-4]]:30: warning: function 'barx' exceeds 
recommended size/complexity
 // CHECK-MESSAGES: :[[@LINE-5]]:30: note: 2 statements (threshold 0)
+
+#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)
+  int a;
+  { // 2
+    int b;
+    { // 3
+// CHECK-MESSAGES: :[[@LINE-1]]:5: note: nesting level 3 starts here 
(threshold 2)
+      int c;
+      { // 4
+        int d;
+      }
+    }
+  }
+  { // 2
+    int e;
+  }
+  { // 2
+    { // 3
+// CHECK-MESSAGES: :[[@LINE-1]]:5: note: nesting level 3 starts here 
(threshold 2)
+      int j;
+    }
+  }
+  macro()
+// CHECK-MESSAGES: :[[@LINE-1]]:3: note: nesting level 3 starts here 
(threshold 2)
+// CHECK-MESSAGES: :[[@LINE-27]]:25: note: expanded from macro 'macro'
+}


_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to