[libc++] [ranges] Fix a missing auto(x) cast in ranges::data.

[• Quuxplusone]

Summary

Also remove some bogus std::forwards. My impression is that these forwards were actually harmless, because ranges::begin(FWD(t)) is always identical to ranges::begin(t) (except when it's ill-formed, and that can't happen in this case). However, they're also superfluous and don't reflect the wording in the standard, so let's eliminate them.

[• Quuxplusone]

Consistency in the new test cases.

[philnik]

LGTM

[ldionne]

libcxx/include/__ranges/data.h

39

FWIW, these were changed after the original "one ranges proposal", which is what we implemented originally. So I suspect there's going to be at least a couple of papers to trivially mark as having been implemented when we take a look.

libcxx/include/__ranges/data.h

 // -*- C++ -*-
 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 #ifndef _LIBCPP___RANGES_DATA_H
 #define _LIBCPP___RANGES_DATA_H
 
 #include <__config>
 #include <__iterator/concepts.h>
 #include <__iterator/iterator_traits.h>
 #include <__memory/pointer_traits.h>
 #include <__ranges/access.h>
-#include <__utility/forward.h>
-#include <concepts>
+#include <__utility/auto_cast.h>
 #include <type_traits>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
 #endif
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 #if !defined(_LIBCPP_HAS_NO_RANGES)
 
 // [range.prim.data]
 
 namespace ranges {
 namespace __data {
   template <class _Tp>
   concept __ptr_to_object = is_pointer_v<_Tp> && is_object_v<remove_pointer_t<_Tp>>;
 
   template <class _Tp>
   concept __member_data =
+    __can_borrow<_Tp> &&
     requires(_Tp&& __t) {
-      { _VSTD::forward<_Tp>(__t) } -> __can_borrow;
-      { __t.data() } -> __ptr_to_object;
+      { _LIBCPP_AUTO_CAST(__t.data()) } -> __ptr_to_object;
     };
 
   template <class _Tp>
   concept __ranges_begin_invocable =
     !__member_data<_Tp> &&
+    __can_borrow<_Tp> &&
     requires(_Tp&& __t) {
-      { _VSTD::forward<_Tp>(__t) } -> __can_borrow;
-      { ranges::begin(_VSTD::forward<_Tp>(__t)) } -> contiguous_iterator;
+      { ranges::begin(__t) } -> contiguous_iterator;
     };
 
   struct __fn {
     template <__member_data _Tp>
-      requires __can_borrow<_Tp>
     _LIBCPP_HIDE_FROM_ABI
-    constexpr __ptr_to_object auto operator()(_Tp&& __t) const
+    constexpr auto operator()(_Tp&& __t) const
         noexcept(noexcept(__t.data())) {
       return __t.data();
     }
 
     template<__ranges_begin_invocable _Tp>
-      requires __can_borrow<_Tp>
     _LIBCPP_HIDE_FROM_ABI
-    constexpr __ptr_to_object auto operator()(_Tp&& __t) const
-        noexcept(noexcept(_VSTD::to_address(ranges::begin(_VSTD::forward<_Tp>(__t))))) {
-      return _VSTD::to_address(ranges::begin(_VSTD::forward<_Tp>(__t)));
+    constexpr auto operator()(_Tp&& __t) const
+        noexcept(noexcept(_VSTD::to_address(ranges::begin(__t)))) {
+      return _VSTD::to_address(ranges::begin(__t));
     }
   };
 }
 
 inline namespace __cpo {
   inline constexpr auto data = __data::__fn{};
 } // namespace __cpo
 } // namespace ranges
 
 #endif // !defined(_LIBCPP_HAS_NO_RANGES)
 
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP___RANGES_DATA_H

libcxx/test/std/ranges/range.access/data.pass.cpp

 static_assert(!std::is_invocable_v<RangeDataT, int [1]>);
 static_assert(!std::is_invocable_v<RangeDataT, int (&&)[1]>);
 static_assert( std::is_invocable_v<RangeDataT, int (&)[1]>);
 
 struct DataMember {
   int x;
   constexpr const int *data() const { return &x; }
 };
-
 static_assert( std::is_invocable_v<RangeDataT, DataMember &>);
 static_assert(!std::is_invocable_v<RangeDataT, DataMember &&>);
+static_assert( std::is_invocable_v<RangeDataT, DataMember const&>);
+static_assert(!std::is_invocable_v<RangeDataT, DataMember const&&>);
+
+constexpr bool testReturnTypes() {
+  {
+    int *x[2];
+    ASSERT_SAME_TYPE(decltype(std::ranges::data(x)), int**);
+  }
+  {
+    int x[2][2];
+    ASSERT_SAME_TYPE(decltype(std::ranges::data(x)), int(*)[2]);
+  }
+  {
+    struct D {
+      char*& data();
+      short*& data() const;
+    };
+    ASSERT_SAME_TYPE(decltype(std::ranges::data(std::declval<D&>())), char*);
+    static_assert(!std::is_invocable_v<RangeDataT, D&&>);
+    ASSERT_SAME_TYPE(decltype(std::ranges::data(std::declval<const D&>())), short*);
+    static_assert(!std::is_invocable_v<RangeDataT, const D&&>);
+  }
+  {
+    struct NC {
+      char *begin() const;
+      char *end() const;
+      int *data();
+    };
+    static_assert(!std::ranges::contiguous_range<NC>);
+    static_assert( std::ranges::contiguous_range<const NC>);
+    ASSERT_SAME_TYPE(decltype(std::ranges::data(std::declval<NC&>())), int*);
+    static_assert(!std::is_invocable_v<RangeDataT, NC&&>);
+    ASSERT_SAME_TYPE(decltype(std::ranges::data(std::declval<const NC&>())), char*);
+    static_assert(!std::is_invocable_v<RangeDataT, const NC&&>);
+  }
+  return true;
+}
 
 struct VoidDataMember {
   void *data() const;
 };
 static_assert(!std::is_invocable_v<RangeDataT, VoidDataMember const&>);
 
 struct Empty { };
 struct EmptyDataMember {
   Empty data() const;
 };
-
 static_assert(!std::is_invocable_v<RangeDataT, EmptyDataMember const&>);
 
-struct EmptyPtrDataMember {
-  Empty x;
-  constexpr const Empty *data() const { return &x; }
-};
-
-struct PtrConvertible {
+struct PtrConvertibleDataMember {
+  struct Ptr {
     operator int*() const;
   };
-struct PtrConvertibleDataMember {
-  PtrConvertible data() const;
+  Ptr data() const;
 };
-
 static_assert(!std::is_invocable_v<RangeDataT, PtrConvertibleDataMember const&>);
 
 struct NonConstDataMember {
   int x;
   constexpr int *data() { return &x; }
 };
 
 struct EnabledBorrowingDataMember {
   constexpr int *data() { return &globalBuff[0]; }
 };
-
 template<>
 inline constexpr bool std::ranges::enable_borrowed_range<EnabledBorrowingDataMember> = true;
 
 struct DataMemberAndBegin {
   int x;
   constexpr const int *data() const { return &x; }
-  constexpr const int *begin() const { return &x; }
+  const int *begin() const;
 };
 
 constexpr bool testDataMember() {
   DataMember a;
   assert(std::ranges::data(a) == &a.x);
 
   NonConstDataMember b;
   assert(std::ranges::data(b) == &b.x);

 static_assert(!std::is_invocable_v<RangeDataT, const BeginFriendRandomAccess&>);
 static_assert(!std::is_invocable_v<RangeDataT, const BeginFriendRandomAccess&&>);
 
 struct BeginMemberRvalue {
   int buff[8];
 
   ContiguousIter begin() &&;
 };
+static_assert(!std::is_invocable_v<RangeDataT, BeginMemberRvalue&>);
 static_assert(!std::is_invocable_v<RangeDataT, BeginMemberRvalue&&>);
 static_assert(!std::is_invocable_v<RangeDataT, BeginMemberRvalue const&>);
+static_assert(!std::is_invocable_v<RangeDataT, BeginMemberRvalue const&&>);
 
 struct BeginMemberBorrowingEnabled {
   constexpr contiguous_iterator<int*> begin() { return contiguous_iterator<int*>{&globalBuff[1]}; }
 };
 template<>
 inline constexpr bool std::ranges::enable_borrowed_range<BeginMemberBorrowingEnabled> = true;
 static_assert( std::is_invocable_v<RangeDataT, BeginMemberBorrowingEnabled &>);
 static_assert( std::is_invocable_v<RangeDataT, BeginMemberBorrowingEnabled &&>);

 
 struct RandomButNotContiguous {
   random_access_iterator<int*> begin() const;
   random_access_iterator<int*> end() const;
 };
 static_assert(!std::is_invocable_v<RangeDataT, RandomButNotContiguous>);
 
 int main(int, char**) {
+  static_assert(testReturnTypes());
+
   testDataMember();
   static_assert(testDataMember());
 
   testViaRangesBegin();
   static_assert(testViaRangesBegin());
 
   return 0;
 }

libcxx/test/std/ranges/range.req/range.refinements/contiguous_range.compile.pass.cpp

 
 struct ContiguousWhenNonConst {
     const int *begin() const;
     const int *end() const;
     int *begin();
     int *end();
     int *data() const;
 };
-static_assert( std::ranges::random_access_range<ContiguousWhenNonConst>);
 static_assert( std::ranges::contiguous_range<ContiguousWhenNonConst>);
+static_assert( std::ranges::random_access_range<const ContiguousWhenNonConst>);
 static_assert(!std::ranges::contiguous_range<const ContiguousWhenNonConst>);
 
 struct ContiguousWhenConst {
     const int *begin() const;
     const int *end() const;
     int *begin();
     int *end();
     const int *data() const;
 };
+static_assert( std::ranges::contiguous_range<const ContiguousWhenConst>);
 static_assert( std::ranges::random_access_range<ContiguousWhenConst>);
 static_assert(!std::ranges::contiguous_range<ContiguousWhenConst>);
-static_assert( std::ranges::contiguous_range<const ContiguousWhenConst>);
 
 struct DataFunctionWrongReturnType {
     const int *begin() const;
     const int *end() const;
     const char *data() const;
 };
 static_assert( std::ranges::random_access_range<DataFunctionWrongReturnType>);
-static_assert(!std::ranges::contiguous_range<const DataFunctionWrongReturnType>);
+static_assert(!std::ranges::contiguous_range<DataFunctionWrongReturnType>);
 
 struct WrongObjectness {
     const int *begin() const;
     const int *end() const;
     void *data() const;
 };
 static_assert(std::ranges::contiguous_range<WrongObjectness>);