Reapply "ADT: Remove references in has_rbegin for reverse()"

This reverts commit r279086, reapplying r279084.  I'm not sure what I
ran before, because the compile failure for ADTTests reproduced locally.

The problem is that TestRev is calling BidirectionalVector::rbegin()
when the BidirectionalVector is const, but rbegin() is always non-const.
I've updated BidirectionalVector::rbegin() to be callable from const.

Original commit message follows.

--

As a follow-up to r278991, add some tests that check that
decltype(reverse(R).begin()) == decltype(R.rbegin()), and get them
passing by adding std::remove_reference to has_rbegin.

I'm using static_assert instead of EXPECT_TRUE (and updated the other
has_rbegin check from r278991 in the same way) since I figure that's
more helpful.

llvm-svn: 279091

llvm/include/llvm/ADT/STLExtras.h

@@ -208,9 +208,8 @@ inline mapped_iterator<ItTy, FuncTy> map_iterator(const ItTy &I, FuncTy F) {
   return mapped_iterator<ItTy, FuncTy>(I, F);
 }
 
-/// \brief Metafunction to determine if type T has a member called rbegin().
-template <typename Ty>
-class has_rbegin {
+/// Helper to determine if type T has a member called rbegin().
+template <typename Ty> class has_rbegin_impl {
   typedef char yes[1];
   typedef char no[2];
 
@@ -224,6 +223,11 @@ public:
   static const bool value = sizeof(test<Ty>(nullptr)) == sizeof(yes);
 };
 
+/// Metafunction to determine if T& or T has a member called rbegin().
+template <typename Ty>
+struct has_rbegin : has_rbegin_impl<typename std::remove_reference<Ty>::type> {
+};
+
 // Returns an iterator_range over the given container which iterates in reverse.
 // Note that the container must have rbegin()/rend() methods for this to work.
 template <typename ContainerTy>

llvm/unittests/ADT/RangeAdapterTest.cpp

@@ -38,18 +38,18 @@ public:
 // begin() and end() don't have implementations as this ensures that we will
 // get a linker error if reverse() chooses begin()/end() over rbegin(), rend().
 class BidirectionalVector {
-  std::vector<int> Vec;
+  mutable std::vector<int> Vec;
 
 public:
   BidirectionalVector(std::initializer_list<int> list) : Vec(list) {}
 
   typedef std::vector<int>::iterator iterator;
-  iterator begin();
-  iterator end();
+  iterator begin() const;
+  iterator end() const;
 
   typedef std::vector<int>::reverse_iterator reverse_iterator;
-  reverse_iterator rbegin() { return Vec.rbegin(); }
-  reverse_iterator rend() { return Vec.rend(); }
+  reverse_iterator rbegin() const { return Vec.rbegin(); }
+  reverse_iterator rend() const { return Vec.rend(); }
 };
 
 /// This is the same as BidirectionalVector but with the addition of const
@@ -75,6 +75,50 @@ public:
   const_reverse_iterator rend() const { return Vec.rend(); }
 };
 
+/// Check that types with custom iterators work.
+class CustomIteratorVector {
+  mutable std::vector<int> V;
+
+public:
+  CustomIteratorVector(std::initializer_list<int> list) : V(list) {}
+
+  typedef std::vector<int>::iterator iterator;
+  class reverse_iterator {
+    std::vector<int>::iterator I;
+
+  public:
+    reverse_iterator() = default;
+    reverse_iterator(const reverse_iterator &) = default;
+    reverse_iterator &operator=(const reverse_iterator &) = default;
+
+    explicit reverse_iterator(std::vector<int>::iterator I) : I(I) {}
+
+    reverse_iterator &operator++() {
+      --I;
+      return *this;
+    }
+    reverse_iterator &operator--() {
+      ++I;
+      return *this;
+    }
+    int &operator*() const { return *std::prev(I); }
+    int *operator->() const { return &*std::prev(I); }
+    friend bool operator==(const reverse_iterator &L,
+                           const reverse_iterator &R) {
+      return L.I == R.I;
+    }
+    friend bool operator!=(const reverse_iterator &L,
+                           const reverse_iterator &R) {
+      return !(L == R);
+    }
+  };
+
+  iterator begin() const { return V.begin(); }
+  iterator end()  const { return V.end(); }
+  reverse_iterator rbegin() const { return reverse_iterator(V.end()); }
+  reverse_iterator rend() const { return reverse_iterator(V.begin()); }
+};
+
 template <typename R> void TestRev(const R &r) {
   int counter = 3;
   for (int i : r)
@@ -98,9 +142,10 @@ TYPED_TEST(RangeAdapterLValueTest, TrivialOperation) {
 
 template <typename T> struct RangeAdapterRValueTest : testing::Test {};
 
-typedef ::testing::Types<std::vector<int>, std::list<int>, ReverseOnlyVector,
-                         BidirectionalVector,
-                         BidirectionalVectorConsts> RangeAdapterRValueTestTypes;
+typedef ::testing::Types<std::vector<int>, std::list<int>, CustomIteratorVector,
+                         ReverseOnlyVector, BidirectionalVector,
+                         BidirectionalVectorConsts>
+    RangeAdapterRValueTestTypes;
 TYPED_TEST_CASE(RangeAdapterRValueTest, RangeAdapterRValueTestTypes);
 
 TYPED_TEST(RangeAdapterRValueTest, TrivialOperation) {
@@ -108,7 +153,20 @@ TYPED_TEST(RangeAdapterRValueTest, TrivialOperation) {
 }
 
 TYPED_TEST(RangeAdapterRValueTest, HasRbegin) {
-  EXPECT_TRUE(has_rbegin<TypeParam>::value);
+  static_assert(has_rbegin<TypeParam>::value, "rbegin() should be defined");
+}
+
+TYPED_TEST(RangeAdapterRValueTest, RangeType) {
+  static_assert(
+      std::is_same<
+          decltype(reverse(*static_cast<TypeParam *>(nullptr)).begin()),
+          decltype(static_cast<TypeParam *>(nullptr)->rbegin())>::value,
+      "reverse().begin() should have the same type as rbegin()");
+  static_assert(
+      std::is_same<
+          decltype(reverse(*static_cast<const TypeParam *>(nullptr)).begin()),
+          decltype(static_cast<const TypeParam *>(nullptr)->rbegin())>::value,
+      "reverse().begin() should have the same type as rbegin() [const]");
 }
 
 } // anonymous namespace