https://github.com/flovent updated https://github.com/llvm/llvm-project/pull/144213
>From 6fce04d8484a3600f292a8bbdd8621db25be7f1d Mon Sep 17 00:00:00 2001 From: flovent <flb...@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 <flb...@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 <flb...@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 <flb...@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. _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits