[PATCH] D31252: [clang-tidy] add readability-compound-statement-size check.

2017-04-27 Thread Roman Lebedev via Phabricator via cfe-commits 
lebedev.ri abandoned this revision.
lebedev.ri added a comment.

Thank you all!
After some thought, i have come to conclusion that it is best to start with 
something less controversial, and simpler.


https://reviews.llvm.org/D31252



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

[PATCH] D31252: [clang-tidy] add readability-compound-statement-size check.

2017-03-23 Thread Alexander Kornienko via Phabricator via cfe-commits 
alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.

Hi Roman,

Welcome to the community! As others noted, adding a separate check so similar 
functionally and implementation-wise to the existing one is not the best way to 
go here. A single check for all similar complexity limits would be a better 
solution. However, first I'd like to understand the specific use case you have 
for this check. Is there a recommendation of a certain style document to impose 
a complexity limit per-compound statement instead of a function? Should both 
limits be used? What would be the specific numbers and how should they interact 
with regard to nesting (e.g. should the warning be issued only on the innermost 
compound statement violating the rule or should it be issued on all levels)?


https://reviews.llvm.org/D31252



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

[PATCH] D31252: [clang-tidy] add readability-compound-statement-size check.

2017-03-23 Thread Roman Lebedev via Phabricator via cfe-commits 
lebedev.ri added a comment.

In https://reviews.llvm.org/D31252#708209, @Eugene.Zelenko wrote:

> Please mention this check in docs/ReleaseNotes.rst (in alphabetical order).


Done

> Will be good idea to run Clang-tidy modernize and readability checks over new 
> code.

I did run them (`-checks=*`) over `CompoundStatementSizeCheck.cpp`, there is no 
`modernize` notes, and no meaningful `readability` notes.


https://reviews.llvm.org/D31252



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

[PATCH] D31252: [clang-tidy] add readability-compound-statement-size check.

2017-03-23 Thread Roman Lebedev via Phabricator via cfe-commits 
lebedev.ri updated this revision to Diff 92775.
lebedev.ri marked 2 inline comments as done.
lebedev.ri added a comment.

No changes compared to v2, just correctly rebased the master branch now.


https://reviews.llvm.org/D31252

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/CompoundStatementSizeCheck.cpp
  clang-tidy/readability/CompoundStatementSizeCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-compound-statement-size.rst
  test/clang-tidy/readability-compound-statement-size.cpp

Index: test/clang-tidy/readability-compound-statement-size.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-compound-statement-size.cpp
@@ -0,0 +1,88 @@
+// RUN: %check_clang_tidy %s readability-compound-statement-size %t -- -config='{CheckOptions: [{key: readability-compound-statement-size.LineThreshold, value: 0}, {key: readability-compound-statement-size.StatementThreshold, value: 0}, {key: readability-compound-statement-size.BranchThreshold, value: 0}]}' -- -std=c++11
+
+// Bad formatting is intentional, don't run clang-format over the whole file!
+
+// the function's base compound statement is NOT being checked.
+
+void foo1() {
+}
+
+void foo2() {;}
+
+void bar2() {{;}}
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: compound statement exceeds recommended size/complexity thresholds [readability-compound-statement-size]
+// CHECK-MESSAGES: :[[@LINE-2]]:14: note: 1 statements (threshold 0)
+
+void foo3() {
+;
+
+}
+
+void bar3() {
+  {
+;
+
+  }
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:3: warning: compound statement exceeds recommended size/complexity thresholds [readability-compound-statement-size]
+// CHECK-MESSAGES: :[[@LINE-6]]:3: note: 3 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-7]]:3: note: 1 statements (threshold 0)
+
+void foo4(int i) { if (i) {} else; {}
+}
+
+void bar4(int i) { { if (i) {} else; {}
+}
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:20: warning: compound statement exceeds recommended size/complexity thresholds [readability-compound-statement-size]
+// CHECK-MESSAGES: :[[@LINE-4]]:20: note: 1 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-5]]:20: note: 3 statements (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-6]]:20: note: 1 branches (threshold 0)
+
+void foo5(int i) {for(;i;)while(i)
+do;while(i);
+}
+
+void bar5(int i) {{for(;i;)while(i)
+do;while(i);
+}
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:19: warning: compound statement exceeds recommended size/complexity thresholds [readability-compound-statement-size]
+// CHECK-MESSAGES: :[[@LINE-5]]:19: note: 2 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-6]]:19: note: 7 statements (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-7]]:19: note: 3 branches (threshold 0)
+
+template  T foo6(T i) {return i;
+}
+int x = foo6(0);
+
+template  T bar6(T i) {{return i;
+}
+}
+int y = bar6(0);
+// CHECK-MESSAGES: :[[@LINE-4]]:36: warning: compound statement exceeds recommended size/complexity thresholds [readability-compound-statement-size]
+// CHECK-MESSAGES: :[[@LINE-5]]:36: note: 1 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-6]]:36: note: 1 statements (threshold 0)
+
+void foo7(int p1, int p2, int p3, int p4, int p5, int p6) {;}
+
+void bar8() { [](){;;;  if(1){}
+
+
+}();
+
+
+}
+// CHECK-MESSAGES: :[[@LINE-7]]:19: warning: compound statement exceeds recommended size/complexity thresholds [readability-compound-statement-size]
+// CHECK-MESSAGES: :[[@LINE-8]]:19: note: 3 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-9]]:19: note: 13 statements (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-10]]:19: note: 1 branches (threshold 0)
+
+void foo9() { class A { void foox() {;;} }; }
+
+void bar9() { { class A { void barx() {{;;}} }; } }
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: compound statement exceeds recommended size/complexity thresholds [readability-compound-statement-size]
+// CHECK-MESSAGES: :[[@LINE-2]]:15: note: 3 statements (threshold 0)
+//
+// CHECK-MESSAGES: :[[@LINE-4]]:40: warning: compound statement exceeds recommended size/complexity thresholds [readability-compound-statement-size]
+// CHECK-MESSAGES: :[[@LINE-5]]:40: note: 2 statements (threshold 0)
Index: docs/clang-tidy/checks/readability-compound-statement-size.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-compound-statement-size.rst
@@ -0,0 +1,29 @@
+.. title:: clang-tidy - readability-compound-statement-size
+
+readability-compound-statement-size
+===
+
+Checks for compound statements based on various metrics.
+
+Similar to the `readability-function-size` check. It works for each compound
+statement, excluding the ones

[PATCH] D31252: [clang-tidy] add readability-compound-statement-size check.

2017-03-23 Thread Roman Lebedev via Phabricator via cfe-commits 
lebedev.ri updated this revision to Diff 92774.
lebedev.ri added a comment.

Addressing review notes.


https://reviews.llvm.org/D31252

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/CompoundStatementSizeCheck.cpp
  clang-tidy/readability/CompoundStatementSizeCheck.h
  clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tidy/readability/IdentifierNamingCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-compound-statement-size.rst
  test/clang-tidy/check_clang_tidy.py
  test/clang-tidy/readability-compound-statement-size.cpp

Index: test/clang-tidy/readability-compound-statement-size.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-compound-statement-size.cpp
@@ -0,0 +1,88 @@
+// RUN: %check_clang_tidy %s readability-compound-statement-size %t -- -config='{CheckOptions: [{key: readability-compound-statement-size.LineThreshold, value: 0}, {key: readability-compound-statement-size.StatementThreshold, value: 0}, {key: readability-compound-statement-size.BranchThreshold, value: 0}]}' -- -std=c++11
+
+// Bad formatting is intentional, don't run clang-format over the whole file!
+
+// the function's base compound statement is NOT being checked.
+
+void foo1() {
+}
+
+void foo2() {;}
+
+void bar2() {{;}}
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: compound statement exceeds recommended size/complexity thresholds [readability-compound-statement-size]
+// CHECK-MESSAGES: :[[@LINE-2]]:14: note: 1 statements (threshold 0)
+
+void foo3() {
+;
+
+}
+
+void bar3() {
+  {
+;
+
+  }
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:3: warning: compound statement exceeds recommended size/complexity thresholds [readability-compound-statement-size]
+// CHECK-MESSAGES: :[[@LINE-6]]:3: note: 3 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-7]]:3: note: 1 statements (threshold 0)
+
+void foo4(int i) { if (i) {} else; {}
+}
+
+void bar4(int i) { { if (i) {} else; {}
+}
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:20: warning: compound statement exceeds recommended size/complexity thresholds [readability-compound-statement-size]
+// CHECK-MESSAGES: :[[@LINE-4]]:20: note: 1 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-5]]:20: note: 3 statements (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-6]]:20: note: 1 branches (threshold 0)
+
+void foo5(int i) {for(;i;)while(i)
+do;while(i);
+}
+
+void bar5(int i) {{for(;i;)while(i)
+do;while(i);
+}
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:19: warning: compound statement exceeds recommended size/complexity thresholds [readability-compound-statement-size]
+// CHECK-MESSAGES: :[[@LINE-5]]:19: note: 2 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-6]]:19: note: 7 statements (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-7]]:19: note: 3 branches (threshold 0)
+
+template  T foo6(T i) {return i;
+}
+int x = foo6(0);
+
+template  T bar6(T i) {{return i;
+}
+}
+int y = bar6(0);
+// CHECK-MESSAGES: :[[@LINE-4]]:36: warning: compound statement exceeds recommended size/complexity thresholds [readability-compound-statement-size]
+// CHECK-MESSAGES: :[[@LINE-5]]:36: note: 1 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-6]]:36: note: 1 statements (threshold 0)
+
+void foo7(int p1, int p2, int p3, int p4, int p5, int p6) {;}
+
+void bar8() { [](){;;;  if(1){}
+
+
+}();
+
+
+}
+// CHECK-MESSAGES: :[[@LINE-7]]:19: warning: compound statement exceeds recommended size/complexity thresholds [readability-compound-statement-size]
+// CHECK-MESSAGES: :[[@LINE-8]]:19: note: 3 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-9]]:19: note: 13 statements (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-10]]:19: note: 1 branches (threshold 0)
+
+void foo9() { class A { void foox() {;;} }; }
+
+void bar9() { { class A { void barx() {{;;}} }; } }
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: compound statement exceeds recommended size/complexity thresholds [readability-compound-statement-size]
+// CHECK-MESSAGES: :[[@LINE-2]]:15: note: 3 statements (threshold 0)
+//
+// CHECK-MESSAGES: :[[@LINE-4]]:40: warning: compound statement exceeds recommended size/complexity thresholds [readability-compound-statement-size]
+// CHECK-MESSAGES: :[[@LINE-5]]:40: note: 2 statements (threshold 0)
Index: test/clang-tidy/check_clang_tidy.py
===
--- test/clang-tidy/check_clang_tidy.py
+++ test/clang-tidy/check_clang_tidy.py
@@ -60,11 +60,6 @@
   if len(clang_tidy_extra_args) == 0:
 clang_tidy_extra_args = ['--', '--std=c++11'] if extension == '.cpp' \
else ['--']
-
-  # Tests should not rely on STL being available, and instead provide mock
-  # implementations of relevant APIs.
-

[PATCH] D31252: [clang-tidy] add readability-compound-statement-size check.

2017-03-22 Thread Eugene Zelenko via Phabricator via cfe-commits 
Eugene.Zelenko added a comment.

Please mention this check in docs/ReleaseNotes.rst (in alphabetical order).

Will be good idea to run Clang-tidy modernize and readability checks over new 
code.




Comment at: clang-tidy/readability/CompoundStatementSizeCheck.cpp:41
+  ++Info.Branches;
+// fallthrough
+case Stmt::CompoundStmtClass:

Please use LLVM_FALLTHROUGH instead (defined in llvm/Support/Compiler.h)



Comment at: clang-tidy/readability/CompoundStatementSizeCheck.cpp:64
+  struct CompoundStatementInfo {
+CompoundStatementInfo() : Lines(0), Statements(0), Branches(0) {}
+unsigned Lines;

Please use default member initializers and = default;


https://reviews.llvm.org/D31252



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

[PATCH] D31252: Clang-tidy: add readability-compound-statement-size check.

2017-03-22 Thread Jonas Toth via Phabricator via cfe-commits 
JonasToth added a comment.

Another thought from me:

Maybe it would be sensible to create a check like `complexity-limits` where 
different forms of complicated constructs are examined. This could include 
extreme inheritance, excessive amount of members in classes (violation of 
single responsibility), very long conditions and these kind of things.
Function size would be another part of complexity and the traversing of all 
statements in the code would be done once. This would be good for performance 
as well, one problem would be the complexity of the check though ;D

Maybe we could even move this to the mail list, there would be good input as 
well.


https://reviews.llvm.org/D31252



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

[PATCH] D31252: Clang-tidy: add readability-compound-statement-size check.

2017-03-22 Thread Jonas Toth via Phabricator via cfe-commits 
JonasToth added a comment.

Hi and welcome to the clang/llvm community :)

It seems that you took `readability-size` as starting point and modified it. 
Thats a good way to learn!

My opinion is, that this should land in `readability-function-size`. This is 
somewhat recursive to function size. Whenever there is a compundstmt, the same 
(or maybe other constraints) exist for that compound statement. 
Thats, in my opinion, the connecting factor to `function-size`.
It should be configurable. Maybe something like "global" constaint on the 
function (e.g. max 800 lines in total) and "local" constraints for 
subcomponents. These two layers of configurability kinda already exist. Your 
check would implement the recursive and local part, and function size the 
global.

What are your thoughts on that? But generally i like that approach to limit 
subparts of the functions as well.
Recommending to refactor such code to use functions would be a good diagnostic 
in my opinion.

When creating or modifying checks it would be nice to include a note in the 
ReleaseNotes as well (in doc/ i believe), but i think we should discuss the 
placement of this functionality first.


https://reviews.llvm.org/D31252



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

[PATCH] D31252: Clang-tidy: add readability-compound-statement-size check.

2017-03-22 Thread Roman Lebedev via Phabricator via cfe-commits 
lebedev.ri added a comment.

Do note that clang-tidy/readability/CompoundStatementSizeCheck.cpp is *very* 
similar to the clang-tidy/readability/FunctionSizeCheck.cpp
I'm not sure if it makes sense to have such duplication. But i'm also not sure 
yet how to deduplicate them.
But in the long run, i would like these two checks to be one, and much more 
configurable.


https://reviews.llvm.org/D31252



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

[PATCH] D31252: Clang-tidy: add readability-compound-statement-size check.

2017-03-22 Thread Roman Lebedev via Phabricator via cfe-commits 
lebedev.ri created this revision.
lebedev.ri added a project: clang-tools-extra.
Herald added a subscriber: mgorny.

This check is quite similar to the readability-function-size
check. But it is more generic. It works for each compound
statement, excluding the ones directly related to the function,
i.e. it does not duplicate the readability-function-size output.

The rationale behind the check is the same as behind
readability-function-size check, it may make sense to be able
to control sizes of the compound statements, not just the
size of the functions.

Eventually, i would like these two checkers to be one, and
handle more cases and be more configurable, e.g. to be able to
apply such a check to the 'True' block of 'If' and so on.

But since this is my first attempt at any llvm/clang patch, 
this is what it is. Please do review carefully.


https://reviews.llvm.org/D31252

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/CompoundStatementSizeCheck.cpp
  clang-tidy/readability/CompoundStatementSizeCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-compound-statement-size.rst
  docs/clang-tidy/checks/readability-function-size.rst
  test/clang-tidy/readability-compound-statement-size.cpp

Index: test/clang-tidy/readability-compound-statement-size.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-compound-statement-size.cpp
@@ -0,0 +1,88 @@
+// RUN: %check_clang_tidy %s readability-compound-statement-size %t -- -config='{CheckOptions: [{key: readability-compound-statement-size.LineThreshold, value: 0}, {key: readability-compound-statement-size.StatementThreshold, value: 0}, {key: readability-compound-statement-size.BranchThreshold, value: 0}]}' -- -std=c++11
+
+// Bad formatting is intentional, don't run clang-format over the whole file!
+
+// the function's base compound statement is NOT being checked.
+
+void foo1() {
+}
+
+void foo2() {;}
+
+void bar2() {{;}}
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: compound statement exceeds recommended size/complexity thresholds [readability-compound-statement-size]
+// CHECK-MESSAGES: :[[@LINE-2]]:14: note: 1 statements (threshold 0)
+
+void foo3() {
+;
+
+}
+
+void bar3() {
+  {
+;
+
+  }
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:3: warning: compound statement exceeds recommended size/complexity thresholds [readability-compound-statement-size]
+// CHECK-MESSAGES: :[[@LINE-6]]:3: note: 3 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-7]]:3: note: 1 statements (threshold 0)
+
+void foo4(int i) { if (i) {} else; {}
+}
+
+void bar4(int i) { { if (i) {} else; {}
+}
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:20: warning: compound statement exceeds recommended size/complexity thresholds [readability-compound-statement-size]
+// CHECK-MESSAGES: :[[@LINE-4]]:20: note: 1 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-5]]:20: note: 3 statements (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-6]]:20: note: 1 branches (threshold 0)
+
+void foo5(int i) {for(;i;)while(i)
+do;while(i);
+}
+
+void bar5(int i) {{for(;i;)while(i)
+do;while(i);
+}
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:19: warning: compound statement exceeds recommended size/complexity thresholds [readability-compound-statement-size]
+// CHECK-MESSAGES: :[[@LINE-5]]:19: note: 2 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-6]]:19: note: 7 statements (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-7]]:19: note: 3 branches (threshold 0)
+
+template  T foo6(T i) {return i;
+}
+int x = foo6(0);
+
+template  T bar6(T i) {{return i;
+}
+}
+int y = bar6(0);
+// CHECK-MESSAGES: :[[@LINE-4]]:36: warning: compound statement exceeds recommended size/complexity thresholds [readability-compound-statement-size]
+// CHECK-MESSAGES: :[[@LINE-5]]:36: note: 1 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-6]]:36: note: 1 statements (threshold 0)
+
+void foo7(int p1, int p2, int p3, int p4, int p5, int p6) {;}
+
+void bar8() { [](){;;;  if(1){}
+
+
+}();
+
+
+}
+// CHECK-MESSAGES: :[[@LINE-7]]:19: warning: compound statement exceeds recommended size/complexity thresholds [readability-compound-statement-size]
+// CHECK-MESSAGES: :[[@LINE-8]]:19: note: 3 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-9]]:19: note: 13 statements (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-10]]:19: note: 1 branches (threshold 0)
+
+void foo9() { class A { void foox() {;;} }; }
+
+void bar9() { { class A { void barx() {{;;}} }; } }
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: compound statement exceeds recommended size/complexity thresholds [readability-compound-statement-size]
+// CHECK-MESSAGES: :[[@LINE-2]]:15: note: 3 statements (threshold 0)
+//
+// CHECK-MESSAGES: :[[@LINE-4]]:40: warning: compound statement exceeds