[PATCH] D44602: [clang-tidy] readability-function-size: add VariableThreshold param.

Jonas Toth via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Mar 18 03:08:58 PDT 2018


JonasToth added inline comments.


================
Comment at: docs/ReleaseNotes.rst:125
+
+  Flags functions exceeding this number of variables declared in the body.
+
----------------
I would rephrase this a little to:

```
 Flags function bodies exceeding this number of declared variables.
```


================
Comment at: test/clang-tidy/readability-function-size.cpp:118
   }
+  // CHECK-MESSAGES: :[[@LINE-22]]:6: note: 6 variables (threshold 1)
 }
----------------
Why is this check here and not on line 100 like the other conditions?


================
Comment at: test/clang-tidy/readability-function-size.cpp:144
   }
+  // CHECK-MESSAGES: :[[@LINE-22]]:6: note: 4 variables (threshold 1)
 }
----------------
same here.


================
Comment at: test/clang-tidy/readability-function-size.cpp:153
+// CHECK-MESSAGES: :[[@LINE-5]]:6: note: 1 statements (threshold 0)
+void variables_1(int i) {
+  int j;
----------------
Could you please add a test for function having more parameters then allowed variables, but no variables?


================
Comment at: test/clang-tidy/readability-function-size.cpp:180
+// CHECK-MESSAGES: :[[@LINE-6]]:6: note: 2 variables (threshold 1)
+void variables_5() {
+  for (int i;;)
----------------
This testfile is not C++17 right now, but do you know how structured bindings are handled? Maybe its worth adding another file for C++17 testing it.

Could you add a little test for range based for loops?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44602





More information about the cfe-commits mailing list