https://github.com/a-tarasyuk updated https://github.com/llvm/llvm-project/pull/135573
>From 93c8fc7faba6ab9537039b8745e62f5d79785cd0 Mon Sep 17 00:00:00 2001 From: Oleksandr Tarasiuk <[email protected]> Date: Thu, 17 Apr 2025 23:58:35 +0300 Subject: [PATCH 1/5] [Clang] enhance loop analysis to handle variable changes inside lambdas --- clang/docs/ReleaseNotes.rst | 2 ++ clang/lib/Sema/SemaStmt.cpp | 19 ++++++++++-- clang/test/SemaCXX/warn-loop-analysis.cpp | 35 +++++++++++++++++++++++ 3 files changed, 54 insertions(+), 2 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index c75d83a6d1a7a..8a330c010a73d 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -385,6 +385,8 @@ Improvements to Clang's diagnostics Fixes #GH131127 +- The ``-Wloop-analysis`` warning now handles variable modifications inside lambda expressions (#GH132038). + Improvements to Clang's time-trace ---------------------------------- diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp index 39c2e157591df..2bb46dd94cca2 100644 --- a/clang/lib/Sema/SemaStmt.cpp +++ b/clang/lib/Sema/SemaStmt.cpp @@ -2002,9 +2002,24 @@ namespace { } void VisitDeclRefExpr(DeclRefExpr *E) { - if (VarDecl *VD = dyn_cast<VarDecl>(E->getDecl())) + if (VarDecl *VD = dyn_cast<VarDecl>(E->getDecl())) { if (Decls.count(VD)) FoundDecl = true; + } else if (CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(E->getDecl()); + MD && isLambdaCallOperator(MD)) { + for (const auto &Capture : MD->getParent()->captures()) { + if (!Capture.capturesVariable()) + continue; + + LambdaCaptureKind CK = Capture.getCaptureKind(); + if (CK != LCK_ByRef) + continue; + + VarDecl *VD = dyn_cast<VarDecl>(Capture.getCapturedVar()); + if (VD && Decls.count(VD)) + FoundDecl = true; + } + } } void VisitPseudoObjectExpr(PseudoObjectExpr *POE) { @@ -2021,7 +2036,7 @@ namespace { bool FoundDeclInUse() { return FoundDecl; } - }; // end class DeclMatcher + }; // end class DeclMatcher void CheckForLoopConditionalStatement(Sema &S, Expr *Second, Expr *Third, Stmt *Body) { diff --git a/clang/test/SemaCXX/warn-loop-analysis.cpp b/clang/test/SemaCXX/warn-loop-analysis.cpp index 324dd386292ac..ec2894a46ee77 100644 --- a/clang/test/SemaCXX/warn-loop-analysis.cpp +++ b/clang/test/SemaCXX/warn-loop-analysis.cpp @@ -299,3 +299,38 @@ void test10() { for (auto[i, j, k] = arr; i < a; ++i) { } for (auto[i, j, k] = arr; i < a; ++arr[0]) { } }; + +namespace GH132038 { +extern void foo(int); +void test1() { + int a = 0; + auto incr_a = [&a]() { ++a; }; + + for (int b = 10; a <= b; incr_a()) + foo(a); + + for (int b = 10; a <= b;) + incr_a(); + + for (int b = 10; a <= b; [&a]() { ++a; }()) { } + for (int b = 10; a <= b; [&a]() { }()) { } +} + +void test2() { + int a = 0; + auto incr_a = [a]() { }; + auto incr_b = [](int b) { }; + + for (int b = 10; a <= b; incr_a()) // expected-warning {{variables 'a' and 'b' used in loop condition not modified in loop body}} + foo(a); + + for (int b = 10; a <= b;) // expected-warning {{variables 'a' and 'b' used in loop condition not modified in loop body}} + incr_a(); + + for (int b = 10; a <= b; incr_b(b)) // expected-warning {{variables 'a' and 'b' used in loop condition not modified in loop body}} + foo(a); + + for (int b = 10; a <= b;) // expected-warning {{variables 'a' and 'b' used in loop condition not modified in loop body}} + incr_b(b); +} +} >From 00053790db55d3f26836e958f8624c0f4bf4fbb8 Mon Sep 17 00:00:00 2001 From: Oleksandr Tarasiuk <[email protected]> Date: Fri, 18 Apr 2025 18:28:16 +0300 Subject: [PATCH 2/5] add FIXME test case --- clang/test/SemaCXX/warn-loop-analysis.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/clang/test/SemaCXX/warn-loop-analysis.cpp b/clang/test/SemaCXX/warn-loop-analysis.cpp index ec2894a46ee77..7773bef0cd238 100644 --- a/clang/test/SemaCXX/warn-loop-analysis.cpp +++ b/clang/test/SemaCXX/warn-loop-analysis.cpp @@ -318,8 +318,11 @@ void test1() { void test2() { int a = 0; + int *c = &a; + auto incr_a = [a]() { }; auto incr_b = [](int b) { }; + auto incr_c = [c]() { ++*c; }; for (int b = 10; a <= b; incr_a()) // expected-warning {{variables 'a' and 'b' used in loop condition not modified in loop body}} foo(a); @@ -332,5 +335,9 @@ void test2() { for (int b = 10; a <= b;) // expected-warning {{variables 'a' and 'b' used in loop condition not modified in loop body}} incr_b(b); + + // FIXME: handle modification of loop control variable inside lambda body + for (a = 10; a <= 20; incr_c()) // expected-warning {{variable 'a' used in loop condition not modified in loop body}} + foo(a); } } >From b96435a39883975abb598ff56103a547ff221887 Mon Sep 17 00:00:00 2001 From: Oleksandr Tarasiuk <[email protected]> Date: Tue, 29 Apr 2025 00:21:54 +0300 Subject: [PATCH 3/5] . --- clang/lib/Sema/AnalysisBasedWarnings.cpp | 10 +++++++++- clang/lib/Sema/SemaStmt.cpp | 8 ++++---- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 34045a7274021..51f5cba24af5d 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -1025,6 +1025,8 @@ static bool DiagnoseUninitializedUse(Sema &S, const VarDecl *VD, return true; } +static void DiagnoseForLoopConditionVarsNotModified() {} + namespace { class FallthroughMapper : public DynamicRecursiveASTVisitor { public: @@ -2701,7 +2703,8 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings( .setAlwaysAdd(Stmt::CStyleCastExprClass) .setAlwaysAdd(Stmt::DeclRefExprClass) .setAlwaysAdd(Stmt::ImplicitCastExprClass) - .setAlwaysAdd(Stmt::UnaryOperatorClass); + .setAlwaysAdd(Stmt::UnaryOperatorClass) + .setAlwaysAdd(Stmt::ForStmtClass); } // Install the logical handler. @@ -2865,6 +2868,11 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings( if (S.getLangOpts().CPlusPlus && !fscope->isCoroutine() && isNoexcept(FD)) checkThrowInNonThrowingFunc(S, FD, AC); + if (!Diags.isIgnored(diag::warn_variables_not_in_loop_body, D->getBeginLoc())) { + D->dumpColor(); + DiagnoseForLoopConditionVarsNotModified(); + } + // If none of the previous checks caused a CFG build, trigger one here // for the logical error handler. if (LogicalErrorHandler::hasActiveDiagnostics(Diags, D->getBeginLoc())) { diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp index 2bb46dd94cca2..91fb052e70de8 100644 --- a/clang/lib/Sema/SemaStmt.cpp +++ b/clang/lib/Sema/SemaStmt.cpp @@ -2312,12 +2312,12 @@ StmtResult Sema::ActOnForStmt(SourceLocation ForLoc, SourceLocation LParenLoc, CheckBreakContinueBinding(Second.get().second); CheckBreakContinueBinding(third.get()); - - if (!Second.get().first) - CheckForLoopConditionalStatement(*this, Second.get().second, third.get(), - Body); CheckForRedundantIteration(*this, third.get(), Body); +// if (!Second.get().first) +// CheckForLoopConditionalStatement(*this, Second.get().second, third.get(), +// Body); + if (Second.get().second && !Diags.isIgnored(diag::warn_comma_operator, Second.get().second->getExprLoc())) >From e6f8c859e863bff8b9e561bf18da10af0b76fa2c Mon Sep 17 00:00:00 2001 From: Oleksandr Tarasiuk <[email protected]> Date: Wed, 7 Jan 2026 15:14:08 +0200 Subject: [PATCH 4/5] clarify -Wloop-analysis ReleaseNotes --- clang/docs/ReleaseNotes.rst | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 10cea25b98fa8..a713db2cfa2c5 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -352,6 +352,8 @@ Improvements to Clang's diagnostics - Improve the ``-Wundefined-func-template`` warning when a function template is not instantiated due to being unreachable in modules. +- The ``-Wloop-analysis`` warning has been extended to catch more cases of variable modification inside lambda expressions (#GH132038). + - Fixed an assertion when referencing an out-of-bounds parameter via a function attribute whose argument list refers to parameters by index and the function is variadic. e.g., @@ -395,7 +397,8 @@ Improvements to Clang's diagnostics constructors to initialize their non-modifiable members. The diagnostic is not new; being controlled via a warning group is what's new. Fixes #GH41104 -- The ``-Wloop-analysis`` warning now handles variable modifications inside lambda expressions (#GH132038). +- The ``-Wloop-analysis`` warning has been extended to catch more cases of + variable modification inside lambda expressions (#GH132038). Improvements to Clang's time-trace ---------------------------------- >From ab5906a34b91060131ae2e02c634304839238fab Mon Sep 17 00:00:00 2001 From: Oleksandr Tarasiuk <[email protected]> Date: Wed, 7 Jan 2026 15:14:28 +0200 Subject: [PATCH 5/5] add FIXME for lambda edge case --- clang/lib/Sema/SemaStmt.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp index 91fb052e70de8..2a9e043924f61 100644 --- a/clang/lib/Sema/SemaStmt.cpp +++ b/clang/lib/Sema/SemaStmt.cpp @@ -2007,6 +2007,15 @@ namespace { FoundDecl = true; } else if (CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(E->getDecl()); MD && isLambdaCallOperator(MD)) { + // FIXME: This has limitations handling updates to the loop control + // variable that occur indirectly inside a lambda called from the loop + // body. For example: + // + // int a = 0; + // int *c = &a; + // auto incr_c = [c]() { ++*c; }; + // for (a = 10; a <= 20; incr_c()) + // foo(a); for (const auto &Capture : MD->getParent()->captures()) { if (!Capture.capturesVariable()) continue; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
