diff options
10 files changed, 212 insertions, 28 deletions
diff --git a/clang-tools-extra/cpp11-migrate/AddOverride/AddOverrideActions.cpp b/clang-tools-extra/cpp11-migrate/AddOverride/AddOverrideActions.cpp index a40f673132c..3d1f1ff8c13 100644 --- a/clang-tools-extra/cpp11-migrate/AddOverride/AddOverrideActions.cpp +++ b/clang-tools-extra/cpp11-migrate/AddOverride/AddOverrideActions.cpp @@ -32,6 +32,10 @@ void AddOverrideFixer::run(const MatchFinder::MatchResult &Result) { const CXXMethodDecl *M = Result.Nodes.getDeclAs<CXXMethodDecl>(MethodId); assert(M && "Bad Callback. No node provided"); + // Check that the method declaration in the main file + if (!SM.isFromMainFile(M->getLocStart())) + return; + // First check that there isn't already an override attribute. if (!M->hasAttr<OverrideAttr>()) { if (M->getLocStart().isFileID()) { diff --git a/clang-tools-extra/cpp11-migrate/LoopConvert/LoopActions.cpp b/clang-tools-extra/cpp11-migrate/LoopConvert/LoopActions.cpp index 4ba22b3f284..88c219aefec 100644 --- a/clang-tools-extra/cpp11-migrate/LoopConvert/LoopActions.cpp +++ b/clang-tools-extra/cpp11-migrate/LoopConvert/LoopActions.cpp @@ -791,7 +791,8 @@ void LoopFixer::doConversion(ASTContext *Context, bool AliasFromForInit, const ForStmt *TheLoop, bool ContainerNeedsDereference, - bool DerefByValue) { + bool DerefByValue, + bool DerefByConstRef) { std::string VarName; bool VarNameFromAlias = Usages.size() == 1 && AliasDecl; bool AliasVarIsRef = false; @@ -849,8 +850,11 @@ void LoopFixer::doConversion(ASTContext *Context, // to 'T&&'. if (DerefByValue) AutoRefType = Context->getRValueReferenceType(AutoRefType); - else + else { + if (DerefByConstRef) + AutoRefType = Context->getConstType(AutoRefType); AutoRefType = Context->getLValueReferenceType(AutoRefType); + } } std::string MaybeDereference = ContainerNeedsDereference ? "*" : ""; @@ -979,6 +983,7 @@ void LoopFixer::findAndVerifyUsages(ASTContext *Context, const Expr *BoundExpr, bool ContainerNeedsDereference, bool DerefByValue, + bool DerefByConstRef, const ForStmt *TheLoop, Confidence ConfidenceLevel) { ForLoopIndexUseVisitor Finder(Context, LoopVar, EndVar, ContainerExpr, @@ -1013,7 +1018,7 @@ void LoopFixer::findAndVerifyUsages(ASTContext *Context, doConversion(Context, LoopVar, getReferencedVariable(ContainerExpr), ContainerString, Finder.getUsages(), Finder.getAliasDecl(), Finder.aliasUseRequired(), Finder.aliasFromForInit(), TheLoop, - ContainerNeedsDereference, DerefByValue); + ContainerNeedsDereference, DerefByValue, DerefByConstRef); ++*AcceptedChanges; } @@ -1051,14 +1056,67 @@ void LoopFixer::run(const MatchFinder::MatchResult &Result) { ConfidenceLevel.lowerTo(RL_Reasonable); const Expr *ContainerExpr = NULL; + bool DerefByValue = false; + bool DerefByConstRef = false; bool ContainerNeedsDereference = false; // FIXME: Try to put most of this logic inside a matcher. Currently, matchers // don't allow the right-recursive checks in digThroughConstructors. - if (FixerKind == LFK_Iterator) + if (FixerKind == LFK_Iterator) { ContainerExpr = findContainer(Context, LoopVar->getInit(), EndVar ? EndVar->getInit() : EndCall, &ContainerNeedsDereference); - else if (FixerKind == LFK_PseudoArray) { + + QualType InitVarType = InitVar->getType(); + QualType CanonicalInitVarType = InitVarType.getCanonicalType(); + + const CXXMemberCallExpr *BeginCall = + Nodes.getNodeAs<CXXMemberCallExpr>(BeginCallName); + assert(BeginCall != 0 && "Bad Callback. No begin call expression."); + QualType CanonicalBeginType = + BeginCall->getMethodDecl()->getResultType().getCanonicalType(); + + if (CanonicalBeginType->isPointerType() && + CanonicalInitVarType->isPointerType()) { + QualType BeginPointeeType = CanonicalBeginType->getPointeeType(); + QualType InitPointeeType = CanonicalInitVarType->getPointeeType(); + // If the initializer and the variable are both pointers check if the + // un-qualified pointee types match otherwise we don't use auto. + if (!Context->hasSameUnqualifiedType(InitPointeeType, BeginPointeeType)) + return; + } else { + // Check for qualified types to avoid conversions from non-const to const + // iterator types. + if (!Context->hasSameType(CanonicalInitVarType, CanonicalBeginType)) + return; + } + + DerefByValue = Nodes.getNodeAs<QualType>(DerefByValueResultName) != 0; + if (!DerefByValue) { + if (const QualType *DerefType = + Nodes.getNodeAs<QualType>(DerefByRefResultName)) { + // A node will only be bound with DerefByRefResultName if we're dealing + // with a user-defined iterator type. Test the const qualification of + // the reference type. + DerefByConstRef = (*DerefType)->getAs<ReferenceType>()->getPointeeType() + .isConstQualified(); + } else { + // By nature of the matcher this case is triggered only for built-in + // iterator types (i.e. pointers). + assert(isa<PointerType>(CanonicalInitVarType) && + "Non-class iterator type is not a pointer type"); + QualType InitPointeeType = CanonicalInitVarType->getPointeeType(); + QualType BeginPointeeType = CanonicalBeginType->getPointeeType(); + // If the initializer and variable have both the same type just use auto + // otherwise we test for const qualification of the pointed-at type. + if (!Context->hasSameType(InitPointeeType, BeginPointeeType)) + DerefByConstRef = InitPointeeType.isConstQualified(); + } + } else { + // If the de-referece operator return by value then test for the canonical + // const qualification of the init variable type. + DerefByConstRef = CanonicalInitVarType.isConstQualified(); + } + } else if (FixerKind == LFK_PseudoArray) { if (!EndCall) return; ContainerExpr = EndCall->getImplicitObjectArgument(); @@ -1071,9 +1129,7 @@ void LoopFixer::run(const MatchFinder::MatchResult &Result) { if (!ContainerExpr && !BoundExpr) return; - bool DerefByValue = Nodes.getNodeAs<QualType>(DerefByValueResultName) != 0; - findAndVerifyUsages(Context, LoopVar, EndVar, ContainerExpr, BoundExpr, - ContainerNeedsDereference, DerefByValue, TheLoop, - ConfidenceLevel); + ContainerNeedsDereference, DerefByValue, DerefByConstRef, + TheLoop, ConfidenceLevel); } diff --git a/clang-tools-extra/cpp11-migrate/LoopConvert/LoopActions.h b/clang-tools-extra/cpp11-migrate/LoopConvert/LoopActions.h index 6a488894ad3..eebee338208 100644 --- a/clang-tools-extra/cpp11-migrate/LoopConvert/LoopActions.h +++ b/clang-tools-extra/cpp11-migrate/LoopConvert/LoopActions.h @@ -77,7 +77,8 @@ class LoopFixer : public clang::ast_matchers::MatchFinder::MatchCallback { bool AliasFromForInit, const clang::ForStmt *TheLoop, bool ContainerNeedsDereference, - bool DerefByValue); + bool DerefByValue, + bool DerefByConstRef); /// \brief Given a loop header that would be convertible, discover all usages /// of the index variable and convert the loop if possible. @@ -88,6 +89,7 @@ class LoopFixer : public clang::ast_matchers::MatchFinder::MatchCallback { const clang::Expr *BoundExpr, bool ContainerNeedsDereference, bool DerefByValue, + bool DerefByConstRef, const clang::ForStmt *TheLoop, Confidence ConfidenceLevel); diff --git a/clang-tools-extra/cpp11-migrate/LoopConvert/LoopMatchers.cpp b/clang-tools-extra/cpp11-migrate/LoopConvert/LoopMatchers.cpp index ffd9f794b0c..a7ca15f3eb3 100644 --- a/clang-tools-extra/cpp11-migrate/LoopConvert/LoopMatchers.cpp +++ b/clang-tools-extra/cpp11-migrate/LoopConvert/LoopMatchers.cpp @@ -22,10 +22,12 @@ const char ConditionBoundName[] = "conditionBound"; const char ConditionVarName[] = "conditionVar"; const char IncrementVarName[] = "incrementVar"; const char InitVarName[] = "initVar"; +const char BeginCallName[] = "beginCall"; const char EndCallName[] = "endCall"; const char ConditionEndVarName[] = "conditionEndVar"; const char EndVarName[] = "endVar"; const char DerefByValueResultName[] = "derefByValueResult"; +const char DerefByRefResultName[] = "derefByRefResult"; // shared matchers static const TypeMatcher AnyType = anything(); @@ -109,10 +111,23 @@ StatementMatcher makeArrayLoopMatcher() { /// - If the end iterator variable 'g' is defined, it is the same as 'f' StatementMatcher makeIteratorLoopMatcher() { StatementMatcher BeginCallMatcher = - memberCallExpr(argumentCountIs(0), callee(methodDecl(hasName("begin")))); + memberCallExpr( + argumentCountIs(0), + callee( + methodDecl(hasName("begin")) + ) + ).bind(BeginCallName); DeclarationMatcher InitDeclMatcher = - varDecl(hasInitializer(anything())).bind(InitVarName); + varDecl( + hasInitializer( + anyOf( + ignoringParenImpCasts(BeginCallMatcher), + materializeTemporaryExpr(ignoringParenImpCasts(BeginCallMatcher)), + hasDescendant(BeginCallMatcher) + ) + ) + ).bind(InitVarName); DeclarationMatcher EndDeclMatcher = varDecl(hasInitializer(anything())).bind(EndVarName); @@ -157,7 +172,11 @@ StatementMatcher makeIteratorLoopMatcher() { returns( // Skip loops where the iterator's operator* returns an // rvalue reference. This is just weird. - qualType(unless(hasCanonicalType(rValueReferenceType()))) + qualType( + unless( + hasCanonicalType(rValueReferenceType()) + ) + ).bind(DerefByRefResultName) ) ) ) @@ -165,6 +184,7 @@ StatementMatcher makeIteratorLoopMatcher() { ) ); + return forStmt( hasLoopInit(anyOf( diff --git a/clang-tools-extra/cpp11-migrate/LoopConvert/LoopMatchers.h b/clang-tools-extra/cpp11-migrate/LoopConvert/LoopMatchers.h index 7824a82ad48..6946f49504b 100644 --- a/clang-tools-extra/cpp11-migrate/LoopConvert/LoopMatchers.h +++ b/clang-tools-extra/cpp11-migrate/LoopConvert/LoopMatchers.h @@ -27,10 +27,12 @@ extern const char ConditionVarName[]; extern const char ConditionEndVarName[]; extern const char IncrementVarName[]; extern const char InitVarName[]; +extern const char BeginCallName[]; extern const char EndExprName[]; extern const char EndCallName[]; extern const char EndVarName[]; extern const char DerefByValueResultName[]; +extern const char DerefByRefResultName[]; clang::ast_matchers::StatementMatcher makeArrayLoopMatcher(); clang::ast_matchers::StatementMatcher makeIteratorLoopMatcher(); diff --git a/clang-tools-extra/test/cpp11-migrate/LoopConvert/Inputs/structures.h b/clang-tools-extra/test/cpp11-migrate/LoopConvert/Inputs/structures.h index e37f2b54794..9dd04f42b5a 100644 --- a/clang-tools-extra/test/cpp11-migrate/LoopConvert/Inputs/structures.h +++ b/clang-tools-extra/test/cpp11-migrate/LoopConvert/Inputs/structures.h @@ -131,10 +131,13 @@ class transparent { template<typename IteratorType> struct Nested { typedef IteratorType* iterator; + typedef const IteratorType* const_iterator; IteratorType *operator->(); IteratorType operator*(); iterator begin(); iterator end(); + const_iterator begin() const; + const_iterator end() const; }; // Like llvm::SmallPtrSet, the iterator has a dereference operator that returns diff --git a/clang-tools-extra/test/cpp11-migrate/LoopConvert/iterator.cpp b/clang-tools-extra/test/cpp11-migrate/LoopConvert/iterator.cpp index 53030b50f03..3d98c0bca24 100644 --- a/clang-tools-extra/test/cpp11-migrate/LoopConvert/iterator.cpp +++ b/clang-tools-extra/test/cpp11-migrate/LoopConvert/iterator.cpp @@ -23,20 +23,20 @@ void f() { // CHECK-NEXT: printf("I found %d\n", elem); S s; - for (S::const_iterator it = s.begin(), e = s.end(); it != e; ++it) { + for (S::iterator it = s.begin(), e = s.end(); it != e; ++it) { printf("s has value %d\n", (*it).x); } // CHECK: for (auto & elem : s) // CHECK-NEXT: printf("s has value %d\n", (elem).x); S *ps; - for (S::const_iterator it = ps->begin(), e = ps->end(); it != e; ++it) { + for (S::iterator it = ps->begin(), e = ps->end(); it != e; ++it) { printf("s has value %d\n", (*it).x); } // CHECK: for (auto & p : *ps) // CHECK-NEXT: printf("s has value %d\n", (p).x); - for (S::const_iterator it = s.begin(), e = s.end(); it != e; ++it) { + for (S::iterator it = s.begin(), e = s.end(); it != e; ++it) { printf("s has value %d\n", it->x); } // CHECK: for (auto & elem : s) @@ -80,18 +80,18 @@ void f() { // CHECK-NEXT: int k = A->x + elem.x; dependent<int> v; - for (dependent<int>::const_iterator it = v.begin(), e = v.end(); + for (dependent<int>::iterator it = v.begin(), e = v.end(); it != e; ++it) { printf("Fibonacci number is %d\n", *it); } - // CHECK: for (auto & elem : v) + // CHECK: for (auto & elem : v) { // CHECK-NEXT: printf("Fibonacci number is %d\n", elem); - for (dependent<int>::const_iterator it(v.begin()), e = v.end(); + for (dependent<int>::iterator it(v.begin()), e = v.end(); it != e; ++it) { printf("Fibonacci number is %d\n", *it); } - // CHECK: for (auto & elem : v) + // CHECK: for (auto & elem : v) { // CHECK-NEXT: printf("Fibonacci number is %d\n", elem); doublyDependent<int,int> intmap; @@ -134,6 +134,46 @@ void f() { } } +// Tests to verify the proper use of auto where the init variable type and the +// initializer type differ or are mostly the same except for const qualifiers. +void different_type() { + // s.begin() returns a type 'iterator' which is just a non-const pointer and + // differs from const_iterator only on the const qualification. + S s; + for (S::const_iterator it = s.begin(), e = s.end(); it != e; ++it) { + printf("s has value %d\n", (*it).x); + } + // CHECK: for (auto const & elem : s) + // CHECK-NEXT: printf("s has value %d\n", (elem).x); + + S *ps; + for (S::const_iterator it = ps->begin(), e = ps->end(); it != e; ++it) { + printf("s has value %d\n", (*it).x); + } + // CHECK: for (auto const & p : *ps) + // CHECK-NEXT: printf("s has value %d\n", (p).x); + + // v.begin() returns a user-defined type 'iterator' which, since it's + // different from const_iterator, disqualifies these loops from + // transformation. + dependent<int> v; + for (dependent<int>::const_iterator it = v.begin(), e = v.end(); + it != e; ++it) { + printf("Fibonacci number is %d\n", *it); + } + // CHECK: for (dependent<int>::const_iterator it = v.begin(), e = v.end(); + // CHECK-NEXT: it != e; ++it) { + // CHECK-NEXT: printf("Fibonacci number is %d\n", *it); + + for (dependent<int>::const_iterator it(v.begin()), e = v.end(); + it != e; ++it) { + printf("Fibonacci number is %d\n", *it); + } + // CHECK: for (dependent<int>::const_iterator it(v.begin()), e = v.end(); + // CHECK-NEXT: it != e; ++it) { + // CHECK-NEXT: printf("Fibonacci number is %d\n", *it); +} + // Tests to ensure that an implicit 'this' is picked up as the container. // If member calls are made to 'this' within the loop, the transform becomes // risky as these calls may affect state that affects the loop. diff --git a/clang-tools-extra/test/cpp11-migrate/LoopConvert/naming-conflict.cpp b/clang-tools-extra/test/cpp11-migrate/LoopConvert/naming-conflict.cpp index 050f44ef900..9597c63a6f2 100644 --- a/clang-tools-extra/test/cpp11-migrate/LoopConvert/naming-conflict.cpp +++ b/clang-tools-extra/test/cpp11-migrate/LoopConvert/naming-conflict.cpp @@ -32,7 +32,7 @@ void sameNames() { void macroConflict() { S MAXs; - for (S::const_iterator it = MAXs.begin(), e = MAXs.end(); it != e; ++it) { + for (S::iterator it = MAXs.begin(), e = MAXs.end(); it != e; ++it) { printf("s has value %d\n", (*it).x); printf("Max of 3 and 5: %d\n", MAX(3,5)); } @@ -40,6 +40,14 @@ void macroConflict() { // CHECK-NEXT: printf("s has value %d\n", (MAXs_it).x); // CHECK-NEXT: printf("Max of 3 and 5: %d\n", MAX(3,5)); + for (S::const_iterator it = MAXs.begin(), e = MAXs.end(); it != e; ++it) { + printf("s has value %d\n", (*it).x); + printf("Max of 3 and 5: %d\n", MAX(3,5)); + } + // CHECK: for (auto const & MAXs_it : MAXs) + // CHECK-NEXT: printf("s has value %d\n", (MAXs_it).x); + // CHECK-NEXT: printf("Max of 3 and 5: %d\n", MAX(3,5)); + T DEFs; for (T::iterator it = DEFs.begin(), e = DEFs.end(); it != e; ++it) { if (*it == DEF) { diff --git a/clang-tools-extra/test/cpp11-migrate/LoopConvert/nesting.cpp b/clang-tools-extra/test/cpp11-migrate/LoopConvert/nesting.cpp index 729b5ff21a6..b26cf218d77 100644 --- a/clang-tools-extra/test/cpp11-migrate/LoopConvert/nesting.cpp +++ b/clang-tools-extra/test/cpp11-migrate/LoopConvert/nesting.cpp @@ -54,4 +54,16 @@ void f() { // CHECK: for (auto & elem : NestT) { // CHECK-NEXT: for (T::iterator TI = (elem).begin(), TE = (elem).end(); TI != TE; ++TI) { // CHECK-NEXT: printf("%d", *TI); + + Nested<S> NestS; + for (Nested<S>::const_iterator I = NestS.begin(), E = NestS.end(); I != E; ++I) { + for (S::const_iterator SI = (*I).begin(), SE = (*I).end(); SI != SE; ++SI) { + printf("%d", *SI); + } + } + // The inner loop is also convertible, but doesn't need to be converted + // immediately. Update this test when that changes! + // CHECK: for (auto const & elem : NestS) { + // CHECK-NEXT: for (S::const_iterator SI = (elem).begin(), SE = (elem).end(); SI != SE; ++SI) { + // CHECK-NEXT: printf("%d", *SI); } diff --git a/clang-tools-extra/test/cpp11-migrate/LoopConvert/single-iterator.cpp b/clang-tools-extra/test/cpp11-migrate/LoopConvert/single-iterator.cpp index 91b6ea5b2f7..c717ed6c5e2 100644 --- a/clang-tools-extra/test/cpp11-migrate/LoopConvert/single-iterator.cpp +++ b/clang-tools-extra/test/cpp11-migrate/LoopConvert/single-iterator.cpp @@ -34,20 +34,20 @@ void f() { // CHECK-NEXT: printf("I found %d\n", elem); S s; - for (S::const_iterator it = s.begin(); it != s.end(); ++it) { + for (S::iterator it = s.begin(); it != s.end(); ++it) { printf("s has value %d\n", (*it).x); } // CHECK: for (auto & elem : s) // CHECK-NEXT: printf("s has value %d\n", (elem).x); S *ps; - for (S::const_iterator it = ps->begin(); it != ps->end(); ++it) { + for (S::iterator it = ps->begin(); it != ps->end(); ++it) { printf("s has value %d\n", (*it).x); } // CHECK: for (auto & p : *ps) // CHECK-NEXT: printf("s has value %d\n", (p).x); - for (S::const_iterator it = s.begin(); it != s.end(); ++it) { + for (S::iterator it = s.begin(); it != s.end(); ++it) { printf("s has value %d\n", it->x); } // CHECK: for (auto & elem : s) @@ -91,18 +91,18 @@ void f() { // CHECK-NEXT: int k = A->x + elem.x; dependent<int> v; - for (dependent<int>::const_iterator it = v.begin(); + for (dependent<int>::iterator it = v.begin(); it != v.end(); ++it) { printf("Fibonacci number is %d\n", *it); } - // CHECK: for (auto & elem : v) + // CHECK: for (auto & elem : v) { // CHECK-NEXT: printf("Fibonacci number is %d\n", elem); - for (dependent<int>::const_iterator it(v.begin()); + for (dependent<int>::iterator it(v.begin()); it != v.end(); ++it) { printf("Fibonacci number is %d\n", *it); } - // CHECK: for (auto & elem : v) + // CHECK: for (auto & elem : v) { // CHECK-NEXT: printf("Fibonacci number is %d\n", elem); doublyDependent<int,int> intmap; @@ -113,3 +113,40 @@ void f() { // CHECK: for (auto & elem : intmap) // CHECK-NEXT: printf("intmap[%d] = %d", elem.first, elem.second); } + +void different_type() { + // Tests to verify the proper use of auto where the init variable type and the + // initializer type differ or are mostly the same except for const qualifiers. + + // s.begin() returns a type 'iterator' which is just a non-const pointer and + // differs from const_iterator only on the const qualification. + S s; + for (S::const_iterator it = s.begin(); it != s.end(); ++it) { + printf("s has value %d\n", (*it).x); + } + // CHECK: for (auto const & elem : s) + // CHECK-NEXT: printf("s has value %d\n", (elem).x); + + S *ps; + for (S::const_iterator it = ps->begin(); it != ps->end(); ++it) { + printf("s has value %d\n", (*it).x); + } + // CHECK: for (auto const & p : *ps) + // CHECK-NEXT: printf("s has value %d\n", (p).x); + + // v.begin() returns a user-defined type 'iterator' which, since it's + // different from const_iterator, disqualifies these loops from + // transformation. + dependent<int> v; + for (dependent<int>::const_iterator it = v.begin(); it != v.end(); ++it) { + printf("Fibonacci number is %d\n", *it); + } + // CHECK: for (dependent<int>::const_iterator it = v.begin(); it != v.end(); ++it) { + // CHECK-NEXT: printf("Fibonacci number is %d\n", *it); + + for (dependent<int>::const_iterator it(v.begin()); it != v.end(); ++it) { + printf("Fibonacci number is %d\n", *it); + } + // CHECK: for (dependent<int>::const_iterator it(v.begin()); it != v.end(); ++it) { + // CHECK-NEXT: printf("Fibonacci number is %d\n", *it); +} |
