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

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.


================
Comment at: docs/ReleaseNotes.rst:125
+
+  Flags functions exceeding this number of variables declared in the body.
+
----------------
lebedev.ri wrote:
> JonasToth wrote:
> > I would rephrase this a little to:
> > 
> > ```
> >  Flags function bodies exceeding this number of declared variables.
> > ```
> I agree that it looks gibberish. Reworded a little. Probably still not ideal.
Its better. But the native speakers should give a final thought.


================
Comment at: test/clang-tidy/readability-function-size.cpp:118
   }
+  // CHECK-MESSAGES: :[[@LINE-22]]:6: note: 6 variables (threshold 1)
 }
----------------
lebedev.ri wrote:
> JonasToth wrote:
> > Why is this check here and not on line 100 like the other conditions?
> It can not be there, the `note: nesting level <> starts here (threshold 2)` are outputted before `note: <> variables (threshold 1)`, and filecheck respects the order of `// CHECK-MESSAGES:`
I see, no problems then :)


================
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;;)
----------------
lebedev.ri wrote:
> JonasToth wrote:
> > 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?
> > 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.
> 
> Nice catch!
> 
> > Could you add a little test for range based for loops?
> 
> Added. I expected them to not work (after a quick look at AST), but apparently they just work..
> Added. I expected them to not work (after a quick look at AST), but apparently they just work..

Life would be so much easier if this is always the case :D


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44602