Skip to content

Commit

Permalink
[clang-tidy] loop convert can handle lambda init capture (#109159)
Browse files Browse the repository at this point in the history
Fixes: #109083
Current implement ignore the whole lambda capture. This patch wants to
do traverse for capture init expr also
  • Loading branch information
HerrCai0907 authored Oct 3, 2024
1 parent eb4a91d commit e984d11
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 10 deletions.
25 changes: 15 additions & 10 deletions clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
}
Expand Down Expand Up @@ -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
Expand All @@ -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() {
Expand Down
2 changes: 2 additions & 0 deletions clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
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 @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit e984d11

Please sign in to comment.