[clang-tools-extra] cce7951 - [clang-tidy] Reduce false positives for `bugprone-infinite-loop` with dependent expressions

[Fabian Wolff via cfe-commits]

Author: Fabian Wolff
Date: 2022-04-20T17:17:54+02:00
New Revision: cce79514ff405d93ab7aab79974e6f80ced85980

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

LOG: [clang-tidy] Reduce false positives for `bugprone-infinite-loop` with dependent expressions

Fixes https://github.com/llvm/llvm-project/issues/51423.

Reviewed By: aaron.ballman

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

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
    clang-tools-extra/docs/ReleaseNotes.rst
    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 99775b2400458..3359a7f8932ed 100644
--- a/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
@@ -117,12 +117,32 @@ static std::string getCondVarNames(const Stmt *Cond) {
   return Result;
 }
 
-static bool isKnownFalse(const Expr &Cond, const ASTContext &Ctx) {
-  if (Cond.isValueDependent())
+static bool isKnownToHaveValue(const Expr &Cond, const ASTContext &Ctx,
+                               bool ExpectedValue) {
+  if (Cond.isValueDependent()) {
+    if (const auto *BinOp = dyn_cast<BinaryOperator>(&Cond)) {
+      // Conjunctions (disjunctions) can still be handled if at least one
+      // conjunct (disjunct) is known to be false (true).
+      if (!ExpectedValue && BinOp->getOpcode() == BO_LAnd)
+        return isKnownToHaveValue(*BinOp->getLHS(), Ctx, false) ||
+               isKnownToHaveValue(*BinOp->getRHS(), Ctx, false);
+      if (ExpectedValue && BinOp->getOpcode() == BO_LOr)
+        return isKnownToHaveValue(*BinOp->getLHS(), Ctx, true) ||
+               isKnownToHaveValue(*BinOp->getRHS(), Ctx, true);
+      if (BinOp->getOpcode() == BO_Comma)
+        return isKnownToHaveValue(*BinOp->getRHS(), Ctx, ExpectedValue);
+    } else if (const auto *UnOp = dyn_cast<UnaryOperator>(&Cond)) {
+      if (UnOp->getOpcode() == UO_LNot)
+        return isKnownToHaveValue(*UnOp->getSubExpr(), Ctx, !ExpectedValue);
+    } else if (const auto *Paren = dyn_cast<ParenExpr>(&Cond))
+      return isKnownToHaveValue(*Paren->getSubExpr(), Ctx, ExpectedValue);
+    else if (const auto *ImplCast = dyn_cast<ImplicitCastExpr>(&Cond))
+      return isKnownToHaveValue(*ImplCast->getSubExpr(), Ctx, ExpectedValue);
     return false;
+  }
   bool Result = false;
   if (Cond.EvaluateAsBooleanCondition(Result, Ctx))
-    return !Result;
+    return Result == ExpectedValue;
   return false;
 }
 
@@ -144,7 +164,7 @@ void InfiniteLoopCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *LoopStmt = Result.Nodes.getNodeAs<Stmt>("loop-stmt");
   const auto *Func = Result.Nodes.getNodeAs<Decl>("func");
 
-  if (isKnownFalse(*Cond, *Result.Context))
+  if (isKnownToHaveValue(*Cond, *Result.Context, false))
     return;
 
   bool ShouldHaveConditionVariables = true;

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 541ff89c14803..81becb0031a45 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -176,6 +176,9 @@ Changes in existing checks
 - Fixed incorrect suggestions for :doc:`readability-container-size-empty
   <clang-tidy/checks/readability-container-size-empty>` when smart pointers are involved.
 
+- Fixed some false positives in :doc:`bugprone-infinite-loop
+  <clang-tidy/checks/bugprone-infinite-loop>` involving dependent expressions.
+
 Removed checks
 ^^^^^^^^^^^^^^
 

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 ed50b5c7d74ea..0074266ce4b87 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
@@ -1,5 +1,5 @@
 // RUN: %check_clang_tidy %s bugprone-infinite-loop %t \
-// RUN:                   -- -- -fexceptions -fblocks
+// RUN:                   -- -- -fexceptions -fblocks -fno-delayed-template-parsing
 
 void simple_infinite_loop1() {
   int i = 0;
@@ -622,3 +622,31 @@ void test_volatile_concrete_address(int i, int size) {
     // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (size) are updated in the loop body [bugprone-infinite-loop]
   }
 }
+
+template <typename T>
+int some_template_fn() { return 1; }
+
+template <typename T>
+void test_dependent_condition() {
+  const int error = some_template_fn<T>();
+  do {
+  } while (false && error == 0);
+
+  const int val = some_template_fn<T>();
+  for (; !(val == 0 || true);) {
+  }
+
+  const int val2 = some_template_fn<T>();
+  for (; !(val2 == 0 || false);) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (val2) are updated in the loop body [bugprone-infinite-loop]
+  }
+
+  const int val3 = some_template_fn<T>();
+  do {
+    // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (val3) are updated in the loop body [bugprone-infinite-loop]
+  } while (1, (true) && val3 == 1);
+
+  const int val4 = some_template_fn<T>();
+  do {
+  } while (1, (false) && val4 == 1);
+}