[ADT] Allow reverse to find free rbegin/rend functions (#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.

Author: kuhar

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>

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.

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};

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()),

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");