[llvm-branch-commits] [clang-tools-extra] d275de3 - [clan-tidy] Fix false positive in bugprone-infinite-loop

[Hans Wennborg via llvm-branch-commits]

Author: Adam Balogh
Date: 2020-01-29T21:59:00+01:00
New Revision: d275de35f8bdb92e7b5789ebdb96df99fab504ab

URL: https://github.com/llvm/llvm-project/commit/d275de35f8bdb92e7b5789ebdb96df99fab504ab
DIFF: https://github.com/llvm/llvm-project/commit/d275de35f8bdb92e7b5789ebdb96df99fab504ab.diff

LOG: [clan-tidy] Fix false positive in bugprone-infinite-loop

The checker bugprone-infinite-loop does not track changes of
variables in the initialization expression of a variable
declared inside the condition of the while statement. This
leads to false positives, similarly to the one in the bug
report https://bugs.llvm.org/show_bug.cgi?id=44618. This
patch fixes this issue by enabling tracking of the variables
of this expression as well.

Differential Revision: https://reviews.llvm.org/D73270

(cherry picked from commit 70f4c6e7b14f225f9628fbdab3620ce037613351)

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
    clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
index 81ae45a00896..c771ba81b250 100644
--- a/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
@@ -170,17 +170,33 @@ void InfiniteLoopCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *LoopStmt = Result.Nodes.getNodeAs<Stmt>("loop-stmt");
   const auto *Func = Result.Nodes.getNodeAs<FunctionDecl>("func");
 
+  bool ShouldHaveConditionVariables = true;
+  if (const auto *While = dyn_cast<WhileStmt>(LoopStmt)) {
+    if (const VarDecl *LoopVarDecl = While->getConditionVariable()) {
+      if (const Expr *Init = LoopVarDecl->getInit()) {
+        ShouldHaveConditionVariables = false;
+        Cond = Init;
+      }
+    }
+  }
+
   if (isAtLeastOneCondVarChanged(Func, LoopStmt, Cond, Result.Context))
     return;
 
   std::string CondVarNames = getCondVarNames(Cond);
-  if (CondVarNames.empty())
+  if (ShouldHaveConditionVariables && CondVarNames.empty())
     return;
 
-  diag(LoopStmt->getBeginLoc(),
-       "this loop is infinite; none of its condition variables (%0)"
-       " are updated in the loop body")
+  if (CondVarNames.empty()) {
+    diag(LoopStmt->getBeginLoc(),
+         "this loop is infinite; it does not check any variables in the"
+         " condition");
+  } else {
+    diag(LoopStmt->getBeginLoc(),
+         "this loop is infinite; none of its condition variables (%0)"
+         " are updated in the loop body")
       << CondVarNames;
+  }
 }
 
 } // namespace bugprone

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
index 33d94820b0f6..d89bdadf0212 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
@@ -4,10 +4,20 @@ void simple_infinite_loop1() {
   int i = 0;
   int j = 0;
   while (i < 10) {
-    // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (i) are updated in the loop body [bugprone-infinite-loop]
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (i) are updated in the loop body [bugprone-infinite-loop]
     j++;
   }
 
+  while (int k = 10) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; it does not check any variables in the condition [bugprone-infinite-loop]
+    j--;
+  }
+
+  while (int k = 10) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; it does not check any variables in the condition [bugprone-infinite-loop]
+    k--;
+  }
+
   do {
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (i) are updated in the loop body [bugprone-infinite-loop]
     j++;
@@ -27,6 +37,16 @@ void simple_infinite_loop2() {
     j++;
   }
 
+  while (int k = Limit) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (Limit) are updated in the loop body [bugprone-infinite-loop]
+    j--;
+  }
+
+  while (int k = Limit) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (Limit) are updated in the loop body [bugprone-infinite-loop]
+    k--;
+  }
+
   do {
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (i, Limit) are updated in the loop body [bugprone-infinite-loop]
     j++;
@@ -44,6 +64,22 @@ void simple_not_infinite1() {
     // Not an error since 'Limit' is updated.
     Limit--;
   }
+
+  while (Limit--) {
+    // Not an error since 'Limit' is updated.
+    i++;
+  }
+
+  while (int k = Limit) {
+    // Not an error since 'Limit' is updated.
+    Limit--;
+  }
+
+  while (int k = Limit--) {
+    // Not an error since 'Limit' is updated.
+    i++;
+  }
+
   do {
     Limit--;
   } while (i < Limit);