Hi revane,
Check if the dereference operator for the iterator returns a const
element and use 'auto const &' in such cases.
Fixes: 15601
http://llvm-reviews.chandlerc.com/D641
Files:
cpp11-migrate/LoopConvert/LoopActions.cpp
cpp11-migrate/LoopConvert/LoopActions.h
cpp11-migrate/LoopConvert/LoopMatchers.cpp
cpp11-migrate/LoopConvert/LoopMatchers.h
test/cpp11-migrate/LoopConvert/iterator.cpp
test/cpp11-migrate/LoopConvert/naming-conflict.cpp
test/cpp11-migrate/LoopConvert/single-iterator.cpp
Index: cpp11-migrate/LoopConvert/LoopActions.cpp
===================================================================
--- cpp11-migrate/LoopConvert/LoopActions.cpp
+++ cpp11-migrate/LoopConvert/LoopActions.cpp
@@ -735,7 +735,8 @@
const UsageResult &Usages,
const DeclStmt *AliasDecl, const ForStmt *TheLoop,
bool ContainerNeedsDereference,
- bool DerefByValue) {
+ bool DerefByValue,
+ bool IteratorPointsToConst) {
std::string VarName;
bool VarNameFromAlias = Usages.size() == 1 && AliasDecl;
bool AliasVarIsRef = false;
@@ -783,8 +784,12 @@
// to 'T&&'.
if (DerefByValue)
AutoRefType = Context->getRValueReferenceType(AutoRefType);
- else
+ else {
+ if (IteratorPointsToConst)
+ AutoRefType = Context->getConstType(AutoRefType);
+
AutoRefType = Context->getLValueReferenceType(AutoRefType);
+ }
}
std::string MaybeDereference = ContainerNeedsDereference ? "*" : "";
@@ -913,6 +918,7 @@
const Expr *BoundExpr,
bool ContainerNeedsDereference,
bool DerefByValue,
+ bool IteratorPointsToConst,
const ForStmt *TheLoop,
Confidence ConfidenceLevel) {
ForLoopIndexUseVisitor Finder(Context, LoopVar, EndVar, ContainerExpr,
@@ -947,7 +953,7 @@
doConversion(Context, LoopVar, getReferencedVariable(ContainerExpr),
ContainerString, Finder.getUsages(),
Finder.getAliasDecl(), TheLoop, ContainerNeedsDereference,
- DerefByValue);
+ DerefByValue, IteratorPointsToConst);
++*AcceptedChanges;
}
@@ -986,12 +992,38 @@
const Expr *ContainerExpr = NULL;
bool ContainerNeedsDereference = false;
+ bool DerefByValue = false;
+ bool IteratorPointsToConst = 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);
+
+ DerefByValue = Nodes.getNodeAs<QualType>(DerefByValueResultName) != 0;
+ if (!DerefByValue) {
+ // If the iterator dereference operator returns by reference, check if it
+ // returns a const reference or not.
+ const QualType *IteratorType =
+ Nodes.getNodeAs<QualType>(DerefByRefResultName);
+ if (IteratorType) {
+ const ReferenceType *IteratorReferenceType =
+ IteratorType->getTypePtr()->getAs<ReferenceType>();
+ IteratorPointsToConst =
+ IteratorReferenceType->getPointeeType().isConstQualified();
+ } else {
+ // For non-class iterator types, check if the pointee type is const or
+ // not.
+ QualType PointerType = InitVar->getType();
+ assert(PointerType.getTypePtr()->isPointerType() &&
+ "Non-class iterator type is not a pointer type.");
+ QualType CanonicalPointerType = PointerType.getCanonicalType();
+ IteratorPointsToConst = CanonicalPointerType.getTypePtr()
+ ->getPointeeType().isConstQualified();
+ }
+ }
+ }
else if (FixerKind == LFK_PseudoArray) {
if (!EndCall)
return;
@@ -1005,9 +1037,8 @@
if (!ContainerExpr && !BoundExpr)
return;
- bool DerefByValue = Nodes.getNodeAs<QualType>(DerefByValueResultName) != 0;
findAndVerifyUsages(Context, LoopVar, EndVar, ContainerExpr, BoundExpr,
- ContainerNeedsDereference, DerefByValue, TheLoop,
+ ContainerNeedsDereference, DerefByValue, IteratorPointsToConst, TheLoop,
ConfidenceLevel);
}
Index: cpp11-migrate/LoopConvert/LoopActions.h
===================================================================
--- cpp11-migrate/LoopConvert/LoopActions.h
+++ cpp11-migrate/LoopConvert/LoopActions.h
@@ -75,7 +75,8 @@
const clang::DeclStmt *AliasDecl,
const clang::ForStmt *TheLoop,
bool ContainerNeedsDereference,
- bool DerefByValue);
+ bool DerefByValue,
+ bool IteratorPointsToConst);
/// \brief Given a loop header that would be convertible, discover all usages
/// of the index variable and convert the loop if possible.
@@ -86,6 +87,7 @@
const clang::Expr *BoundExpr,
bool ContainerNeedsDereference,
bool DerefByValue,
+ bool IteratorPointsToConst,
const clang::ForStmt *TheLoop,
Confidence ConfidenceLevel);
Index: cpp11-migrate/LoopConvert/LoopMatchers.cpp
===================================================================
--- cpp11-migrate/LoopConvert/LoopMatchers.cpp
+++ cpp11-migrate/LoopConvert/LoopMatchers.cpp
@@ -26,6 +26,7 @@
const char ConditionEndVarName[] = "conditionEndVar";
const char EndVarName[] = "endVar";
const char DerefByValueResultName[] = "derefByValueResult";
+const char DerefByRefResultName[] = "derefByRefResult";
// shared matchers
static const TypeMatcher AnyType = anything();
@@ -157,7 +158,8 @@
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)
)
)
)
Index: cpp11-migrate/LoopConvert/LoopMatchers.h
===================================================================
--- cpp11-migrate/LoopConvert/LoopMatchers.h
+++ cpp11-migrate/LoopConvert/LoopMatchers.h
@@ -31,6 +31,7 @@
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();
Index: test/cpp11-migrate/LoopConvert/iterator.cpp
===================================================================
--- test/cpp11-migrate/LoopConvert/iterator.cpp
+++ test/cpp11-migrate/LoopConvert/iterator.cpp
@@ -26,20 +26,20 @@
for (S::const_iterator it = s.begin(), e = s.end(); it != e; ++it) {
printf("s has value %d\n", (*it).x);
}
- // CHECK: for (auto & elem : s)
+ // 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 & p : *ps)
+ // CHECK: for (auto const & 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) {
printf("s has value %d\n", it->x);
}
- // CHECK: for (auto & elem : s)
+ // CHECK: for (auto const & elem : s)
// CHECK-NEXT: printf("s has value %d\n", elem.x);
for (S::iterator it = s.begin(), e = s.end(); it != e; ++it) {
@@ -84,14 +84,14 @@
it != e; ++it) {
printf("Fibonacci number is %d\n", *it);
}
- // CHECK: for (auto & elem : v)
+ // CHECK: for (auto const & elem : v)
// CHECK-NEXT: printf("Fibonacci number is %d\n", elem);
for (dependent<int>::const_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 const & elem : v)
// CHECK-NEXT: printf("Fibonacci number is %d\n", elem);
doublyDependent<int,int> intmap;
@@ -174,14 +174,14 @@
void doLoop() const {
for (const_iterator I = begin(), E = end(); I != E; ++I) {
- // CHECK: for (auto & elem : *this) {
+ // CHECK: for (auto const & elem : *this) {
}
for (const_iterator I = C::begin(), E = C::end(); I != E; ++I) {
- // CHECK: for (auto & elem : *this) {
+ // CHECK: for (auto const & elem : *this) {
}
for (const_iterator I = begin(), E = end(); I != E; ++I) {
// CHECK: for (const_iterator I = begin(), E = end(); I != E; ++I) {
- // RISKY: for (auto & elem : *this) {
+ // RISKY: for (auto const & elem : *this) {
doSomething();
}
}
Index: test/cpp11-migrate/LoopConvert/naming-conflict.cpp
===================================================================
--- test/cpp11-migrate/LoopConvert/naming-conflict.cpp
+++ test/cpp11-migrate/LoopConvert/naming-conflict.cpp
@@ -36,7 +36,7 @@
printf("s has value %d\n", (*it).x);
printf("Max of 3 and 5: %d\n", MAX(3,5));
}
- // CHECK: for (auto & MAXs_it : MAXs)
+ // 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));
Index: test/cpp11-migrate/LoopConvert/single-iterator.cpp
===================================================================
--- test/cpp11-migrate/LoopConvert/single-iterator.cpp
+++ test/cpp11-migrate/LoopConvert/single-iterator.cpp
@@ -37,20 +37,20 @@
for (S::const_iterator it = s.begin(); it != s.end(); ++it) {
printf("s has value %d\n", (*it).x);
}
- // CHECK: for (auto & elem : s)
+ // 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 & p : *ps)
+ // CHECK: for (auto const & p : *ps)
// CHECK-NEXT: printf("s has value %d\n", (p).x);
for (S::const_iterator it = s.begin(); it != s.end(); ++it) {
printf("s has value %d\n", it->x);
}
- // CHECK: for (auto & elem : s)
+ // CHECK: for (auto const & elem : s)
// CHECK-NEXT: printf("s has value %d\n", elem.x);
for (S::iterator it = s.begin(); it != s.end(); ++it) {
@@ -95,14 +95,14 @@
it != v.end(); ++it) {
printf("Fibonacci number is %d\n", *it);
}
- // CHECK: for (auto & elem : v)
+ // CHECK: for (auto const & elem : v)
// CHECK-NEXT: printf("Fibonacci number is %d\n", elem);
for (dependent<int>::const_iterator it(v.begin());
it != v.end(); ++it) {
printf("Fibonacci number is %d\n", *it);
}
- // CHECK: for (auto & elem : v)
+ // CHECK: for (auto const & elem : v)
// CHECK-NEXT: printf("Fibonacci number is %d\n", elem);
doublyDependent<int,int> intmap;
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
