[libcxx-commits] [PATCH] D116991: [libc++] [ranges] SFINAE away ranges::cbegin(const T&&) for non-borrowed T.

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone created this revision.
Quuxplusone added reviewers: CaseyCarter, ldionne, libc++.
Quuxplusone added a project: libc++.
Quuxplusone requested review of this revision.
Herald added a subscriber: libcxx-commits.
Herald added 1 blocking reviewer(s): libc++.

Fixes https://github.com/llvm/llvm-project/issues/52952 — thanks @CaseyCarter for the bug report!

I believe the issue here is fundamentally similar to https://quuxplusone.github.io/blog/2021/07/30/perfect-forwarding-call-wrapper/ — we have an overload set where we really want //only one of the overloads// to ever be viable, but sometimes it can happen that the overload we want to select is SFINAEd away but another overload can be viable with just a slight conversion of the argument type (for example, in this case, converting the argument type `const BeginMember&&` to the viable overload's parameter type `const BeginMember&`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116991

Files:
  libcxx/include/__ranges/access.h
  libcxx/test/std/ranges/range.access/begin.pass.cpp
  libcxx/test/std/ranges/range.access/end.pass.cpp


Index: libcxx/test/std/ranges/range.access/end.pass.cpp
===================================================================
--- libcxx/test/std/ranges/range.access/end.pass.cpp
+++ libcxx/test/std/ranges/range.access/end.pass.cpp
@@ -37,16 +37,19 @@
 
 struct EndMember {
   int x;
-  constexpr const int *begin() const { return nullptr; }
+  const int *begin() const;
   constexpr const int *end() const { return &x; }
 };
 
 // Ensure that we can't call with rvalues with borrowing disabled.
-static_assert( std::is_invocable_v<RangeEndT,  EndMember &>);
-static_assert( std::is_invocable_v<RangeEndT,  EndMember const&>);
-static_assert(!std::is_invocable_v<RangeEndT,  EndMember &&>);
+static_assert( std::is_invocable_v<RangeEndT, EndMember &>);
+static_assert(!std::is_invocable_v<RangeEndT, EndMember &&>);
+static_assert( std::is_invocable_v<RangeEndT, EndMember const&>);
+static_assert(!std::is_invocable_v<RangeEndT, EndMember const&&>);
 static_assert( std::is_invocable_v<RangeCEndT, EndMember &>);
+static_assert(!std::is_invocable_v<RangeCEndT, EndMember &&>);
 static_assert( std::is_invocable_v<RangeCEndT, EndMember const&>);
+static_assert(!std::is_invocable_v<RangeCEndT, EndMember const&&>);
 
 constexpr bool testReturnTypes() {
   {
Index: libcxx/test/std/ranges/range.access/begin.pass.cpp
===================================================================
--- libcxx/test/std/ranges/range.access/begin.pass.cpp
+++ libcxx/test/std/ranges/range.access/begin.pass.cpp
@@ -48,7 +48,7 @@
 static_assert( std::is_invocable_v<RangeCBeginT, BeginMember &>);
 static_assert(!std::is_invocable_v<RangeCBeginT, BeginMember &&>);
 static_assert( std::is_invocable_v<RangeCBeginT, BeginMember const&>);
-static_assert( std::is_invocable_v<RangeCBeginT, BeginMember const&&>);
+static_assert(!std::is_invocable_v<RangeCBeginT, BeginMember const&&>);
 
 constexpr bool testReturnTypes() {
   {
Index: libcxx/include/__ranges/access.h
===================================================================
--- libcxx/include/__ranges/access.h
+++ libcxx/include/__ranges/access.h
@@ -163,11 +163,12 @@
 namespace __cbegin {
   struct __fn {
     template <class _Tp>
+      requires is_lvalue_reference_v<_Tp&&>
     [[nodiscard]] _LIBCPP_HIDE_FROM_ABI
-    constexpr auto operator()(_Tp& __t) const
-      noexcept(noexcept(ranges::begin(static_cast<const _Tp&>(__t))))
-      -> decltype(      ranges::begin(static_cast<const _Tp&>(__t)))
-      { return          ranges::begin(static_cast<const _Tp&>(__t)); }
+    constexpr auto operator()(_Tp&& __t) const
+      noexcept(noexcept(ranges::begin(static_cast<const __uncvref_t<_Tp>&>(__t))))
+      -> decltype(      ranges::begin(static_cast<const __uncvref_t<_Tp>&>(__t)))
+      { return          ranges::begin(static_cast<const __uncvref_t<_Tp>&>(__t)); }
 
     template <class _Tp>
       requires is_rvalue_reference_v<_Tp&&>
@@ -190,11 +191,12 @@
 namespace __cend {
   struct __fn {
     template <class _Tp>
+      requires is_lvalue_reference_v<_Tp&&>
     [[nodiscard]] _LIBCPP_HIDE_FROM_ABI
-    constexpr auto operator()(_Tp& __t) const
-      noexcept(noexcept(ranges::end(static_cast<const _Tp&>(__t))))
-      -> decltype(      ranges::end(static_cast<const _Tp&>(__t)))
-      { return          ranges::end(static_cast<const _Tp&>(__t)); }
+    constexpr auto operator()(_Tp&& __t) const
+      noexcept(noexcept(ranges::end(static_cast<const __uncvref_t<_Tp>&>(__t))))
+      -> decltype(      ranges::end(static_cast<const __uncvref_t<_Tp>&>(__t)))
+      { return          ranges::end(static_cast<const __uncvref_t<_Tp>&>(__t)); }
 
     template <class _Tp>
       requires is_rvalue_reference_v<_Tp&&>


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D116991.398828.patch
Type: text/x-patch
Size: 3686 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20220111/afdb5416/attachment-0001.bin>