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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
