[clang-tools-extra] r176631 - Have LoopConvert use 'auto &&' where necessary
Edwin Vane
edwin.vane at intel.com
Thu Mar 7 08:22:06 PST 2013
Author: revane
Date: Thu Mar 7 10:22:05 2013
New Revision: 176631
URL: http://llvm.org/viewvc/llvm-project?rev=176631&view=rev
Log:
Have LoopConvert use 'auto &&' where necessary
For iterators where the dereference operator returns by value, LoopConvert
should use 'auto &&' in the range-based for loop expression.
If the dereference operator returns an rvalue reference, this is deemed too
strange and the for loop is not converted.
Moved test case from iterator_failing.cpp to iterator.cpp and added extra
tests.
Fixes PR15437.
Reviewer: gribozavr
Removed:
clang-tools-extra/trunk/test/cpp11-migrate/LoopConvert/iterator_failing.cpp
Modified:
clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopActions.cpp
clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopActions.h
clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopMatchers.cpp
clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopMatchers.h
clang-tools-extra/trunk/test/cpp11-migrate/LoopConvert/Inputs/structures.h
clang-tools-extra/trunk/test/cpp11-migrate/LoopConvert/iterator.cpp
Modified: clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopActions.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopActions.cpp?rev=176631&r1=176630&r2=176631&view=diff
==============================================================================
--- clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopActions.cpp (original)
+++ clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopActions.cpp Thu Mar 7 10:22:05 2013
@@ -734,7 +734,8 @@ void LoopFixer::doConversion(ASTContext
StringRef ContainerString,
const UsageResult &Usages,
const DeclStmt *AliasDecl, const ForStmt *TheLoop,
- bool ContainerNeedsDereference) {
+ bool ContainerNeedsDereference,
+ bool DerefByValue) {
std::string VarName;
if (Usages.size() == 1 && AliasDecl) {
@@ -767,8 +768,15 @@ void LoopFixer::doConversion(ASTContext
// Now, we need to construct the new range expresion.
SourceRange ParenRange(TheLoop->getLParenLoc(), TheLoop->getRParenLoc());
- QualType AutoRefType =
- Context->getLValueReferenceType(Context->getAutoDeductType());
+ QualType AutoRefType = Context->getAutoDeductType();
+
+ // If an iterator's operator*() returns a 'T&' we can bind that to 'auto&'.
+ // If operator*() returns 'T' we can bind that to 'auto&&' which will deduce
+ // to 'T&&'.
+ if (DerefByValue)
+ AutoRefType = Context->getRValueReferenceType(AutoRefType);
+ else
+ AutoRefType = Context->getLValueReferenceType(AutoRefType);
std::string MaybeDereference = ContainerNeedsDereference ? "*" : "";
std::string TypeString = AutoRefType.getAsString();
@@ -895,6 +903,7 @@ void LoopFixer::findAndVerifyUsages(ASTC
const Expr *ContainerExpr,
const Expr *BoundExpr,
bool ContainerNeedsDereference,
+ bool DerefByValue,
const ForStmt *TheLoop,
Confidence ConfidenceLevel) {
ForLoopIndexUseVisitor Finder(Context, LoopVar, EndVar, ContainerExpr,
@@ -928,7 +937,8 @@ void LoopFixer::findAndVerifyUsages(ASTC
doConversion(Context, LoopVar, getReferencedVariable(ContainerExpr),
ContainerString, Finder.getUsages(),
- Finder.getAliasDecl(), TheLoop, ContainerNeedsDereference);
+ Finder.getAliasDecl(), TheLoop, ContainerNeedsDereference,
+ DerefByValue);
++*AcceptedChanges;
}
@@ -986,6 +996,9 @@ void LoopFixer::run(const MatchFinder::M
if (!ContainerExpr && !BoundExpr)
return;
+ bool DerefByValue = Nodes.getNodeAs<QualType>(DerefByValueResultName) != 0;
+
findAndVerifyUsages(Context, LoopVar, EndVar, ContainerExpr, BoundExpr,
- ContainerNeedsDereference, TheLoop, ConfidenceLevel);
+ ContainerNeedsDereference, DerefByValue, TheLoop,
+ ConfidenceLevel);
}
Modified: clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopActions.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopActions.h?rev=176631&r1=176630&r2=176631&view=diff
==============================================================================
--- clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopActions.h (original)
+++ clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopActions.h Thu Mar 7 10:22:05 2013
@@ -74,7 +74,8 @@ class LoopFixer : public clang::ast_matc
const UsageResult &Usages,
const clang::DeclStmt *AliasDecl,
const clang::ForStmt *TheLoop,
- bool ContainerNeedsDereference);
+ bool ContainerNeedsDereference,
+ bool DerefByValue);
/// \brief Given a loop header that would be convertible, discover all usages
/// of the index variable and convert the loop if possible.
@@ -84,6 +85,7 @@ class LoopFixer : public clang::ast_matc
const clang::Expr *ContainerExpr,
const clang::Expr *BoundExpr,
bool ContainerNeedsDereference,
+ bool DerefByValue,
const clang::ForStmt *TheLoop,
Confidence ConfidenceLevel);
Modified: clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopMatchers.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopMatchers.cpp?rev=176631&r1=176630&r2=176631&view=diff
==============================================================================
--- clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopMatchers.cpp (original)
+++ clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopMatchers.cpp Thu Mar 7 10:22:05 2013
@@ -25,6 +25,7 @@ const char InitVarName[] = "initVar";
const char EndCallName[] = "endCall";
const char ConditionEndVarName[] = "conditionEndVar";
const char EndVarName[] = "endVar";
+const char DerefByValueResultName[] = "derefByValueResult";
// shared matchers
static const TypeMatcher AnyType = anything();
@@ -137,30 +138,75 @@ StatementMatcher makeIteratorLoopMatcher
hasArgument(0, IteratorComparisonMatcher),
hasArgument(1, IteratorBoundMatcher));
- return forStmt(
+ // This matcher tests that a declaration is a CXXRecordDecl that has an
+ // overloaded operator*(). If the operator*() returns by value instead of by
+ // reference then the return type is tagged with DerefByValueResultName.
+ internal::Matcher<VarDecl> TestDerefReturnsByValue =
+ hasType(
+ recordDecl(
+ hasMethod(
+ allOf(
+ hasOverloadedOperatorName("*"),
+ anyOf(
+ // Tag the return type if it's by value.
+ returns(
+ qualType(
+ unless(hasCanonicalType(referenceType()))
+ ).bind(DerefByValueResultName)
+ ),
+ returns(
+ // Skip loops where the iterator's operator* returns an
+ // rvalue reference. This is just weird.
+ qualType(unless(hasCanonicalType(rValueReferenceType())))
+ )
+ )
+ )
+ )
+ )
+ );
+
+ return
+ forStmt(
hasLoopInit(anyOf(
- declStmt(declCountIs(2),
- containsDeclaration(0, InitDeclMatcher),
- containsDeclaration(1, EndDeclMatcher)),
- declStmt(hasSingleDecl(InitDeclMatcher)))),
+ declStmt(
+ declCountIs(2),
+ containsDeclaration(0, InitDeclMatcher),
+ containsDeclaration(1, EndDeclMatcher)
+ ),
+ declStmt(hasSingleDecl(InitDeclMatcher))
+ )),
hasCondition(anyOf(
- binaryOperator(hasOperatorName("!="),
- hasLHS(IteratorComparisonMatcher),
- hasRHS(IteratorBoundMatcher)),
- binaryOperator(hasOperatorName("!="),
- hasLHS(IteratorBoundMatcher),
- hasRHS(IteratorComparisonMatcher)),
- OverloadedNEQMatcher)),
+ binaryOperator(
+ hasOperatorName("!="),
+ hasLHS(IteratorComparisonMatcher),
+ hasRHS(IteratorBoundMatcher)
+ ),
+ binaryOperator(
+ hasOperatorName("!="),
+ hasLHS(IteratorBoundMatcher),
+ hasRHS(IteratorComparisonMatcher)
+ ),
+ OverloadedNEQMatcher
+ )),
hasIncrement(anyOf(
- unaryOperator(hasOperatorName("++"),
- hasUnaryOperand(declRefExpr(to(
- varDecl(hasType(pointsTo(AnyType)))
- .bind(IncrementVarName))))),
- operatorCallExpr(
- hasOverloadedOperatorName("++"),
- hasArgument(0, declRefExpr(to(
- varDecl().bind(IncrementVarName))))))))
- .bind(LoopName);
+ unaryOperator(
+ hasOperatorName("++"),
+ hasUnaryOperand(
+ declRefExpr(to(
+ varDecl(hasType(pointsTo(AnyType))).bind(IncrementVarName)
+ ))
+ )
+ ),
+ operatorCallExpr(
+ hasOverloadedOperatorName("++"),
+ hasArgument(0,
+ declRefExpr(to(
+ varDecl(TestDerefReturnsByValue).bind(IncrementVarName)
+ ))
+ )
+ )
+ ))
+ ).bind(LoopName);
}
/// \brief The matcher used for array-like containers (pseudoarrays).
Modified: clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopMatchers.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopMatchers.h?rev=176631&r1=176630&r2=176631&view=diff
==============================================================================
--- clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopMatchers.h (original)
+++ clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopMatchers.h Thu Mar 7 10:22:05 2013
@@ -30,6 +30,7 @@ extern const char InitVarName[];
extern const char EndExprName[];
extern const char EndCallName[];
extern const char EndVarName[];
+extern const char DerefByValueResultName[];
clang::ast_matchers::StatementMatcher makeArrayLoopMatcher();
clang::ast_matchers::StatementMatcher makeIteratorLoopMatcher();
Modified: clang-tools-extra/trunk/test/cpp11-migrate/LoopConvert/Inputs/structures.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/cpp11-migrate/LoopConvert/Inputs/structures.h?rev=176631&r1=176630&r2=176631&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/cpp11-migrate/LoopConvert/Inputs/structures.h (original)
+++ clang-tools-extra/trunk/test/cpp11-migrate/LoopConvert/Inputs/structures.h Thu Mar 7 10:22:05 2013
@@ -150,4 +150,27 @@ struct PtrSet {
iterator end() const;
};
+template <typename T>
+struct TypedefDerefContainer {
+ struct iterator {
+ typedef T &deref_type;
+ bool operator!=(const iterator &other) const;
+ deref_type operator*();
+ iterator &operator++();
+ };
+ iterator begin() const;
+ iterator end() const;
+};
+
+template <typename T>
+struct RValueDerefContainer {
+ struct iterator {
+ typedef T &&deref_type;
+ bool operator!=(const iterator &other) const;
+ deref_type operator*();
+ iterator &operator++();
+ };
+ iterator begin() const;
+ iterator end() const;
+};
#endif // STRUCTURES_H
Modified: clang-tools-extra/trunk/test/cpp11-migrate/LoopConvert/iterator.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/cpp11-migrate/LoopConvert/iterator.cpp?rev=176631&r1=176630&r2=176631&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/cpp11-migrate/LoopConvert/iterator.cpp (original)
+++ clang-tools-extra/trunk/test/cpp11-migrate/LoopConvert/iterator.cpp Thu Mar 7 10:22:05 2013
@@ -1,5 +1,5 @@
// RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp
-// RUN: cpp11-migrate -loop-convert %t.cpp -- -I %S/Inputs
+// RUN: cpp11-migrate -loop-convert %t.cpp -- -I %S/Inputs -std=c++11
// RUN: FileCheck -input-file=%t.cpp %s
// RUN: cpp11-migrate -loop-convert %t.cpp -risk=risky -- -I %S/Inputs
// RUN: FileCheck -check-prefix=RISKY -input-file=%t.cpp %s
@@ -101,6 +101,37 @@ void f() {
}
// CHECK: for ({{[a-zA-Z_ ]*&? ?}}[[VAR:[a-z_]+]] : intmap)
// CHECK-NEXT: printf("intmap[%d] = %d", [[VAR]].first, [[VAR]].second);
+
+ // PtrSet's iterator dereferences by value so auto & can't be used.
+ {
+ PtrSet<int*> int_ptrs;
+ for (PtrSet<int*>::iterator I = int_ptrs.begin(),
+ E = int_ptrs.end(); I != E; ++I) {
+ // CHECK: for (auto && int_ptr : int_ptrs) {
+ }
+ }
+
+ // This container uses an iterator where the derefence type is a typedef of
+ // a reference type. Make sure non-const auto & is still used. A failure here
+ // means canonical types aren't being tested.
+ {
+ TypedefDerefContainer<int> int_ptrs;
+ for (TypedefDerefContainer<int>::iterator I = int_ptrs.begin(),
+ E = int_ptrs.end(); I != E; ++I) {
+ // CHECK: for (auto & int_ptr : int_ptrs) {
+ }
+ }
+
+ {
+ // Iterators returning an rvalue reference should disqualify the loop from
+ // transformation.
+ RValueDerefContainer<int> container;
+ for (RValueDerefContainer<int>::iterator I = container.begin(),
+ E = container.end(); I != E; ++I) {
+ // CHECK: for (RValueDerefContainer<int>::iterator I = container.begin(),
+ // CHECK-NEXT: E = container.end(); I != E; ++I) {
+ }
+ }
}
// Tests to ensure that an implicit 'this' is picked up as the container.
Removed: clang-tools-extra/trunk/test/cpp11-migrate/LoopConvert/iterator_failing.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/cpp11-migrate/LoopConvert/iterator_failing.cpp?rev=176630&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/cpp11-migrate/LoopConvert/iterator_failing.cpp (original)
+++ clang-tools-extra/trunk/test/cpp11-migrate/LoopConvert/iterator_failing.cpp (removed)
@@ -1,14 +0,0 @@
-// RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp
-// RUN: cpp11-migrate -loop-convert %t.cpp -- -I %S/Inputs
-// RUN: FileCheck -input-file=%t.cpp %s
-// XFAIL: *
-#include "structures.h"
-
-void f() {
- // See PR15437 for details.
- PtrSet<int*> int_ptrs;
- for (PtrSet<int*>::iterator I = int_ptrs.begin(),
- E = int_ptrs.end(); I != E; ++I) {
- // CHECK: for (const auto & int_ptr : int_ptrs) {
- }
-}
More information about the cfe-commits
mailing list