lebedev.ri added inline comments.
================
Comment at: clang-tidy/readability/FunctionSizeCheck.cpp:30-31
+ bool VisitDecompositionDecl(DecompositionDecl *) {
+ // DecompositionDecl was already visited as VarDecl. Don't count it twice.
+ Info.Variables--;
+ return true;
----------------
aaron.ballman wrote:
> This comment could be clarified. I think what it's trying to express is that
> the decomposition declaration was already counted as a variable declaration
> but we do not want to count it as such.
>
> I think a better way to express this is to change `VisitVarDecl()` to check
> `isa<DecompositionDecl>()` and not increment in that case, rather than
> increment in one place and decrement in another. You could also check
> `isa<ParmVarDecl>()` at the same time and skip subtracting off formal
> parameters.
> This comment could be clarified. I think what it's trying to express is that
> the decomposition declaration was already counted as a variable declaration
> but we do not want to count it as such.
Yep.
> I think a better way to express this is to change VisitVarDecl() to check
> isa<DecompositionDecl>() and not increment in that case, rather than
> increment in one place and decrement in another.
I agree.
> You could also check isa<ParmVarDecl>() at the same time and skip subtracting
> off formal parameters.
That is a behavior-changing change, actually.
But i don't have any strong preferences here, so changed, test added.
================
Comment at: docs/clang-tidy/checks/readability-function-size.rst:42
+
+ Flag functions exceeding this number of variables declared in the body.
+ The default is `-1` (ignore the number of variables).
----------------
aaron.ballman wrote:
> This should clarify that parameters do not count as variables declared in the
> body.
This gets oh so more interesting with the testcase i added.
I'm not sure whether we actually want to ignore all the `ParmVarDecl`, or keep
```
unsigned ActualNumberParameters = Func->getNumParams();
unsigned BodyVariables = FI.Variables - ActualNumberParameters;
```
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
