This revision was automatically updated to reflect the committed changes.
Closed by commit rL305554: [clang-tidy] readability-function-size: fix nesting
level calculation (authored by lebedevri).
Changed prior to commit:
https://reviews.llvm.org/D34202?vs=102548=102813#toc
Repository:
rL
malcolm.parsons added a comment.
LGTM!
Comment at: clang-tidy/readability/FunctionSizeCheck.cpp:58
+// is already nested NestingThreshold levels deep, record the start
location
+// of this new compound statement
+if (CurrentNestingLevel == Info.NestingThreshold)
lebedev.ri added a comment.
In https://reviews.llvm.org/D34202#780424, @malcolm.parsons wrote:
> My coding standards require the `{}`, so while I may not agree with your
> definition of nesting, it doesn't matter.
Well, seeing `readability-deep-nesting.cpp` in the issue, i suppose the
malcolm.parsons added a comment.
My coding standards require the `{}`, so while I may not agree with your
definition of nesting, it doesn't matter.
Repository:
rL LLVM
https://reviews.llvm.org/D34202
___
cfe-commits mailing list
lebedev.ri added a comment.
In https://reviews.llvm.org/D34202#780394, @malcolm.parsons wrote:
> What do you expect for this?
>
> if (true)
> if (true)
> if (true) {
> int j;
> }
>
that it is equivalent to
if (true && true && true) { // 1
int j;
malcolm.parsons added a comment.
What do you expect for this?
if (true)
if (true)
if (true) {
int j;
}
Repository:
rL LLVM
https://reviews.llvm.org/D34202
___
cfe-commits mailing list
lebedev.ri updated this revision to Diff 102548.
lebedev.ri added a comment.
Simplify it even further by moving the logic into it's proper place -
`TraverseCompoundStmt()`
Repository:
rL LLVM
https://reviews.llvm.org/D34202
Files:
clang-tidy/readability/FunctionSizeCheck.cpp
lebedev.ri updated this revision to Diff 102542.
lebedev.ri marked an inline comment as done.
lebedev.ri added a comment.
Fix spelling.
Repository:
rL LLVM
https://reviews.llvm.org/D34202
Files:
clang-tidy/readability/FunctionSizeCheck.cpp
test/clang-tidy/readability-function-size.cpp
lebedev.ri added inline comments.
Comment at: clang-tidy/readability/FunctionSizeCheck.cpp:38
+} else {
+ assert(!isa(Node));
+ if (TrackedParent.back())
malcolm.parsons wrote:
> I don't think this assert adds anything.
Yes, however the original
malcolm.parsons added inline comments.
Comment at: clang-tidy/readability/FunctionSizeCheck.cpp:38
+} else {
+ assert(!isa(Node));
+ if (TrackedParent.back())
I don't think this assert adds anything.
Comment at:
lebedev.ri updated this revision to Diff 102533.
lebedev.ri edited the summary of this revision.
lebedev.ri added a comment.
Ok, i really messed up the initial https://reviews.llvm.org/D32942 :)
Malcolm, thank you for bothering to report :)
This should be so much better.
Repository:
rL LLVM
lebedev.ri planned changes to this revision.
lebedev.ri added a comment.
Hmm, this is moving in the right direction, but now it invalidated (decreases
the nesting level) too eagerly
Repository:
rL LLVM
https://reviews.llvm.org/D34202
___
lebedev.ri created this revision.
lebedev.ri added a project: clang-tools-extra.
A followup for https://reviews.llvm.org/D32942.
Malcolm Parsons has provided a valid testcase that the initial version of the
check complained about nested `if`'s.
As it turns out, the culprit is the **partially**
13 matches
Mail list logo
