Index: clang-tidy/modernize/LoopConvertUtils.h =================================================================== --- clang-tidy/modernize/LoopConvertUtils.h +++ clang-tidy/modernize/LoopConvertUtils.h @@ -205,6 +205,18 @@ : Expression(E), IsArrow(false), Range(Expression->getSourceRange()) {} Usage(const Expr *E, bool IsArrow, SourceRange Range) : Expression(E), IsArrow(IsArrow), Range(std::move(Range)) {} + + // Comparators for llvm::SmallSet. + bool operator<(const Usage &Other) const { + return std::make_tuple(Expression, IsArrow, Range.getBegin(), + Range.getEnd()) < + std::make_tuple(Other.Expression, Other.IsArrow, + Other.Range.getBegin(), Other.Range.getEnd()); + } + bool operator==(const Usage &Other) const { + return Expression == Other.Expression && IsArrow == Other.IsArrow && + Range == Other.Range; + } }; /// \brief A class to encapsulate lowering of the tool's confidence level. @@ -277,6 +289,9 @@ /// \brief Accessor for Usages. const UsageResult &getUsages() const { return Usages; } + /// \brief Returns true and adds the Usage if it was not added before. + bool addUsage(const Usage &U); + /// \brief Get the container indexed by IndexVar, if any. const Expr *getContainerIndexed() const { return ContainerExpr; } @@ -336,6 +351,7 @@ /// A container which holds all usages of IndexVar as the index of /// ArraySubscriptExpressions. UsageResult Usages; + llvm::SmallSet UsageSet; 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,14 @@ DependentExprs.push_back(std::make_pair(Node, ID)); } +bool ForLoopIndexUseVisitor::addUsage(const Usage &U) { + if (UsageSet.insert(U).second) { + Usages.push_back(U); + return true; + } + return false; +} + /// \brief If the unary operator is a dereference of IndexVar, include it /// as a valid usage and prune the traversal. /// @@ -475,7 +483,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 +557,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 +587,7 @@ if (isIndexInSubscriptExpr(Context, MemberCall->getArg(0), IndexVar, Member->getBase(), ContainerExpr, ContainerNeedsDereference)) { - Usages.push_back(Usage(MemberCall)); + addUsage(Usage(MemberCall)); return true; } } @@ -614,7 +622,7 @@ switch (OpCall->getOperator()) { case OO_Star: if (isDereferenceOfOpCall(OpCall, IndexVar)) { - Usages.push_back(Usage(OpCall)); + addUsage(Usage(OpCall)); return true; } break; @@ -625,7 +633,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 +682,7 @@ if (!ContainerExpr) ContainerExpr = Arr; - Usages.push_back(Usage(E)); + addUsage(Usage(E)); return true; } @@ -746,12 +754,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