[clang-tools-extra] [clang-tidy] bugprone-infinite-loop: Add support for tuple structured bindings (PR #147410)

2025-07-07 Thread Aviral Goel via cfe-commits

https://github.com/aviralg created 
https://github.com/llvm/llvm-project/pull/147410

Tuple structured bindings can introduce new variable declarations under 
`BindingDecl` nodes which are currently ignored by the infinite loop checker. 
This PR implements support for extracting variables from these nodes. 
Subsequent logic for checking if these variables have changed has been reused.

>From 782095442544692d3f0e32ed5da4ee1d772c1f9a Mon Sep 17 00:00:00 2001
From: Aviral Goel 
Date: Mon, 7 Jul 2025 14:38:00 -0700
Subject: [PATCH] [clang-tidy] bugprone-infinite-loop: Add support for tuple
 structured bindings

Tuple structured bindings can introduce new variable declarations under
`BindingDecl` nodes which are currently ignored by the infinite loop checker.
This PR implements support for extracting variables from these nodes.
Subsequent logic for checking if these variables have changed has been reused.
---
 .../clang-tidy/bugprone/InfiniteLoopCheck.cpp | 35 +--
 .../checkers/bugprone/infinite-loop.cpp   | 15 
 2 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
index 07116a7ff15f5..2ae0dbc3f53ba 100644
--- a/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
@@ -65,24 +65,37 @@ static bool isChanged(const Stmt *LoopStmt, const VarDecl 
*Var,
   return ExprMutationAnalyzer(*LoopStmt, *Context).isMutated(Var);
 }
 
+static bool isVarDeclPossiblyChanged(const Decl *Func, const Stmt *LoopStmt,
+ const VarDecl *Var, ASTContext *Context) {
+  if (!Var->isLocalVarDeclOrParm())
+return true;
+
+  if (Var->getType().isVolatileQualified())
+return true;
+
+  if (!Var->getType().getTypePtr()->isIntegerType())
+return true;
+
+  return hasPtrOrReferenceInFunc(Func, Var) ||
+ isChanged(LoopStmt, Var, Context);
+}
+
 /// Return whether `Cond` is a variable that is possibly changed in `LoopStmt`.
 static bool isVarThatIsPossiblyChanged(const Decl *Func, const Stmt *LoopStmt,
const Stmt *Cond, ASTContext *Context) {
   if (const auto *DRE = dyn_cast(Cond)) {
-if (const auto *Var = dyn_cast(DRE->getDecl())) {
-  if (!Var->isLocalVarDeclOrParm())
-return true;
-
-  if (Var->getType().isVolatileQualified())
-return true;
+const ValueDecl *VD = DRE->getDecl();
 
-  if (!Var->getType().getTypePtr()->isIntegerType())
-return true;
+if (const auto *Var = dyn_cast(VD)) {
+  return isVarDeclPossiblyChanged(Func, LoopStmt, Var, Context);
+}
 
-  return hasPtrOrReferenceInFunc(Func, Var) ||
- isChanged(LoopStmt, Var, Context);
-  // FIXME: Track references.
+if (const auto *BD = dyn_cast(VD)) {
+  return isVarDeclPossiblyChanged(Func, LoopStmt, BD->getHoldingVar(),
+  Context);
 }
+
+// FIXME: Track references.
   } else if (isa(Cond)) {
 // FIXME: Handle MemberExpr.
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 bc14ece3f332c..e5d3a7da0d763 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
@@ -592,6 +592,21 @@ void test_structured_bindings_bad() {
   }
 }
 
+std::tuple get_chunk() {
+  static uint8_t buf[10];
+  return { &buf[0], 2 };
+}
+
+void test_structured_bindings_tuple() {
+  auto [ p, size ] = get_chunk();
+  size_t maxLen = 8;
+
+  while (size < maxLen) {
+// No warning. The loop is finite because 'size' is being incremented in 
each iteration and compared against 'maxLen' for termination
+p[size++] = ' ';
+  }
+}
+
 void test_volatile_cast() {
   // This is a no-op cast. Clang ignores the qualifier, we should too.
   for (int i = 0; (volatile int)i < 10;) {

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-tools-extra] [clang-tidy] bugprone-infinite-loop: Add support for tuple structured bindings (PR #147410)

2025-07-16 Thread Aviral Goel via cfe-commits

https://github.com/aviralg updated 
https://github.com/llvm/llvm-project/pull/147410

>From be9fc41e5b1b52cf3f0bfb6acc4debae5561575a Mon Sep 17 00:00:00 2001
From: Aviral Goel 
Date: Mon, 7 Jul 2025 14:38:00 -0700
Subject: [PATCH] [clang-tidy] bugprone-infinite-loop: Add support for tuple
 structured bindings

Tuple structured bindings introduce implicit variable declarations under
`BindingDecl` nodes which are currently ignored by the infinite loop checker.
This PR adds support for these bindings to this checker through the following
changes:

1. The pattern matcher in `ExprMutationAnalyzer` has been updated to match
against `DeclRefExpr` nodes that refer to these implicit variables via
`BindingDecl` nodes.

2. Enumeration of a loop's condition's variables for mutation analysis has been
updated to recognize these implicit variables so they can be checked for
mutation.

3. Enumeration of the names of a loop's condition's variables for error
reporting has been similarly updated.

The changes have been tested against a mock tuple implementation lifted from
`clang/unittests/Analysis/FlowSensitive/TransferTest.cpp`
---
 .../clang-tidy/bugprone/InfiniteLoopCheck.cpp | 61 +++
 .../checkers/bugprone/infinite-loop.cpp   | 47 ++
 .../Analysis/Analyses/ExprMutationAnalyzer.h  |  2 +
 clang/lib/Analysis/ExprMutationAnalyzer.cpp   | 39 
 4 files changed, 126 insertions(+), 23 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
index 3c3024d538785..a0011eab4251b 100644
--- a/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
@@ -64,24 +64,53 @@ static bool isChanged(const Stmt *LoopStmt, const VarDecl 
*Var,
   return ExprMutationAnalyzer(*LoopStmt, *Context).isMutated(Var);
 }
 
+bool isVolatileOrNonIntegerType(QualType QT) {
+
+  if (QT.isVolatileQualified())
+return true;
+
+  const Type *T = QT.getTypePtr();
+
+  if (T->isIntegerType())
+return false;
+
+  if (T->isRValueReferenceType()) {
+QT = dyn_cast(T)->getPointeeType();
+return isVolatileOrNonIntegerType(QT);
+  }
+
+  return true;
+}
+
+static bool isVarDeclPossiblyChanged(const Decl *Func, const Stmt *LoopStmt,
+ const VarDecl *Var, ASTContext *Context) {
+  if (!Var->isLocalVarDeclOrParm())
+return true;
+
+  if (isVolatileOrNonIntegerType(Var->getType()))
+return true;
+
+  return hasPtrOrReferenceInFunc(Func, Var) ||
+ isChanged(LoopStmt, Var, Context);
+}
+
 /// Return whether `Cond` is a variable that is possibly changed in `LoopStmt`.
 static bool isVarThatIsPossiblyChanged(const Decl *Func, const Stmt *LoopStmt,
const Stmt *Cond, ASTContext *Context) {
   if (const auto *DRE = dyn_cast(Cond)) {
-if (const auto *Var = dyn_cast(DRE->getDecl())) {
-  if (!Var->isLocalVarDeclOrParm())
-return true;
+const ValueDecl *VD = DRE->getDecl();
 
-  if (Var->getType().isVolatileQualified())
-return true;
-
-  if (!Var->getType().getTypePtr()->isIntegerType())
-return true;
+if (const auto *Var = dyn_cast(VD)) {
+  return isVarDeclPossiblyChanged(Func, LoopStmt, Var, Context);
+}
 
-  return hasPtrOrReferenceInFunc(Func, Var) ||
- isChanged(LoopStmt, Var, Context);
-  // FIXME: Track references.
+if (const auto *BD = dyn_cast(VD)) {
+  if (const auto *Var = BD->getHoldingVar()) {
+return isVarDeclPossiblyChanged(Func, LoopStmt, Var, Context);
+  }
 }
+
+// FIXME: Track references.
   } else if (isa(Cond)) {
 // FIXME: Handle MemberExpr.
@@ -121,8 +150,16 @@ static bool isAtLeastOneCondVarChanged(const Decl *Func, 
const Stmt *LoopStmt,
 /// Return the variable names in `Cond`.
 static std::string getCondVarNames(const Stmt *Cond) {
   if (const auto *DRE = dyn_cast(Cond)) {
-if (const auto *Var = dyn_cast(DRE->getDecl()))
+const ValueDecl *VD = DRE->getDecl();
+
+if (const auto *Var = dyn_cast(VD))
   return std::string(Var->getName());
+
+if (const auto *BD = dyn_cast(VD)) {
+  if (const auto *Var = BD->getHoldingVar()) {
+return Var->getName().str();
+  }
+}
   }
 
   std::string Result;
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 bc14ece3f332c..0895b91f77d9b 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
@@ -592,6 +592,53 @@ void test_structured_bindings_bad() {
   }
 }
 
+namespace std {
+using size_t = int;
+template  struct tuple_size;
+template  struct tuple_element;
+template  class tuple;
+
+namespace {
+template 
+struct size_helper { sta

[clang] [clang-tools-extra] [clang-tidy] bugprone-infinite-loop: Add support for tuple structured bindings (PR #147410)

2025-07-16 Thread Aviral Goel via cfe-commits

https://github.com/aviralg ready_for_review 
https://github.com/llvm/llvm-project/pull/147410
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits