[clang-tools-extra] e984d11 - [clang-tidy] loop convert can handle lambda init capture (#109159)

[via cfe-commits]

Author: Congcong Cai
Date: 2024-10-03T16:16:49+08:00
New Revision: e984d11d7257343da366d9fa03749a43a6d6af72

URL: https://github.com/llvm/llvm-project/commit/e984d11d7257343da366d9fa03749a43a6d6af72
DIFF: https://github.com/llvm/llvm-project/commit/e984d11d7257343da366d9fa03749a43a6d6af72.diff

LOG: [clang-tidy] loop convert can handle lambda init capture (#109159)

Fixes: #109083
Current implement ignore the whole lambda capture. This patch wants to
do traverse for capture init expr also

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
    clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
index c0bf4903ec3911..90c16bfddd84f8 100644
--- a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
@@ -777,7 +777,7 @@ bool ForLoopIndexUseVisitor::TraverseLambdaCapture(LambdaExpr *LE,
                                                    const LambdaCapture *C,
                                                    Expr *Init) {
   if (C->capturesVariable()) {
-    const ValueDecl *VDecl = C->getCapturedVar();
+    ValueDecl *VDecl = C->getCapturedVar();
     if (areSameVariable(IndexVar, VDecl)) {
       // FIXME: if the index is captured, it will count as an usage and the
       // alias (if any) won't work, because it is only used in case of having
@@ -787,6 +787,8 @@ bool ForLoopIndexUseVisitor::TraverseLambdaCapture(LambdaExpr *LE,
                                                        : Usage::UK_CaptureByRef,
                      C->getLocation()));
     }
+    if (VDecl->isInitCapture())
+      TraverseStmtImpl(cast<VarDecl>(VDecl)->getInit());
   }
   return VisitorBase::TraverseLambdaCapture(LE, C, Init);
 }
@@ -816,6 +818,17 @@ bool ForLoopIndexUseVisitor::VisitDeclStmt(DeclStmt *S) {
   return true;
 }
 
+bool ForLoopIndexUseVisitor::TraverseStmtImpl(Stmt *S) {
+  // All this pointer swapping is a mechanism for tracking immediate parentage
+  // of Stmts.
+  const Stmt *OldNextParent = NextStmtParent;
+  CurrStmtParent = NextStmtParent;
+  NextStmtParent = S;
+  bool Result = VisitorBase::TraverseStmt(S);
+  NextStmtParent = OldNextParent;
+  return Result;
+}
+
 bool ForLoopIndexUseVisitor::TraverseStmt(Stmt *S) {
   // If this is an initialization expression for a lambda capture, prune the
   // traversal so that we don't end up diagnosing the contained DeclRefExpr as
@@ -828,15 +841,7 @@ bool ForLoopIndexUseVisitor::TraverseStmt(Stmt *S) {
       return true;
     }
   }
-
-  // All this pointer swapping is a mechanism for tracking immediate parentage
-  // of Stmts.
-  const Stmt *OldNextParent = NextStmtParent;
-  CurrStmtParent = NextStmtParent;
-  NextStmtParent = S;
-  bool Result = VisitorBase::TraverseStmt(S);
-  NextStmtParent = OldNextParent;
-  return Result;
+  return TraverseStmtImpl(S);
 }
 
 std::string VariableNamer::createIndexName() {

diff  --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
index cfceb3b586842d..ca9c1855038b53 100644
--- a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
+++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
@@ -354,6 +354,8 @@ class ForLoopIndexUseVisitor
   bool VisitDeclStmt(DeclStmt *S);
   bool TraverseStmt(Stmt *S);
 
+  bool TraverseStmtImpl(Stmt *S);
+
   /// Add an expression to the list of expressions on which the container
   /// expression depends.
   void addComponent(const Expr *E);

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 702a8d2a3576b9..e4cef50dd26075 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -171,6 +171,10 @@ Changes in existing checks
   as a replacement for parameters of incomplete C array type in C++20 and 
   ``std::array`` or ``std::vector`` before C++20.
 
+- Improved :doc:`modernize-loop-convert
+  <clang-tidy/checks/modernize/loop-convert>` check to fix false positive when
+  using loop variable in initializer of lambda capture.
+
 - Improved :doc:`modernize-min-max-use-initializer-list
   <clang-tidy/checks/modernize/min-max-use-initializer-list>` check by fixing
   a false positive when only an implicit conversion happened inside an

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
index 695925a5d877c9..df2a2c1af1f547 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
@@ -980,3 +980,30 @@ namespace PR78381 {
     }
   }
 }
+
+namespace GH109083 {
+void test() {
+  const int N = 6;
+  int Arr[N] = {1, 2, 3, 4, 5, 6};
+
+  for (int I = 0; I < N; ++I) {
+    auto V = [T = Arr[I]]() {};
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead [modernize-loop-convert]
+  // CHECK-FIXES: for (int I : Arr)
+  // CHECK-FIXES-NEXT: auto V = [T = I]() {};
+  for (int I = 0; I < N; ++I) {
+    auto V = [T = 10 + Arr[I]]() {};
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead [modernize-loop-convert]
+  // CHECK-FIXES: for (int I : Arr)
+  // CHECK-FIXES-NEXT: auto V = [T = 10 + I]() {};
+
+  for (int I = 0; I < N; ++I) {
+    auto V = [T = I]() {};
+  }
+  for (int I = 0; I < N; ++I) {
+    auto V = [T = I + 10]() {};
+  }
+}
+} // namespace GH109083