[clang-tools-extra] r178485 - Improve loop convert's variable aliasing

[Edwin Vane]

Author: revane
Date: Mon Apr  1 13:15:06 2013
New Revision: 178485

URL: http://llvm.org/viewvc/llvm-project?rev=178485&view=rev
Log:
Improve loop convert's variable aliasing

Loop convert's variable name aliasing may cause issues if the variable is
declared as a value (copy). The converted loop will declare the variable as a
reference which may inadvertently cause modifications to the container if it
were used and modified as a temporary copy.

This is fixed by preserving the reference or value qualifiers of the aliased
variable. That is, if the variable was declared as a value the loop variable
will also be declared as a value and similarly for references.

Fixes: PR15600
Author: Jack Yang <jack.yang at intel.com>


Modified:
    clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopActions.cpp
    clang-tools-extra/trunk/test/cpp11-migrate/LoopConvert/naming-alias.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=178485&r1=178484&r2=178485&view=diff
==============================================================================
--- clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopActions.cpp (original)
+++ clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopActions.cpp Mon Apr  1 13:15:06 2013
@@ -737,10 +737,14 @@ void LoopFixer::doConversion(ASTContext
                              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 @@ void LoopFixer::doConversion(ASTContext
 
   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();

Modified: clang-tools-extra/trunk/test/cpp11-migrate/LoopConvert/naming-alias.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/cpp11-migrate/LoopConvert/naming-alias.cpp?rev=178485&r1=178484&r2=178485&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/cpp11-migrate/LoopConvert/naming-alias.cpp (original)
+++ clang-tools-extra/trunk/test/cpp11-migrate/LoopConvert/naming-alias.cpp Mon Apr  1 13:15:06 2013
@@ -57,3 +57,40 @@ void aliasing() {
   // 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;
+}