[PATCH] D73270: [clang-tidy] Fix false positive in bugprone-infinite-loop

[Balogh, Ádám via Phabricator via cfe-commits]

baloghadamsoftware marked 2 inline comments as done.
baloghadamsoftware added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp:186
+    }
+  }
+
----------------
gribozavr2 wrote:
> I think this logic should go into `isChanged()`, similarly to how it handles for loops today.
`isChanged()` looks for changes of a specific variable. This part collects the variables to check. The fundamental problem with the current logic is that it only look for the condition variables in the `Condition` of the loop statement, but it does not contain the initial value of the variable declared inside the condition part of the `while` statement. Thus in case of `while (int m = n) { n--; }` the condition is only the expression `m`. Thus we simply cannot grab `n` by only checking the condition thus consider this loop as infinite because `m` is not changed. Taking the whole loop statement instead of the condition is also wrong because then the following obviously infinite loop is not detected: `while (n) { m--; }` because `m` is also grabbed from the body which must not. We must explicitly check the variable of the initialization expression of variable declared in the condition of `while` loops because it is a separate child of the statement.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp:202
+    }
+  }
+
----------------
gribozavr2 wrote:
> I think this logic should go into `getCondVarNames`. It even seems like the existing logic should work without changes, it is walking all children of a provided statement.
The same is valid here that I wrote above. `getCondVarNames()` did not grab the variable names in the initialization expression of the variable declared inside the condition part of the `while` loop because they are no children of the `Condition` part in Clang. You can see this in the bugzilla bug report as well. There the variable `pattern` did not appear in the diagnostic message.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73270/new/

https://reviews.llvm.org/D73270