[clang-tools-extra] r246879 - Avoid repeated replacements on loop-convert check.
Angel Garcia Gomez via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 4 14:37:05 PDT 2015
Author: angelgarcia
Date: Fri Sep 4 16:37:05 2015
New Revision: 246879
URL: http://llvm.org/viewvc/llvm-project?rev=246879&view=rev
Log:
Avoid repeated replacements on loop-convert check.
Summary: The InitListExpr subtree is visited twice, this caused the check to do multiple replacements. Added a set to avoid it.
Reviewers: klimek, alexfh
Subscribers: cfe-commits, alexfh
Differential Revision: http://reviews.llvm.org/D12631
Modified:
clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.cpp
clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.h
clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-extra.cpp
Modified: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.cpp?rev=246879&r1=246878&r2=246879&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.cpp Fri Sep 4 16:37:05 2015
@@ -460,6 +460,15 @@ void ForLoopIndexUseVisitor::addComponen
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 @@ bool ForLoopIndexUseVisitor::TraverseUna
// 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 @@ bool ForLoopIndexUseVisitor::TraverseMem
// 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 @@ bool ForLoopIndexUseVisitor::TraverseCXX
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 @@ bool ForLoopIndexUseVisitor::TraverseCXX
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 @@ bool ForLoopIndexUseVisitor::TraverseCXX
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 @@ bool ForLoopIndexUseVisitor::TraverseArr
if (!ContainerExpr)
ContainerExpr = Arr;
- Usages.push_back(Usage(E));
+ addUsage(Usage(E));
return true;
}
@@ -746,12 +755,12 @@ bool ForLoopIndexUseVisitor::VisitDeclRe
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<ValueDecl>(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);
Modified: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.h?rev=246879&r1=246878&r2=246879&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.h (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.h Fri Sep 4 16:37:05 2015
@@ -277,6 +277,9 @@ public:
/// \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 @@ private:
/// A container which holds all usages of IndexVar as the index of
/// ArraySubscriptExpressions.
UsageResult Usages;
+ llvm::SmallSet<SourceLocation, 8> UsageLocations;
bool OnlyUsedAsIndex;
/// The DeclStmt for an alias to the container element.
const DeclStmt *AliasDecl;
Modified: clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-extra.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-extra.cpp?rev=246879&r1=246878&r2=246879&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-extra.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-extra.cpp Fri Sep 4 16:37:05 2015
@@ -884,3 +884,32 @@ void iterators() {
}
} // 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
More information about the cfe-commits
mailing list