diff --git a/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp index 07116a7ff15f5..e83e1b062eb2a 100644 --- a/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp @@ -50,7 +50,7 @@ static Matcher loopEndingStmt(Matcher 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(LoopStmt)) return (ForLoop->getInc() && @@ -83,6 +83,23 @@ static bool isVarThatIsPossiblyChanged(const Decl *Func, const Stmt *LoopStmt, isChanged(LoopStmt, Var, Context); // FIXME: Track references. } + + if (const auto *BD = dyn_cast(DRE->getDecl())) { + if (const auto *DD = + dyn_cast(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(Cond)) { // FIXME: Handle MemberExpr. @@ -124,6 +141,10 @@ static std::string getCondVarNames(const Stmt *Cond) { if (const auto *DRE = dyn_cast(Cond)) { if (const auto *Var = dyn_cast(DRE->getDecl())) return std::string(Var->getName()); + + if (const auto *BD = dyn_cast(DRE->getDecl())) { + return std::string(BD->getName()); + } } std::string Result; @@ -215,10 +236,17 @@ static bool overlap(ArrayRef SCC, /// returns true iff `Cond` involves at least one static local variable. static bool hasStaticLocalVariable(const Stmt *Cond) { - if (const auto *DRE = dyn_cast(Cond)) + if (const auto *DRE = dyn_cast(Cond)) { if (const auto *VD = dyn_cast(DRE->getDecl())) if (VD->isStaticLocal()) return true; + + if (const auto *BD = dyn_cast(DRE->getDecl())) + if (const auto *DD = dyn_cast(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(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(Var); VD && VD->isEscapingByref()) return true; if (const auto *DS = dyn_cast(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(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 19ccd1790e757..790ddfaef9103 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -159,6 +159,10 @@ Changes in existing checks false positives on deleted constructors that cannot be used to construct objects, even if they have public or protected access. +- Improved :doc:`bugprone-infinite-loop + ` check by adding detection for + variables introduced by structured bindings. + - Added an option to :doc:`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..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 @@ -711,3 +711,144 @@ 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 +struct pair { + T first; + U second; + + pair(T a, U b) : first(a), second(b) {} +}; +} + +template +void structured_binding_in_template_byval(T a, U b) { + auto [c, d] = std::pair(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 +void structured_binding_in_template_bylref(T a, U b) { + auto p = std::pair(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 +void structured_binding_in_template_byrref(T a, U b) { + auto p = std::pair(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); +}