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

Reply via email to