[PATCH] D44602: [clang-tidy] readability-function-size: add VariableThreshold param.
Aaron Ballman via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 20 11:19:35 PDT 2018
aaron.ballman 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);
+ };
+}
----------------
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?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44602
More information about the cfe-commits
mailing list