[clang-tools-extra] [clang-tidy] Improve `bugprone-infinite-loop` check by adding handing for structured bindings (PR #144213)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Jul 17 06:33:54 PDT 2025
https://github.com/flovent updated https://github.com/llvm/llvm-project/pull/144213
>From 6fce04d8484a3600f292a8bbdd8621db25be7f1d Mon Sep 17 00:00:00 2001
From: flovent <flbven at protonmail.com>
Date: Sat, 14 Jun 2025 15:52:26 +0800
Subject: [PATCH 1/4] [clang-tidy] Improve `bugprone-infinite-loop` check by
adding handing for strucuted bindings
---
.../clang-tidy/bugprone/InfiniteLoopCheck.cpp | 33 +++++++-
.../clang-tidy/utils/Aliasing.cpp | 14 ++--
clang-tools-extra/clang-tidy/utils/Aliasing.h | 2 +-
clang-tools-extra/docs/ReleaseNotes.rst | 4 +
.../checkers/bugprone/infinite-loop.cpp | 81 +++++++++++++++++++
5 files changed, 124 insertions(+), 10 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
index 3c3024d538785..951d67ab5c21a 100644
--- a/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
@@ -49,7 +49,7 @@ static Matcher<Stmt> loopEndingStmt(Matcher<Stmt> Internal) {
}
/// Return whether `Var` was changed in `LoopStmt`.
-static bool isChanged(const Stmt *LoopStmt, const VarDecl *Var,
+static bool isChanged(const Stmt *LoopStmt, const ValueDecl *Var,
ASTContext *Context) {
if (const auto *ForLoop = dyn_cast<ForStmt>(LoopStmt))
return (ForLoop->getInc() &&
@@ -82,6 +82,23 @@ static bool isVarThatIsPossiblyChanged(const Decl *Func, const Stmt *LoopStmt,
isChanged(LoopStmt, Var, Context);
// FIXME: Track references.
}
+
+ if (const auto *BD = dyn_cast<BindingDecl>(DRE->getDecl())) {
+ if (const auto *DD =
+ dyn_cast_if_present<DecompositionDecl>(BD->getDecomposedDecl())) {
+ if (!DD->isLocalVarDeclOrParm())
+ return true;
+
+ if (DD->getType().isVolatileQualified())
+ return true;
+
+ if (!BD->getType().getTypePtr()->isIntegerType())
+ return true;
+
+ return hasPtrOrReferenceInFunc(Func, BD) ||
+ isChanged(LoopStmt, BD, Context);
+ }
+ }
} else if (isa<MemberExpr, CallExpr, ObjCIvarRefExpr, ObjCPropertyRefExpr,
ObjCMessageExpr>(Cond)) {
// FIXME: Handle MemberExpr.
@@ -123,6 +140,10 @@ static std::string getCondVarNames(const Stmt *Cond) {
if (const auto *DRE = dyn_cast<DeclRefExpr>(Cond)) {
if (const auto *Var = dyn_cast<VarDecl>(DRE->getDecl()))
return std::string(Var->getName());
+
+ if (const auto *BD = dyn_cast<BindingDecl>(DRE->getDecl())) {
+ return std::string(BD->getName());
+ }
}
std::string Result;
@@ -214,10 +235,18 @@ static bool overlap(ArrayRef<CallGraphNode *> SCC,
/// returns true iff `Cond` involves at least one static local variable.
static bool hasStaticLocalVariable(const Stmt *Cond) {
- if (const auto *DRE = dyn_cast<DeclRefExpr>(Cond))
+ if (const auto *DRE = dyn_cast<DeclRefExpr>(Cond)) {
if (const auto *VD = dyn_cast<VarDecl>(DRE->getDecl()))
if (VD->isStaticLocal())
return true;
+
+ if (const auto *BD = dyn_cast<BindingDecl>(DRE->getDecl()))
+ if (const auto *DD =
+ dyn_cast_if_present<DecompositionDecl>(BD->getDecomposedDecl()))
+ if (DD->isStaticLocal())
+ return true;
+ }
+
for (const Stmt *Child : Cond->children())
if (Child && hasStaticLocalVariable(Child))
return true;
diff --git a/clang-tools-extra/clang-tidy/utils/Aliasing.cpp b/clang-tools-extra/clang-tidy/utils/Aliasing.cpp
index 2facf0625605e..cbe4873b5c022 100644
--- a/clang-tools-extra/clang-tidy/utils/Aliasing.cpp
+++ b/clang-tools-extra/clang-tidy/utils/Aliasing.cpp
@@ -14,14 +14,14 @@
namespace clang::tidy::utils {
/// Return whether \p S is a reference to the declaration of \p Var.
-static bool isAccessForVar(const Stmt *S, const VarDecl *Var) {
+static bool isAccessForVar(const Stmt *S, const ValueDecl *Var) {
if (const auto *DRE = dyn_cast<DeclRefExpr>(S))
return DRE->getDecl() == Var;
return false;
}
-static bool capturesByRef(const CXXRecordDecl *RD, const VarDecl *Var) {
+static bool capturesByRef(const CXXRecordDecl *RD, const ValueDecl *Var) {
return llvm::any_of(RD->captures(), [Var](const LambdaCapture &C) {
return C.capturesVariable() && C.getCaptureKind() == LCK_ByRef &&
C.getCapturedVar() == Var;
@@ -29,9 +29,9 @@ static bool capturesByRef(const CXXRecordDecl *RD, const VarDecl *Var) {
}
/// Return whether \p Var has a pointer or reference in \p S.
-static bool isPtrOrReferenceForVar(const Stmt *S, const VarDecl *Var) {
+static bool isPtrOrReferenceForVar(const Stmt *S, const ValueDecl *Var) {
// Treat block capture by reference as a form of taking a reference.
- if (Var->isEscapingByref())
+ if (const auto *VD = dyn_cast<VarDecl>(Var); VD && VD->isEscapingByref())
return true;
if (const auto *DS = dyn_cast<DeclStmt>(S)) {
@@ -61,7 +61,7 @@ static bool isPtrOrReferenceForVar(const Stmt *S, const VarDecl *Var) {
}
/// Return whether \p Var has a pointer or reference in \p S.
-static bool hasPtrOrReferenceInStmt(const Stmt *S, const VarDecl *Var) {
+static bool hasPtrOrReferenceInStmt(const Stmt *S, const ValueDecl *Var) {
if (isPtrOrReferenceForVar(S, Var))
return true;
@@ -77,7 +77,7 @@ static bool hasPtrOrReferenceInStmt(const Stmt *S, const VarDecl *Var) {
}
static bool refersToEnclosingLambdaCaptureByRef(const Decl *Func,
- const VarDecl *Var) {
+ const ValueDecl *Var) {
const auto *MD = dyn_cast<CXXMethodDecl>(Func);
if (!MD)
return false;
@@ -89,7 +89,7 @@ static bool refersToEnclosingLambdaCaptureByRef(const Decl *Func,
return capturesByRef(RD, Var);
}
-bool hasPtrOrReferenceInFunc(const Decl *Func, const VarDecl *Var) {
+bool hasPtrOrReferenceInFunc(const Decl *Func, const ValueDecl *Var) {
return hasPtrOrReferenceInStmt(Func->getBody(), Var) ||
refersToEnclosingLambdaCaptureByRef(Func, Var);
}
diff --git a/clang-tools-extra/clang-tidy/utils/Aliasing.h b/clang-tools-extra/clang-tidy/utils/Aliasing.h
index 7dad16fc57f1e..6c0763b766805 100644
--- a/clang-tools-extra/clang-tidy/utils/Aliasing.h
+++ b/clang-tools-extra/clang-tidy/utils/Aliasing.h
@@ -25,7 +25,7 @@ namespace clang::tidy::utils {
/// For `f()` and `n` the function returns ``true`` because `p` is a
/// pointer to `n` created in `f()`.
-bool hasPtrOrReferenceInFunc(const Decl *Func, const VarDecl *Var);
+bool hasPtrOrReferenceInFunc(const Decl *Func, const ValueDecl *Var);
} // namespace clang::tidy::utils
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 9eb3835fe8340..3e3f52060e470 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -196,6 +196,10 @@ Changes in existing checks
<clang-tidy/checks/bugprone/exception-escape>` check to print stack trace
of a potentially escaped exception.
+- Improved :doc:`bugprone-infinite-loop
+ <clang-tidy/checks/bugprone/infinite-loop>` check by adding detection for
+ variables introduced by structured bindings.
+
- Added an option to :doc:`bugprone-multi-level-implicit-pointer-conversion
<clang-tidy/checks/bugprone/multi-level-implicit-pointer-conversion>` to
choose whether to enable the check in C code or not.
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..a8787593da6fe 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
@@ -711,3 +711,84 @@ void test_local_static_recursion() {
while (i >= 0)
p(0); // we don't know what p points to so no warning
}
+
+struct PairVal {
+ int a;
+ int b;
+ PairVal(int a, int b) : a(a), b(b) {}
+};
+
+void structured_binding_infinite_loop1() {
+ auto [x, y] = PairVal(0, 0);
+ while (x < 10) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (x) are updated in the loop body [bugprone-infinite-loop]
+ y++;
+ }
+ while (y < 10) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (y) are updated in the loop body [bugprone-infinite-loop]
+ x++;
+ }
+}
+
+void structured_binding_infinite_loop2() {
+ auto [x, y] = PairVal(0, 0);
+ while (x < 10) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (x) are updated in the loop body [bugprone-infinite-loop]
+ // No update to x or y
+ }
+ while (y < 10) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (y) are updated in the loop body [bugprone-infinite-loop]
+ // No update to x or y
+ }
+}
+
+void structured_binding_not_infinite1() {
+ auto [x, y] = PairVal(0, 0);
+ while (x < 10) {
+ x++;
+ }
+ while (y < 10) {
+ y++;
+ }
+}
+
+void volatile_structured_binding_in_condition() {
+ volatile auto [x, y] = PairVal(0, 0);
+ while (!x) {}
+}
+
+void test_local_static_structured_binding_recursion() {
+ static auto [i, _] = PairVal(0, 0);
+ int j = 0;
+
+ i--;
+ while (i >= 0)
+ test_local_static_structured_binding_recursion(); // no warning, recursively decrement i
+ for (; i >= 0;)
+ test_local_static_structured_binding_recursion(); // no warning, recursively decrement i
+ for (; i + j >= 0;)
+ test_local_static_structured_binding_recursion(); // no warning, recursively decrement i
+ for (; i >= 0; i--)
+ ; // no warning, i decrements
+ while (j >= 0)
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (j) are updated in the loop body [bugprone-infinite-loop]
+ test_local_static_structured_binding_recursion();
+
+ int (*p)(int) = 0;
+
+ while (i >= 0)
+ // 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]
+ p = 0;
+ while (i >= 0)
+ p(0); // we don't know what p points to so no warning
+}
+
+struct S { int a; };
+void issue_138842_reduced() {
+ int x = 10;
+ auto [y] = S{1};
+
+ while (y < x) {
+ y++;
+ }
+}
>From e64caa013c5324ecbcf0934654eded45cb8d9e06 Mon Sep 17 00:00:00 2001
From: flovent <flbven at protonmail.com>
Date: Sat, 14 Jun 2025 18:36:30 +0800
Subject: [PATCH 2/4] dyn_cast_if_present to dyn_cast
---
clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
index 951d67ab5c21a..072c337300719 100644
--- a/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
@@ -85,7 +85,7 @@ static bool isVarThatIsPossiblyChanged(const Decl *Func, const Stmt *LoopStmt,
if (const auto *BD = dyn_cast<BindingDecl>(DRE->getDecl())) {
if (const auto *DD =
- dyn_cast_if_present<DecompositionDecl>(BD->getDecomposedDecl())) {
+ dyn_cast<DecompositionDecl>(BD->getDecomposedDecl())) {
if (!DD->isLocalVarDeclOrParm())
return true;
@@ -241,8 +241,7 @@ static bool hasStaticLocalVariable(const Stmt *Cond) {
return true;
if (const auto *BD = dyn_cast<BindingDecl>(DRE->getDecl()))
- if (const auto *DD =
- dyn_cast_if_present<DecompositionDecl>(BD->getDecomposedDecl()))
+ if (const auto *DD = dyn_cast<DecompositionDecl>(BD->getDecomposedDecl()))
if (DD->isStaticLocal())
return true;
}
>From 6023b0709a26d4d17e9c9d0459e10cf0dd0c630a Mon Sep 17 00:00:00 2001
From: flovent <flbven at protonmail.com>
Date: Tue, 24 Jun 2025 19:58:11 +0800
Subject: [PATCH 3/4] add test for structured binding in template
---
.../checkers/bugprone/infinite-loop.cpp | 60 +++++++++++++++++++
1 file changed, 60 insertions(+)
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 a8787593da6fe..3365ff46ffeb6 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
@@ -792,3 +792,63 @@ void issue_138842_reduced() {
y++;
}
}
+
+namespace std {
+template <typename T, typename U>
+struct pair {
+ T first;
+ U second;
+
+ pair(T a, U b) : first(a), second(b) {}
+};
+}
+
+template <typename T, typename U>
+void structured_binding_in_template_byval(T a, U b) {
+ auto [c, d] = std::pair<T, U>(a,b);
+
+ while (c < 10) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (c) are updated in the loop body [bugprone-infinite-loop]
+ d++;
+ }
+
+ while (c < 10) {
+ c++; // no warning
+ }
+}
+
+template <typename T, typename U>
+void structured_binding_in_template_bylref(T a, U b) {
+ auto p = std::pair<T, U>(a,b);
+ auto& [c, d] = p;
+
+ while (c < 10) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (c) are updated in the loop body [bugprone-infinite-loop]
+ d++;
+ }
+
+ while (c < 10) {
+ c++; // no warning
+ }
+}
+
+template <typename T, typename U>
+void structured_binding_in_template_byrref(T a, U b) {
+ auto p = std::pair<T, U>(a,b);
+ auto&& [c, d] = p;
+
+ while (c < 10) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (c) are updated in the loop body [bugprone-infinite-loop]
+ d++;
+ }
+
+ while (c < 10) {
+ c++; // no warning
+ }
+}
+
+void structured_binding_in_template_instantiation(int b) {
+ structured_binding_in_template_byval(b, 0);
+ structured_binding_in_template_bylref(b, 0);
+ structured_binding_in_template_byrref(b, 0);
+}
>From 87a68f7ca9144f05eb37a51abc537af2fcb53fb2 Mon Sep 17 00:00:00 2001
From: flovent <flbven at protonmail.com>
Date: Thu, 17 Jul 2025 21:33:09 +0800
Subject: [PATCH 4/4] (NFC) avoid duplicated code
---
.../clang-tidy/bugprone/InfiniteLoopCheck.cpp | 54 +++++++++----------
1 file changed, 24 insertions(+), 30 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
index 072c337300719..4b495e3877000 100644
--- a/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
@@ -64,41 +64,35 @@ static bool isChanged(const Stmt *LoopStmt, const ValueDecl *Var,
return ExprMutationAnalyzer(*LoopStmt, *Context).isMutated(Var);
}
-/// 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<DeclRefExpr>(Cond)) {
- if (const auto *Var = dyn_cast<VarDecl>(DRE->getDecl())) {
- if (!Var->isLocalVarDeclOrParm())
- return true;
-
- if (Var->getType().isVolatileQualified())
- return true;
-
- if (!Var->getType().getTypePtr()->isIntegerType())
- return true;
+static bool isVarPossiblyChanged(const Decl *Func, const Stmt *LoopStmt,
+ const ValueDecl *VD, ASTContext *Context) {
+ const VarDecl *Var = nullptr;
+ if (const auto *VarD = dyn_cast<VarDecl>(VD)) {
+ Var = VarD;
+ } else if (const auto *BD = dyn_cast<BindingDecl>(VD)) {
+ if (const auto *DD = dyn_cast<DecompositionDecl>(BD->getDecomposedDecl()))
+ Var = DD;
+ }
- return hasPtrOrReferenceInFunc(Func, Var) ||
- isChanged(LoopStmt, Var, Context);
- // FIXME: Track references.
- }
+ if (!Var)
+ return false;
- if (const auto *BD = dyn_cast<BindingDecl>(DRE->getDecl())) {
- if (const auto *DD =
- dyn_cast<DecompositionDecl>(BD->getDecomposedDecl())) {
- if (!DD->isLocalVarDeclOrParm())
- return true;
+ if (!Var->isLocalVarDeclOrParm() || Var->getType().isVolatileQualified())
+ return true;
- if (DD->getType().isVolatileQualified())
- return true;
+ if (!VD->getType().getTypePtr()->isIntegerType())
+ return true;
- if (!BD->getType().getTypePtr()->isIntegerType())
- return true;
+ return hasPtrOrReferenceInFunc(Func, VD) || isChanged(LoopStmt, VD, Context);
+ // FIXME: Track references.
+}
- return hasPtrOrReferenceInFunc(Func, BD) ||
- isChanged(LoopStmt, BD, 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<DeclRefExpr>(Cond)) {
+ if (const auto *VD = dyn_cast<ValueDecl>(DRE->getDecl()))
+ return isVarPossiblyChanged(Func, LoopStmt, VD, Context);
} else if (isa<MemberExpr, CallExpr, ObjCIvarRefExpr, ObjCPropertyRefExpr,
ObjCMessageExpr>(Cond)) {
// FIXME: Handle MemberExpr.
More information about the cfe-commits
mailing list