lebedev.ri added inline comments.
================ Comment at: test/clang-tidy/readability-function-size.cpp:207-212 +void variables_8() { + int a, b; + struct A { + A(int c, int d); + }; +} ---------------- aaron.ballman wrote: > lebedev.ri wrote: > > aaron.ballman wrote: > > > I think the current behavior here is correct and the previous behavior > > > was incorrect. However, it brings up an interesting question about what > > > to do here: > > > ``` > > > void f() { > > > struct S { > > > void bar() { > > > int a, b; > > > } > > > }; > > > } > > > ``` > > > Does `f()` contain zero variables or two? I would contend that it has no > > > variables because S::bar() is a different scope than f(). But I can see a > > > case being made about the complexity of f() being increased by the > > > presence of the local class definition. Perhaps this is a different facet > > > of the test about number of types? > > As previously briefly discussed in IRC, i **strongly** believe that the > > current behavior is correct, and `readability-function-size` > > should analyze/diagnose the function as a whole, including all > > sub-classes/sub-functions. > Do you know of any coding standards related to this check that weigh in on > this? > > What do you think about this: > ``` > #define SWAP(x, y) ({__typeof__(x) temp = x; x = y; y = x;}) > > void f() { > int a = 10, b = 12; > SWAP(a, b); > } > ``` > Does f() have two variables or three? Should presence of the `SWAP` macro > cause this code to be more complex due to having too many variables? Datapoint: the doc (`docs/clang-tidy/checks/readability-function-size.rst`) actually already states that macros *are* counted. ``` .. option:: StatementThreshold Flag functions exceeding this number of statements. This may differ significantly from the number of lines for macro-heavy code. The default is `800`. ``` ``` .. 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). ``` 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