Index: clang-tidy/modernize/LoopConvertUtils.h =================================================================== --- clang-tidy/modernize/LoopConvertUtils.h +++ clang-tidy/modernize/LoopConvertUtils.h @@ -277,6 +277,9 @@ /// \brief Accessor for Usages. const UsageResult &getUsages() const { return Usages; } + /// \brief Adds the Usage if it was not added before. + void addUsage(const Usage &U); + /// \brief Get the container indexed by IndexVar, if any. const Expr *getContainerIndexed() const { return ContainerExpr; } @@ -336,6 +339,7 @@ /// A container which holds all usages of IndexVar as the index of /// ArraySubscriptExpressions. UsageResult Usages; + llvm::SmallSet UsageLocations; bool OnlyUsedAsIndex; /// The DeclStmt for an alias to the container element. const DeclStmt *AliasDecl; Index: clang-tidy/modernize/LoopConvertUtils.cpp =================================================================== --- clang-tidy/modernize/LoopConvertUtils.cpp +++ clang-tidy/modernize/LoopConvertUtils.cpp @@ -460,6 +460,15 @@ DependentExprs.push_back(std::make_pair(Node, ID)); } +void ForLoopIndexUseVisitor::addUsage(const Usage &U) { + SourceLocation Begin = U.Range.getBegin(); + if (Begin.isMacroID()) + Begin = Context->getSourceManager().getSpellingLoc(Begin); + + if (UsageLocations.insert(Begin).second) + Usages.push_back(U); +} + /// \brief If the unary operator is a dereference of IndexVar, include it /// as a valid usage and prune the traversal. /// @@ -475,7 +484,7 @@ // If we dereference an iterator that's actually a pointer, count the // occurrence. if (isDereferenceOfUop(Uop, IndexVar)) { - Usages.push_back(Usage(Uop)); + addUsage(Usage(Uop)); return true; } @@ -549,8 +558,8 @@ // If something complicated is happening (i.e. the next token isn't an // arrow), give up on making this work. if (!ArrowLoc.isInvalid()) { - Usages.push_back(Usage(ResultExpr, /*IsArrow=*/true, - SourceRange(Base->getExprLoc(), ArrowLoc))); + addUsage(Usage(ResultExpr, /*IsArrow=*/true, + SourceRange(Base->getExprLoc(), ArrowLoc))); return true; } } @@ -579,7 +588,7 @@ if (isIndexInSubscriptExpr(Context, MemberCall->getArg(0), IndexVar, Member->getBase(), ContainerExpr, ContainerNeedsDereference)) { - Usages.push_back(Usage(MemberCall)); + addUsage(Usage(MemberCall)); return true; } } @@ -614,7 +623,7 @@ switch (OpCall->getOperator()) { case OO_Star: if (isDereferenceOfOpCall(OpCall, IndexVar)) { - Usages.push_back(Usage(OpCall)); + addUsage(Usage(OpCall)); return true; } break; @@ -625,7 +634,7 @@ if (isIndexInSubscriptExpr(Context, OpCall->getArg(1), IndexVar, OpCall->getArg(0), ContainerExpr, ContainerNeedsDereference)) { - Usages.push_back(Usage(OpCall)); + addUsage(Usage(OpCall)); return true; } break; @@ -674,7 +683,7 @@ if (!ContainerExpr) ContainerExpr = Arr; - Usages.push_back(Usage(E)); + addUsage(Usage(E)); return true; } @@ -746,12 +755,12 @@ bool ForLoopIndexUseVisitor::TraverseLambdaCapture(LambdaExpr *LE, const LambdaCapture *C) { if (C->capturesVariable()) { - const VarDecl* VDecl = C->getCapturedVar(); + const VarDecl *VDecl = C->getCapturedVar(); if (areSameVariable(IndexVar, cast(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 // exactly one usage. - Usages.push_back(Usage(nullptr, false, C->getLocation())); + addUsage(Usage(nullptr, false, C->getLocation())); } } return VisitorBase::TraverseLambdaCapture(LE, C); Index: test/clang-tidy/modernize-loop-convert-extra.cpp =================================================================== --- test/clang-tidy/modernize-loop-convert-extra.cpp +++ test/clang-tidy/modernize-loop-convert-extra.cpp @@ -884,3 +884,32 @@ } } // namespace Lambdas + +namespace InitLists { + +struct D { int i; }; +struct E { D d; }; +int g(int b); + +void f() { + const unsigned N = 3; + int Array[N]; + + // Subtrees of InitListExpr are visited twice. Test that we do not do repeated + // replacements. + for (unsigned i = 0; i < N; ++i) { + int a{ Array[i] }; + int b{ g(Array[i]) }; + int c{ g( { Array[i] } ) }; + D d{ { g( { Array[i] } ) } }; + E e{ { { g( { Array[i] } ) } } }; + } + // CHECK-MESSAGES: :[[@LINE-7]]:3: warning: use range-based for loop instead + // CHECK-FIXES: int a{ elem }; + // CHECK-FIXES-NEXT: int b{ g(elem) }; + // CHECK-FIXES-NEXT: int c{ g( { elem } ) }; + // CHECK-FIXES-NEXT: D d{ { g( { elem } ) } }; + // CHECK-FIXES-NEXT: E e{ { { g( { elem } ) } } }; +} + +} // namespace InitLists