[PATCH] D64736: [clang-tidy] New bugprone-infinite-loop check for detecting obvious infinite loops

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

baloghadamsoftware marked 15 inline comments as done.
baloghadamsoftware added a comment.

Thank you for the very throughout review. I updated my patch according to your comments and tried to answer your concerns.



================
Comment at: test/clang-tidy/bugprone-infinite-loop.cpp:156
+  }
+  int &ii = i;
+}
----------------
gribozavr wrote:
> Is all escaping that syntactically follows the loop actually irrelevant? For example:
> 
> ```
> int i = 0;
> int j = 0;
> int *p = &j;
> for (int k = 0; k < 5; k++) {
>   *p = 100;
>   if (k != 0) {
>     // This loop would be reported as infinite.
>     while (i < 10) {
>     }
>   }
>   p = &i;
> }
> ```
You are right, but how frequent are such cases? I found no false positives at all when checking some ope-source projects. Should we spend resources to detect escaping in a nesting loop? I could not find a cheap way yet. I suppose this can only be done when nesting two loops. (I am sure we can ignore the cases where the outer loop is implemented using a `goto`.


================
Comment at: test/clang-tidy/bugprone-infinite-loop.cpp:225
+  while (1)
+    ; //FIXME: We expect report in this case.
+}
----------------
gribozavr wrote:
> Why?
> 
> Intentional infinite loops *that perform side-effects in their body* are quite common -- for example, network servers, command-line interpreters etc.
> 
> Also, if the loop body calls an arbitrary function, we don't know if it will throw, or `exit()`, or kill the current thread or whatnot.
We already handle loop exit statements including calls to `[[noreturn]]` functions. Is that not enough?


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

https://reviews.llvm.org/D64736