From 9d748f949977dc972694fb5a9f3a11575521f182 Mon Sep 17 00:00:00 2001 From: "Duncan P. N. Exon Smith" Date: Thu, 18 Aug 2016 17:15:25 +0000 Subject: [PATCH] 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 | 10 +++-- llvm/unittests/ADT/RangeAdapterTest.cpp | 76 +++++++++++++++++++++++++++++---- 2 files changed, 74 insertions(+), 12 deletions(-) diff --git a/llvm/include/llvm/ADT/STLExtras.h b/llvm/include/llvm/ADT/STLExtras.h index 8e4d186..eb515fd 100644 --- a/llvm/include/llvm/ADT/STLExtras.h +++ b/llvm/include/llvm/ADT/STLExtras.h @@ -208,9 +208,8 @@ inline mapped_iterator map_iterator(const ItTy &I, FuncTy F) { return mapped_iterator(I, F); } -/// \brief Metafunction to determine if type T has a member called rbegin(). -template -class has_rbegin { +/// Helper to determine if type T has a member called rbegin(). +template class has_rbegin_impl { typedef char yes[1]; typedef char no[2]; @@ -224,6 +223,11 @@ public: static const bool value = sizeof(test(nullptr)) == sizeof(yes); }; +/// Metafunction to determine if T& or T has a member called rbegin(). +template +struct has_rbegin : has_rbegin_impl::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 diff --git a/llvm/unittests/ADT/RangeAdapterTest.cpp b/llvm/unittests/ADT/RangeAdapterTest.cpp index 2b12fe4..4c7bef5 100644 --- a/llvm/unittests/ADT/RangeAdapterTest.cpp +++ b/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 Vec; + mutable std::vector Vec; public: BidirectionalVector(std::initializer_list list) : Vec(list) {} typedef std::vector::iterator iterator; - iterator begin(); - iterator end(); + iterator begin() const; + iterator end() const; typedef std::vector::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 V; + +public: + CustomIteratorVector(std::initializer_list list) : V(list) {} + + typedef std::vector::iterator iterator; + class reverse_iterator { + std::vector::iterator I; + + public: + reverse_iterator() = default; + reverse_iterator(const reverse_iterator &) = default; + reverse_iterator &operator=(const reverse_iterator &) = default; + + explicit reverse_iterator(std::vector::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 void TestRev(const R &r) { int counter = 3; for (int i : r) @@ -98,9 +142,10 @@ TYPED_TEST(RangeAdapterLValueTest, TrivialOperation) { template struct RangeAdapterRValueTest : testing::Test {}; -typedef ::testing::Types, std::list, ReverseOnlyVector, - BidirectionalVector, - BidirectionalVectorConsts> RangeAdapterRValueTestTypes; +typedef ::testing::Types, std::list, 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::value); + static_assert(has_rbegin::value, "rbegin() should be defined"); +} + +TYPED_TEST(RangeAdapterRValueTest, RangeType) { + static_assert( + std::is_same< + decltype(reverse(*static_cast(nullptr)).begin()), + decltype(static_cast(nullptr)->rbegin())>::value, + "reverse().begin() should have the same type as rbegin()"); + static_assert( + std::is_same< + decltype(reverse(*static_cast(nullptr)).begin()), + decltype(static_cast(nullptr)->rbegin())>::value, + "reverse().begin() should have the same type as rbegin() [const]"); } } // anonymous namespace -- 2.7.4