- Simplified if statement as per Edwin's comment
And yes, if a loop uses an "alias" variable as well as the iterator/array it
won't be marked as an aliased variable.
In the statement:
bool VarNameFromAlias = Usages.size() == 1 && AliasDecl;
Usages.size() returns how many times the iterator or array was called.
AliasDecl is populated by isAliasDecl() which finds the aliased variable
declaration inside the loop body.
If both statements are true, then we know that the iterator/array was only
used once in the aliased variable declaration.
Hi revane, gribozavr,
http://llvm-reviews.chandlerc.com/D588
CHANGE SINCE LAST DIFF
http://llvm-reviews.chandlerc.com/D588?vs=1432&id=1459#toc
Files:
cpp11-migrate/LoopConvert/LoopActions.cpp
test/cpp11-migrate/LoopConvert/naming-alias.cpp
Index: cpp11-migrate/LoopConvert/LoopActions.cpp
===================================================================
--- cpp11-migrate/LoopConvert/LoopActions.cpp
+++ cpp11-migrate/LoopConvert/LoopActions.cpp
@@ -737,10 +737,14 @@
bool ContainerNeedsDereference,
bool DerefByValue) {
std::string VarName;
+ bool VarNameFromAlias = Usages.size() == 1 && AliasDecl;
+ bool AliasVarIsRef = false;
- if (Usages.size() == 1 && AliasDecl) {
+ if (VarNameFromAlias) {
const VarDecl *AliasVar = cast<VarDecl>(AliasDecl->getSingleDecl());
VarName = AliasVar->getName().str();
+ AliasVarIsRef = AliasVar->getType()->isReferenceType();
+
// We keep along the entire DeclStmt to keep the correct range here.
const SourceRange &ReplaceRange = AliasDecl->getSourceRange();
Replace->insert(
@@ -770,13 +774,18 @@
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);
+ // If the new variable name is from the aliased variable, then the reference
+ // type for the new variable should only be used if the aliased variable was
+ // declared as a reference.
+ if (!VarNameFromAlias || AliasVarIsRef) {
+ // 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();
Index: test/cpp11-migrate/LoopConvert/naming-alias.cpp
===================================================================
--- test/cpp11-migrate/LoopConvert/naming-alias.cpp
+++ test/cpp11-migrate/LoopConvert/naming-alias.cpp
@@ -57,3 +57,40 @@
// CHECK-NEXT: Val &t = func(elem);
// CHECK-NEXT: int y = t.x;
}
+
+void refs_and_vals() {
+ // The following tests check that the transform correctly preserves the
+ // reference or value qualifiers of the aliased variable. That is, if the
+ // variable was declared as a value, the loop variable will be declared as a
+ // value and vice versa for references.
+
+ S s;
+ const S s_const = s;
+
+ for (S::const_iterator it = s_const.begin(); it != s_const.end(); ++it) {
+ MutableVal alias = *it; { }
+ alias.x = 0;
+ }
+ // CHECK: for (auto alias : s_const)
+ // CHECK-NOT: MutableVal {{[a-z_]+}} =
+ // CHECK-NEXT: { }
+ // CHECK-NEXT: alias.x = 0;
+
+ for (S::iterator it = s.begin(), e = s.end(); it != e; ++it) {
+ MutableVal alias = *it; { }
+ alias.x = 0;
+ }
+ // CHECK: for (auto alias : s)
+ // CHECK-NOT: MutableVal {{[a-z_]+}} =
+ // CHECK-NEXT: { }
+ // CHECK-NEXT: alias.x = 0;
+
+ for (S::iterator it = s.begin(), e = s.end(); it != e; ++it) {
+ MutableVal &alias = *it; { }
+ alias.x = 0;
+ }
+ // CHECK: for (auto & alias : s)
+ // CHECK-NOT: MutableVal &{{[a-z_]+}} =
+ // CHECK-NEXT: { }
+ // CHECK-NEXT: alias.x = 0;
+}
Index: cpp11-migrate/LoopConvert/LoopActions.cpp
===================================================================
--- cpp11-migrate/LoopConvert/LoopActions.cpp
+++ cpp11-migrate/LoopConvert/LoopActions.cpp
@@ -737,10 +737,14 @@
bool ContainerNeedsDereference,
bool DerefByValue) {
std::string VarName;
+ bool VarNameFromAlias = Usages.size() == 1 && AliasDecl;
+ bool AliasVarIsRef = false;
- if (Usages.size() == 1 && AliasDecl) {
+ if (VarNameFromAlias) {
const VarDecl *AliasVar = cast<VarDecl>(AliasDecl->getSingleDecl());
VarName = AliasVar->getName().str();
+ AliasVarIsRef = AliasVar->getType()->isReferenceType();
+
// We keep along the entire DeclStmt to keep the correct range here.
const SourceRange &ReplaceRange = AliasDecl->getSourceRange();
Replace->insert(
@@ -770,13 +774,18 @@
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);
+ // If the new variable name is from the aliased variable, then the reference
+ // type for the new variable should only be used if the aliased variable was
+ // declared as a reference.
+ if (!VarNameFromAlias || AliasVarIsRef) {
+ // 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();
Index: test/cpp11-migrate/LoopConvert/naming-alias.cpp
===================================================================
--- test/cpp11-migrate/LoopConvert/naming-alias.cpp
+++ test/cpp11-migrate/LoopConvert/naming-alias.cpp
@@ -57,3 +57,40 @@
// CHECK-NEXT: Val &t = func(elem);
// CHECK-NEXT: int y = t.x;
}
+
+void refs_and_vals() {
+ // The following tests check that the transform correctly preserves the
+ // reference or value qualifiers of the aliased variable. That is, if the
+ // variable was declared as a value, the loop variable will be declared as a
+ // value and vice versa for references.
+
+ S s;
+ const S s_const = s;
+
+ for (S::const_iterator it = s_const.begin(); it != s_const.end(); ++it) {
+ MutableVal alias = *it; { }
+ alias.x = 0;
+ }
+ // CHECK: for (auto alias : s_const)
+ // CHECK-NOT: MutableVal {{[a-z_]+}} =
+ // CHECK-NEXT: { }
+ // CHECK-NEXT: alias.x = 0;
+
+ for (S::iterator it = s.begin(), e = s.end(); it != e; ++it) {
+ MutableVal alias = *it; { }
+ alias.x = 0;
+ }
+ // CHECK: for (auto alias : s)
+ // CHECK-NOT: MutableVal {{[a-z_]+}} =
+ // CHECK-NEXT: { }
+ // CHECK-NEXT: alias.x = 0;
+
+ for (S::iterator it = s.begin(), e = s.end(); it != e; ++it) {
+ MutableVal &alias = *it; { }
+ alias.x = 0;
+ }
+ // CHECK: for (auto & alias : s)
+ // CHECK-NOT: MutableVal &{{[a-z_]+}} =
+ // CHECK-NEXT: { }
+ // CHECK-NEXT: alias.x = 0;
+}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
