From b40bf83eab6049a06159ad69b6422de9d6f3e2ee Mon Sep 17 00:00:00 2001 From: Edwin Vane Date: Thu, 9 May 2013 20:03:52 +0000 Subject: [PATCH] Transform for loops over pseudo-arrays only if begin/end members exist 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. llvm-svn: 181539 --- .../LoopConvert/LoopMatchers.cpp | 49 ++++++++++++++++++- .../LoopConvert/free_begin_end_fail.cpp | 32 ++++++++++++ .../cpp11-migrate/LoopConvert/pseudoarray.cpp | 41 +++++++++++++++- 3 files changed, 120 insertions(+), 2 deletions(-) create mode 100644 clang-tools-extra/test/cpp11-migrate/LoopConvert/free_begin_end_fail.cpp diff --git a/clang-tools-extra/cpp11-migrate/LoopConvert/LoopMatchers.cpp b/clang-tools-extra/cpp11-migrate/LoopConvert/LoopMatchers.cpp index a7ca15f3eb3d..d69f2a888119 100644 --- a/clang-tools-extra/cpp11-migrate/LoopConvert/LoopMatchers.cpp +++ b/clang-tools-extra/cpp11-migrate/LoopConvert/LoopMatchers.cpp @@ -258,10 +258,57 @@ StatementMatcher makeIteratorLoopMatcher() { /// - 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( diff --git a/clang-tools-extra/test/cpp11-migrate/LoopConvert/free_begin_end_fail.cpp b/clang-tools-extra/test/cpp11-migrate/LoopConvert/free_begin_end_fail.cpp new file mode 100644 index 000000000000..27f7e8bf75ec --- /dev/null +++ b/clang-tools-extra/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 +struct MyContainer { +}; + +int *begin(const MyArray &Arr); +int *end(const MyArray &Arr); + +template +T *begin(const MyContainer &C); +template +T *end(const MyContainer &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 C; + for (int *I = begin(C), *E = end(C); I != E; ++I) {} + // CHECK: for (auto & elem : C) {} +} diff --git a/clang-tools-extra/test/cpp11-migrate/LoopConvert/pseudoarray.cpp b/clang-tools-extra/test/cpp11-migrate/LoopConvert/pseudoarray.cpp index 37105f9e0b40..5aeaf79fc9ff 100644 --- a/clang-tools-extra/test/cpp11-migrate/LoopConvert/pseudoarray.cpp +++ b/clang-tools-extra/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 @@ void noContainer() { 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) {} +} +