commit b457b68f8d2d5769127a15aacc6f6071ea4ebf57
author Manuel Klimek <klimek@google.com> Wed Sep 23 22:28:14 2015 +0000
committer Manuel Klimek <klimek@google.com> Wed Sep 23 22:28:14 2015 +0000
tree 235ac7b0d35d2285e4727719ee5613e67a0545c5
parent beb5bccf88876eb3087674429ca71c4a6bee6b7d

Fix loop-convert for trivially copyable types.

Previously, we would rewrite:
void f(const vector<int> &v) {
  for (size_t i = 0; i < v.size(); ++i) {
to
  for (const auto &elem : v) {

Now we rewrite it to:
  for (auto elem : v) {
(and similarly for iterator based loops).

llvm-svn: 248438

clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp

diff --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
index 054e5ce..774ecbb 100644
--- a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
@@ -500,7 +500,10 @@
   // 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) {
+  bool UseCopy = (VarNameFromAlias && !AliasVarIsRef) ||
+                 (Descriptor.DerefByConstRef && Descriptor.IsTriviallyCopyable);
+
+  if (!UseCopy) {
     if (Descriptor.DerefByConstRef) {
       AutoType =
           Context->getLValueReferenceType(Context->getConstType(AutoType));
@@ -547,35 +550,29 @@
                                               RangeDescriptor &Descriptor) {
   // On arrays and pseudoarrays, we must figure out the qualifiers from the
   // usages.
-  if (usagesAreConst(Usages)) {
+  if (usagesAreConst(Usages) ||
+      containerIsConst(ContainerExpr, Descriptor.ContainerNeedsDereference)) {
     Descriptor.DerefByConstRef = true;
-  } else {
-    if (usagesReturnRValues(Usages)) {
-      // If the index usages (dereference, subscript, at, ...) return rvalues,
-      // then we should not use a reference, because we need to keep the code
-      // correct if it mutates the returned objects.
-      Descriptor.DerefByValue = true;
-      // Try to find the type of the elements on the container, to check if
-      // they are trivially copyable.
-      for (const Usage &U : Usages) {
-        if (!U.Expression || U.Expression->getType().isNull())
-          continue;
-        QualType Type = U.Expression->getType().getCanonicalType();
-        if (U.Kind == Usage::UK_MemberThroughArrow) {
-          if (!Type->isPointerType())
-            continue;
-          Type = Type->getPointeeType();
-        }
-        Descriptor.IsTriviallyCopyable = Type.isTriviallyCopyableType(*Context);
-        break;
+  }
+  if (usagesReturnRValues(Usages)) {
+    // If the index usages (dereference, subscript, at, ...) return rvalues,
+    // then we should not use a reference, because we need to keep the code
+    // correct if it mutates the returned objects.
+    Descriptor.DerefByValue = true;
+  }
+  // Try to find the type of the elements on the container, to check if
+  // they are trivially copyable.
+  for (const Usage &U : Usages) {
+    if (!U.Expression || U.Expression->getType().isNull())
+      continue;
+    QualType Type = U.Expression->getType().getCanonicalType();
+    if (U.Kind == Usage::UK_MemberThroughArrow) {
+      if (!Type->isPointerType()) {
+        continue;
       }
-    } else if (containerIsConst(ContainerExpr,
-                                Descriptor.ContainerNeedsDereference)) {
-      // The usages are lvalue references, we add a 'const' if the container
-      // is 'const'. This will always be the case with const arrays (unless
-      // usagesAreConst() returned true).
-      Descriptor.DerefByConstRef = true;
+      Type = Type->getPointeeType();
     }
+    Descriptor.IsTriviallyCopyable = Type.isTriviallyCopyableType(*Context);
   }
 }
 
@@ -604,10 +601,11 @@
       // 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.
-      Descriptor.DerefByConstRef = (*DerefType)
-                                       ->getAs<ReferenceType>()
-                                       ->getPointeeType()
-                                       .isConstQualified();
+      auto ValueType = (*DerefType)->getAs<ReferenceType>()->getPointeeType();
+
+      Descriptor.DerefByConstRef = ValueType.isConstQualified();
+      Descriptor.IsTriviallyCopyable =
+          ValueType.isTriviallyCopyableType(*Context);
     } else {
       // By nature of the matcher this case is triggered only for built-in
       // iterator types (i.e. pointers).
@@ -617,6 +615,9 @@
       // We test for const qualification of the pointed-at type.
       Descriptor.DerefByConstRef =
           CanonicalInitVarType->getPointeeType().isConstQualified();
+      Descriptor.IsTriviallyCopyable =
+          CanonicalInitVarType->getPointeeType().isTriviallyCopyableType(
+              *Context);
     }
   }
 }