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 <agoe...@apple.com>
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<RValueReferenceType>(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<DeclRefExpr>(Cond)) {
- if (const auto *Var = dyn_cast<VarDecl>(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<VarDecl>(VD)) {
+ return isVarDeclPossiblyChanged(Func, LoopStmt, Var, Context);
+ }
- return hasPtrOrReferenceInFunc(Func, Var) ||
- isChanged(LoopStmt, Var, Context);
- // FIXME: Track references.
+ if (const auto *BD = dyn_cast<BindingDecl>(VD)) {
+ if (const auto *Var = BD->getHoldingVar()) {
+ return isVarDeclPossiblyChanged(Func, LoopStmt, Var, Context);
+ }
}
+
+ // FIXME: Track references.
} else if (isa<MemberExpr, CallExpr, ObjCIvarRefExpr, ObjCPropertyRefExpr,
ObjCMessageExpr>(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<DeclRefExpr>(Cond)) {
- if (const auto *Var = dyn_cast<VarDecl>(DRE->getDecl()))
+ const ValueDecl *VD = DRE->getDecl();
+
+ if (const auto *Var = dyn_cast<VarDecl>(VD))
return std::string(Var->getName());
+
+ if (const auto *BD = dyn_cast<BindingDecl>(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 <class> struct tuple_size;
+ template <std::size_t, class> struct tuple_element;
+ template <class...> class tuple;
+
+namespace {
+ template <class T, T v>
+ struct size_helper { static const T value = v; };
+} // namespace
+
+template <class... T>
+struct tuple_size<tuple<T...>> : size_helper<std::size_t, sizeof...(T)> {};
+
+template <std::size_t I, class... T>
+struct tuple_element<I, tuple<T...>> {
+ using type = __type_pack_element<I, T...>;
+};
+
+template <class...> class tuple {};
+
+template <std::size_t I, class... T>
+typename tuple_element<I, tuple<T...>>::type get(tuple<T...>);
+} // namespace std
+
+std::tuple<int*, int> &get_chunk();
+
+void test_structured_bindings_tuple() {
+ auto [buffer, size ] = get_chunk();
+ int 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
+ buffer[size++] = 2;
+ }
+}
+
+void test_structured_bindings_tuple_ref() {
+ auto& [buffer, size ] = get_chunk();
+ int 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
+ buffer[size++] = 2;
+ }
+}
+
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;) {
diff --git a/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
b/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
index 3344959072c22..441bc56b29de8 100644
--- a/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
+++ b/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
@@ -54,6 +54,8 @@ class ExprMutationAnalyzer {
const Stmt *findMutationMemoized(const Expr *Exp,
llvm::ArrayRef<MutationFinder> Finders,
Memoized::ResultMap &MemoizedResults);
+ const ast_matchers::internal::BindableMatcher<Stmt>
+ makeDeclRefExprMatcher(const Decl *Dec);
const Stmt *tryEachDeclRef(const Decl *Dec, MutationFinder Finder);
const Stmt *findExprMutation(ArrayRef<ast_matchers::BoundNodes> Matches);
diff --git a/clang/lib/Analysis/ExprMutationAnalyzer.cpp
b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
index 823d7543f085f..fb36d9a4cea64 100644
--- a/clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -113,6 +113,14 @@ AST_MATCHER_P(Stmt, canResolveToExpr, const Stmt *, Inner)
{
return canExprResolveTo(Exp, Target);
}
+AST_MATCHER_P(BindingDecl, hasHoldingVar,
+ ast_matchers::internal::Matcher<VarDecl>, InnerMatcher) {
+ if (const VarDecl *HoldingVar = Node.getHoldingVar()) {
+ return InnerMatcher.matches(*HoldingVar, Finder, Builder);
+ }
+ return false;
+}
+
// use class member to store data can reduce stack usage to avoid stack
overflow
// when recursive call.
class ExprPointeeResolve {
@@ -310,21 +318,30 @@ const Stmt
*ExprMutationAnalyzer::Analyzer::findMutationMemoized(
return nullptr;
}
+const ast_matchers::internal::BindableMatcher<Stmt>
+ExprMutationAnalyzer::Analyzer::makeDeclRefExprMatcher(const Decl *Dec) {
+
+ // For VarDecl created implicitly via structured bindings, create a matcher
+ // for DeclRefExpr nodes which refer to this VarDecl via BindingDecl nodes.
+ if (const auto *VD = dyn_cast<VarDecl>(Dec)) {
+ if (VD->isImplicit()) {
+ return declRefExpr(to(bindingDecl(hasHoldingVar(equalsNode(VD)))));
+ }
+ }
+
+ return declRefExpr(to(
+ // `Dec` or a binding if `Dec` is a decomposition.
+ anyOf(equalsNode(Dec), bindingDecl(forDecomposition(equalsNode(Dec))))));
+}
+
const Stmt *
ExprMutationAnalyzer::Analyzer::tryEachDeclRef(const Decl *Dec,
MutationFinder Finder) {
- const auto Refs = match(
- findAll(
- declRefExpr(to(
- // `Dec` or a binding if `Dec` is a decomposition.
- anyOf(equalsNode(Dec),
- bindingDecl(forDecomposition(equalsNode(Dec))))
- //
- ))
- .bind(NodeID<Expr>::value)),
- Stm, Context);
+ const auto matcher = makeDeclRefExprMatcher(Dec);
+ const auto nodeId = NodeID<Expr>::value;
+ const auto Refs = match(findAll(matcher.bind(nodeId)), Stm, Context);
for (const auto &RefNodes : Refs) {
- const auto *E = RefNodes.getNodeAs<Expr>(NodeID<Expr>::value);
+ const auto *E = RefNodes.getNodeAs<Expr>(nodeId);
if ((this->*Finder)(E))
return E;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
