Skip to content

[clang-tidy] Improve bugprone-infinite-loop check by adding handing for structured bindings #144213

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 22, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 38 additions & 16 deletions clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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() &&
Expand All @@ -64,24 +64,35 @@ static bool isChanged(const Stmt *LoopStmt, const VarDecl *Var,
return ExprMutationAnalyzer(*LoopStmt, *Context).isMutated(Var);
}

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;
}

if (!Var)
return false;

if (!Var->isLocalVarDeclOrParm() || Var->getType().isVolatileQualified())
return true;

if (!VD->getType().getTypePtr()->isIntegerType())
return true;

return hasPtrOrReferenceInFunc(Func, VD) || isChanged(LoopStmt, VD, Context);
// FIXME: Track references.
}

/// 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;

return hasPtrOrReferenceInFunc(Func, Var) ||
isChanged(LoopStmt, Var, Context);
// FIXME: Track references.
}
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.
Expand Down Expand Up @@ -123,6 +134,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;
Expand Down Expand Up @@ -214,10 +229,17 @@ 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<DecompositionDecl>(BD->getDecomposedDecl()))
if (DD->isStaticLocal())
return true;
}

for (const Stmt *Child : Cond->children())
if (Child && hasStaticLocalVariable(Child))
return true;
Expand Down
14 changes: 7 additions & 7 deletions clang-tools-extra/clang-tidy/utils/Aliasing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,24 +14,24 @@
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;
});
}

/// 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)) {
Expand Down Expand Up @@ -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;

Expand All @@ -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;
Expand All @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion clang-tools-extra/clang-tidy/utils/Aliasing.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 4 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,10 @@ New check aliases
Changes in existing checks
^^^^^^^^^^^^^^^^^^^^^^^^^^

- Improved :doc:`bugprone-infinite-loop
<clang-tidy/checks/bugprone/infinite-loop>` check by adding detection for
variables introduced by structured bindings.

Removed checks
^^^^^^^^^^^^^^

Expand Down
202 changes: 202 additions & 0 deletions clang-tools-extra/test/clang-tidy/checkers/bugprone/infinite-loop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -711,3 +711,205 @@ 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++;
}
}

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);
}

void array_structured_binding() {
int arr[2] = {0, 0};
auto [x, y] = arr;

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) {
y++; // no warning
}
}

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;
}
}
Loading