[PATCH] Transform for loops over pseudo-arrays only if begin/end members exist

[Edwin Vane]

Hi tareqsiraj,

For loops using pseudo-arrays, classes that can be used like arrays from
the Loop Convert Transform's point of view, should only get transformed
if the pseudo-array class has begin()/end() members for the
range-based for-loop to call.

Free versions of begin()/end() should also be allowed but this is an
enhancement for another revision.

http://llvm-reviews.chandlerc.com/D775

Files:
  cpp11-migrate/LoopConvert/LoopMatchers.cpp
  test/cpp11-migrate/LoopConvert/free_begin_end_fail.cpp
  test/cpp11-migrate/LoopConvert/pseudoarray.cpp

Index: cpp11-migrate/LoopConvert/LoopMatchers.cpp
===================================================================
--- cpp11-migrate/LoopConvert/LoopMatchers.cpp
+++ cpp11-migrate/LoopConvert/LoopMatchers.cpp
@@ -238,10 +238,57 @@
 ///   - If the end iterator variable 'g' is defined, it is the same as 'j'
 ///   - The container's iterators would not be invalidated during the loop
 StatementMatcher makePseudoArrayLoopMatcher() {
+  // Test that the incoming type has a record declaration that has methods
+  // called 'begin' and 'end'. If the incoming type is const, then make sure
+  // these methods are also marked const.
+  // 
+  // FIXME: To be completely thorough this matcher should also ensure the
+  // return type of begin/end is an iterator that dereferences to the same as
+  // what operator[] or at() returns. Such a test isn't likely to fail except
+  // for pathological cases.
+  //
+  // FIXME: Also, a record doesn't necessarily need begin() and end(). Free
+  // functions called begin() and end() taking the container as an argument
+  // are also allowed.
+  TypeMatcher RecordWithBeginEnd = 
+    qualType(anyOf(
+      qualType(
+        isConstQualified(),
+        hasDeclaration(
+          recordDecl(
+            hasMethod(
+              methodDecl(
+                hasName("begin"),
+                isConst()
+              )
+            ),
+            hasMethod(
+              methodDecl(
+                hasName("end"),
+                isConst()
+              )
+            )
+          )
+        ) // hasDeclaration
+      ), // qualType
+      qualType(
+        unless(isConstQualified()),
+        hasDeclaration(
+          recordDecl(
+            hasMethod(hasName("begin")),
+            hasMethod(hasName("end"))
+          )
+        )
+      ) // qualType
+    )
+  );
+
   StatementMatcher SizeCallMatcher =
       memberCallExpr(argumentCountIs(0),
                      callee(methodDecl(anyOf(hasName("size"),
-                                             hasName("length")))));
+                                             hasName("length")))),
+                     on(anyOf(hasType(pointsTo(RecordWithBeginEnd)),
+                              hasType(RecordWithBeginEnd))));
 
   StatementMatcher EndInitMatcher =
       expr(anyOf(
Index: test/cpp11-migrate/LoopConvert/free_begin_end_fail.cpp
===================================================================
--- /dev/null
+++ test/cpp11-migrate/LoopConvert/free_begin_end_fail.cpp
@@ -0,0 +1,32 @@
+// RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp
+// RUN: cpp11-migrate -loop-convert %t.cpp -- -I %S/Inputs -std=c++11
+// RUN: FileCheck -input-file=%t.cpp %s
+// XFAIL: *
+
+struct MyArray {
+  unsigned size();
+};
+
+template <typename T>
+struct MyContainer {
+};
+
+int *begin(const MyArray &Arr);
+int *end(const MyArray &Arr);
+
+template <typename T>
+T *begin(const MyContainer<T> &C);
+template <typename T>
+T *end(const MyContainer<T> &C);
+
+// The Loop Convert Transform doesn't detect free functions begin()/end() and
+// so fails to transform these cases which it should.
+void f() {
+  MyArray Arr;
+  for (unsigned i = 0, e = Arr.size(); i < e; ++i) {}
+  // CHECK: for (auto & elem : Arr) {}
+
+  MyContainer<int> C;
+  for (int *I = begin(C), *E = end(C); I != E; ++I) {}
+  // CHECK: for (auto & elem : C) {}
+}
Index: test/cpp11-migrate/LoopConvert/pseudoarray.cpp
===================================================================
--- test/cpp11-migrate/LoopConvert/pseudoarray.cpp
+++ test/cpp11-migrate/LoopConvert/pseudoarray.cpp
@@ -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
 #include "structures.h"
 
@@ -64,3 +64,42 @@
   for (auto i = 0; i < v.size(); ++i) ;
   // CHECK: for (auto & elem : v) ;
 }
+
+struct NoBeginEnd {
+  unsigned size() const;
+};
+
+struct NoConstBeginEnd {
+  NoConstBeginEnd();
+  unsigned size() const;
+  unsigned begin();
+  unsigned end();
+};
+
+struct ConstBeginEnd {
+  ConstBeginEnd();
+  unsigned size() const;
+  unsigned begin() const;
+  unsigned end() const;
+};
+
+// Shouldn't transform pseudo-array uses if the container doesn't provide
+// begin() and end() of the right const-ness.
+void NoBeginEndTest() {
+  NoBeginEnd NBE;
+  for (unsigned i = 0, e = NBE.size(); i < e; ++i) {}
+  // CHECK: for (unsigned i = 0, e = NBE.size(); i < e; ++i) {}
+
+  const NoConstBeginEnd const_NCBE;
+  for (unsigned i = 0, e = const_NCBE.size(); i < e; ++i) {}
+  // CHECK: for (unsigned i = 0, e = const_NCBE.size(); i < e; ++i) {}
+
+  ConstBeginEnd CBE;
+  for (unsigned i = 0, e = CBE.size(); i < e; ++i) {}
+  // CHECK: for (auto & elem : CBE) {}
+
+  const ConstBeginEnd const_CBE;
+  for (unsigned i = 0, e = const_CBE.size(); i < e; ++i) {}
+  // CHECK: for (auto & elem : const_CBE) {}
+}
+
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D775.1.patch
Type: text/x-patch
Size: 4968 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130509/51b80d2b/attachment.bin>