[llvm] [ADT] Allow reverse to find free rbegin/rend functions (PR #87840)
Jakub Kuderski via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 5 16:28:14 PDT 2024
https://github.com/kuhar created https://github.com/llvm/llvm-project/pull/87840
Lift the requirement that rbegin/rend must be member functions. Also allow the rbegin/rend to be found through Argument Dependent Lookup (ADL) and add `adl_rbegin`/`adl_rend` to STLExtras.
>From 40c47d8a3126a64b2b89ea8f909cffb38531a681 Mon Sep 17 00:00:00 2001
From: Jakub Kuderski <jakub at nod-labs.com>
Date: Fri, 5 Apr 2024 19:22:37 -0400
Subject: [PATCH] [ADT] Allow reverse to find free rbegin/rend functions
Lift the requirement that rbegin/rend must be member functions.
Also allow the rbegin/rend to be found through Argument Dependent Lookup
(ADL) and add `adl_rbegin`/`adl_rend` to STLExtras.
---
llvm/include/llvm/ADT/ADL.h | 32 ++++++++++++++++++++++++
llvm/include/llvm/ADT/STLExtras.h | 33 +++++++++----------------
llvm/unittests/ADT/IteratorTest.cpp | 15 +++++++++++
llvm/unittests/ADT/RangeAdapterTest.cpp | 4 ---
llvm/unittests/ADT/STLExtrasTest.cpp | 10 ++++++++
5 files changed, 69 insertions(+), 25 deletions(-)
diff --git a/llvm/include/llvm/ADT/ADL.h b/llvm/include/llvm/ADT/ADL.h
index ab1f28ff6b9c09..812d9a4b52d818 100644
--- a/llvm/include/llvm/ADT/ADL.h
+++ b/llvm/include/llvm/ADT/ADL.h
@@ -37,6 +37,22 @@ constexpr auto end_impl(RangeT &&range)
return end(std::forward<RangeT>(range));
}
+using std::rbegin;
+
+template <typename RangeT>
+constexpr auto rbegin_impl(RangeT &&range)
+ -> decltype(rbegin(std::forward<RangeT>(range))) {
+ return rbegin(std::forward<RangeT>(range));
+}
+
+using std::rend;
+
+template <typename RangeT>
+constexpr auto rend_impl(RangeT &&range)
+ -> decltype(rend(std::forward<RangeT>(range))) {
+ return rend(std::forward<RangeT>(range));
+}
+
using std::swap;
template <typename T>
@@ -72,6 +88,22 @@ constexpr auto adl_end(RangeT &&range)
return adl_detail::end_impl(std::forward<RangeT>(range));
}
+/// Returns the reverse-begin iterator to \p range using `std::rbegin` and
+/// function found through Argument-Dependent Lookup (ADL).
+template <typename RangeT>
+constexpr auto adl_rbegin(RangeT &&range)
+ -> decltype(adl_detail::rbegin_impl(std::forward<RangeT>(range))) {
+ return adl_detail::rbegin_impl(std::forward<RangeT>(range));
+}
+
+/// Returns the reverse-end iterator to \p range using `std::rend` and
+/// functions found through Argument-Dependent Lookup (ADL).
+template <typename RangeT>
+constexpr auto adl_rend(RangeT &&range)
+ -> decltype(adl_detail::rend_impl(std::forward<RangeT>(range))) {
+ return adl_detail::rend_impl(std::forward<RangeT>(range));
+}
+
/// Swaps \p lhs with \p rhs using `std::swap` and functions found through
/// Argument-Dependent Lookup (ADL).
template <typename T>
diff --git a/llvm/include/llvm/ADT/STLExtras.h b/llvm/include/llvm/ADT/STLExtras.h
index b5eb319ccf346e..08a708e5c5871e 100644
--- a/llvm/include/llvm/ADT/STLExtras.h
+++ b/llvm/include/llvm/ADT/STLExtras.h
@@ -405,32 +405,23 @@ class mapped_iterator_base
}
};
-/// Helper to determine if type T has a member called rbegin().
-template <typename Ty> class has_rbegin_impl {
- using yes = char[1];
- using no = char[2];
-
- template <typename Inner>
- static yes& test(Inner *I, decltype(I->rbegin()) * = nullptr);
-
- template <typename>
- static no& test(...);
-
-public:
- static const bool value = sizeof(test<Ty>(nullptr)) == sizeof(yes);
-};
+namespace detail {
+template <typename Range>
+using check_has_free_function_rbegin =
+ decltype(adl_rbegin(std::declval<Range &>()));
-/// Metafunction to determine if T& or T has a member called rbegin().
-template <typename Ty>
-struct has_rbegin : has_rbegin_impl<std::remove_reference_t<Ty>> {};
+template <typename Range>
+static constexpr bool HasFreeFunctionRBegin =
+ is_detected<check_has_free_function_rbegin, Range>::value;
+} // namespace detail
// Returns an iterator_range over the given container which iterates in reverse.
template <typename ContainerTy> auto reverse(ContainerTy &&C) {
- if constexpr (has_rbegin<ContainerTy>::value)
- return make_range(C.rbegin(), C.rend());
+ if constexpr (detail::HasFreeFunctionRBegin<ContainerTy>)
+ return make_range(adl_rbegin(C), adl_rend(C));
else
- return make_range(std::make_reverse_iterator(std::end(C)),
- std::make_reverse_iterator(std::begin(C)));
+ return make_range(std::make_reverse_iterator(adl_end(C)),
+ std::make_reverse_iterator(adl_begin(C)));
}
/// An iterator adaptor that filters the elements of given inner iterators.
diff --git a/llvm/unittests/ADT/IteratorTest.cpp b/llvm/unittests/ADT/IteratorTest.cpp
index 3a4a5b02b1040f..a0d3c9b564d857 100644
--- a/llvm/unittests/ADT/IteratorTest.cpp
+++ b/llvm/unittests/ADT/IteratorTest.cpp
@@ -395,6 +395,21 @@ TEST(PointerIterator, Range) {
EXPECT_EQ(A + I++, P);
}
+namespace rbegin_detail {
+struct WithFreeRBegin {
+ int data[3] = {42, 43, 44};
+};
+
+auto rbegin(const WithFreeRBegin &X) { return std::rbegin(X.data); }
+auto rend(const WithFreeRBegin &X) { return std::rend(X.data); }
+} // namespace rbegin_detail
+
+TEST(ReverseTest, ADL) {
+ // Check that we can find the rbegin/rend functions via ADL.
+ rbegin_detail::WithFreeRBegin Foo;
+ EXPECT_THAT(reverse(Foo), ElementsAre(44, 43, 42));
+}
+
TEST(ZipIteratorTest, Basic) {
using namespace std;
const SmallVector<unsigned, 6> pi{3, 1, 4, 1, 5, 9};
diff --git a/llvm/unittests/ADT/RangeAdapterTest.cpp b/llvm/unittests/ADT/RangeAdapterTest.cpp
index eb75ac301805bc..c1a8a984f233b0 100644
--- a/llvm/unittests/ADT/RangeAdapterTest.cpp
+++ b/llvm/unittests/ADT/RangeAdapterTest.cpp
@@ -150,10 +150,6 @@ TYPED_TEST(RangeAdapterRValueTest, TrivialOperation) {
TestRev(reverse(TypeParam({0, 1, 2, 3})));
}
-TYPED_TEST(RangeAdapterRValueTest, HasRbegin) {
- static_assert(has_rbegin<TypeParam>::value, "rbegin() should be defined");
-}
-
TYPED_TEST(RangeAdapterRValueTest, RangeType) {
static_assert(
std::is_same_v<decltype(reverse(std::declval<TypeParam>()).begin()),
diff --git a/llvm/unittests/ADT/STLExtrasTest.cpp b/llvm/unittests/ADT/STLExtrasTest.cpp
index b73891b59f0264..3927bc59c031a3 100644
--- a/llvm/unittests/ADT/STLExtrasTest.cpp
+++ b/llvm/unittests/ADT/STLExtrasTest.cpp
@@ -405,6 +405,14 @@ std::vector<int>::const_iterator end(const some_struct &s) {
return s.data.end();
}
+std::vector<int>::const_reverse_iterator rbegin(const some_struct &s) {
+ return s.data.rbegin();
+}
+
+std::vector<int>::const_reverse_iterator rend(const some_struct &s) {
+ return s.data.rend();
+}
+
void swap(some_struct &lhs, some_struct &rhs) {
// make swap visible as non-adl swap would even seem to
// work with std::swap which defaults to moving
@@ -573,6 +581,8 @@ TEST(STLExtrasTest, ADLTest) {
EXPECT_EQ(*adl_begin(s), 1);
EXPECT_EQ(*(adl_end(s) - 1), 5);
+ EXPECT_EQ(*adl_rbegin(s), 5);
+ EXPECT_EQ(*(adl_rend(s) - 1), 1);
adl_swap(s, s2);
EXPECT_EQ(s.swap_val, "lhs");
More information about the llvm-commits
mailing list