[PATCH] D32942: [clang-tidy] readability-function-size: add NestingThreshold param.

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons added a comment.

In https://reviews.llvm.org/D32942#779143, @lebedev.ri wrote:

> In https://reviews.llvm.org/D32942#778729, @malcolm.parsons wrote:
>
> > In https://reviews.llvm.org/D32942#777001, @lebedev.ri wrote:
> >
> > > Which makes sense, since in AST, they are nested:
> >
> >
> > They're not nested in the formatting, so I don't think it makes sense.
>
>
> As usual, all the meaningful review happens post-factum :)


I didn't get an email about this change until it was pushed.

> So, it should warn on:
>  ...
>  but should not on what you/I posted in the previous comment.

Yes.

> The difference seems to be some kind of implicit `CompoundStmt` added by `IfStmt`?

CompoundStmt represents the `{}`.

> Assuming that is the case, maybe this is as simple as checking whether this `CompoundStmt` is implicit or not?

My prototype of this feature used `ifStmt(stmt().bind("if"), unless(hasParent(ifStmt(hasElse(equalsBoundNode("if"))))))`.

> Best to move open a new bug about this. I'll see what can be done.

Do you need me to report a bug?


Repository:
  rL LLVM

https://reviews.llvm.org/D32942