[libc++] Fix LWG3589 "The const lvalue reference overload of get for subrange..."

[• Quuxplusone]

Summary

https://cplusplus.github.io/LWG/issue3589

[Mordante]

libcxx/include/__ranges/subrange.h

230

I don't mind (_Index == 1), but the current code doesn't match the LWG issue.

libcxx/test/std/ranges/range.utility/range.subrange/get.pass.cpp

49

Use after move of r, likewise in the two blocks below.

[• Quuxplusone]

libcxx/include/__ranges/subrange.h

230

I'll throw parens around the whole expression, because now I'm paranoid some compiler won't like top-level requires A || B; but I'll keep the parens on the individual terms too, for clarity.

libcxx/test/std/ranges/range.utility/range.subrange/get.pass.cpp

49

Since this copies the sentinel even when r is an rvalue, I think it's OK — might even be a test-coverage benefit!
Or I guess rather: Since std::get<0>(std::move(r)) isn't supposed to touch the sentinel member of r, (etc.)

[• Quuxplusone]

Tweak parens (I went with @Mordante's version after all), re-poke CI just for the heck of it.

[• Quuxplusone]

Re-poke CI, since "patch application failed" last time.

[• Quuxplusone]

Mark two ranges tests as UNSUPPORTED: libcpp-no-unsupported-ranges to fix https://buildkite.com/llvm-project/libcxx-ci/builds/8211
(D118507 is tangentially related.)

[• Quuxplusone]

My last diff misspelled libcpp-has-no-incomplete-ranges, and anyway I just pushed that fix, so remove it from here again. Poke CI.

[Mordante]

LGTM with one minor issue.

libcxx/test/std/ranges/range.utility/range.subrange/get.pass.cpp

49

Please add a comment why this is safe

libcxx/docs/Status/Cxx2bIssues.csv

 `3570 <https://wg21.link/LWG3570>`__,"``basic_osyncstream::emit`` should be an unformatted output function","October 2021","",""
 `3571 <https://wg21.link/LWG3571>`__,"``flush_emit`` should set ``badbit`` if the ``emit`` call fails","October 2021","",""
 `3572 <https://wg21.link/LWG3572>`__,"``copyable-box`` should be fully ``constexpr``","October 2021","|Complete|","14.0","|ranges|"
 `3573 <https://wg21.link/LWG3573>`__,"Missing Throws element for ``basic_string_view(It begin, End end)``","October 2021","|Complete|","14.0"
 `3574 <https://wg21.link/LWG3574>`__,"``common_iterator`` should be completely ``constexpr``-able","October 2021","|Complete|","14.0","|ranges|"
 `3580 <https://wg21.link/LWG3580>`__,"``iota_view``'s ``iterator``'s binary ``operator+`` should be improved","October 2021","|Complete|","14.0","|ranges|"
 `3581 <https://wg21.link/LWG3581>`__,"The range constructor makes ``basic_string_view`` not trivially move constructible","October 2021","|Complete|","14.0","|ranges|"
 `3585 <https://wg21.link/LWG3585>`__,"``variant`` converting assignment with immovable alternative","October 2021","",""
-`3589 <https://wg21.link/LWG3589>`__,"The ``const`` lvalue reference overload of ``get`` for ``subrange`` does not constrain ``I`` to be ``copyable`` when ``N == 0``","October 2021","","","|ranges|"
+`3589 <https://wg21.link/LWG3589>`__,"The ``const`` lvalue reference overload of ``get`` for ``subrange`` does not constrain ``I`` to be ``copyable`` when ``N == 0``","October 2021","|Complete|","14.0","|ranges|"
 `3590 <https://wg21.link/LWG3590>`__,"``split_view::base() const &`` is overconstrained","October 2021","","","|ranges|"
 `3591 <https://wg21.link/LWG3591>`__,"``lazy_split_view<input_view>::inner-iterator::base() &&`` invalidates outer iterators","October 2021","","","|ranges|"
 `3592 <https://wg21.link/LWG3592>`__,"``lazy_split_view`` needs to check the simpleness of Pattern","October 2021","","","|ranges|"
 `3593 <https://wg21.link/LWG3593>`__,"Several iterators' ``base() const &`` and ``lazy_split_view::outer-iterator::value_type::end()`` missing ``noexcept``","October 2021","","","|ranges|"
 `3595 <https://wg21.link/LWG3595>`__,"Exposition-only classes proxy and postfix-proxy for ``common_iterator`` should be fully ``constexpr``","October 2021","|Complete|","14.0","|ranges|"
 "","","","",""
 `3645 <https://wg21.link/LWG3645>`__,"``resize_and_overwrite`` is overspecified to call its callback with lvalues", "Not voted in","|Complete|","14.0",""
 "","","","",""

libcxx/include/__ranges/subrange.h

subrange(_Range&&) -> subrange<iterator_t<_Range>, sentinel_t<_Range>,
                                  (sized_range<_Range> || sized_sentinel_for<sentinel_t<_Range>, iterator_t<_Range>>)
                                    ? subrange_kind::sized : subrange_kind::unsized>;
 
   template<borrowed_range _Range>
   subrange(_Range&&, make_unsigned_t<range_difference_t<_Range>>)
     -> subrange<iterator_t<_Range>, sentinel_t<_Range>, subrange_kind::sized>;
 
   template<size_t _Index, class _Iter, class _Sent, subrange_kind _Kind>
-    requires (_Index < 2)
+    requires ((_Index == 0 && copyable<_Iter>) || _Index == 1)
   _LIBCPP_HIDE_FROM_ABI
   constexpr auto get(const subrange<_Iter, _Sent, _Kind>& __subrange) {
     if constexpr (_Index == 0)
       return __subrange.begin();
     else
       return __subrange.end();
   }
 

libcxx/test/std/ranges/range.utility/range.subrange/get.pass.cpp

 
 // class std::ranges::subrange;
 
 #include <ranges>
 
 #include <cassert>
 #include "test_macros.h"
 #include "test_iterators.h"
-#include "types.h"
 
 template<size_t I, class S>
-concept GetInvocable = requires {
+concept HasGet = requires {
   std::get<I>(std::declval<S>());
 };
 
-static_assert( GetInvocable<0, std::ranges::subrange<int*>>);
-static_assert( GetInvocable<1, std::ranges::subrange<int*>>);
-static_assert(!GetInvocable<2, std::ranges::subrange<int*>>);
-static_assert(!GetInvocable<3, std::ranges::subrange<int*>>);
+static_assert( HasGet<0, std::ranges::subrange<int*>>);
+static_assert( HasGet<1, std::ranges::subrange<int*>>);
+static_assert(!HasGet<2, std::ranges::subrange<int*>>);
+static_assert(!HasGet<3, std::ranges::subrange<int*>>);
 
 constexpr bool test() {
-  std::ranges::subrange<int*> a(globalBuff, globalBuff + 8, 8);
-  assert(std::get<0>(a) == a.begin());
-  assert(std::get<1>(a) == a.end());
-
-  assert(a.begin() == std::get<0>(std::move(a)));
-  std::ranges::subrange<int*> b(globalBuff, globalBuff + 8, 8);
-  assert(b.end() == std::get<1>(std::move(b)));
+  {
+    using It = int*;
+    using Sent = sentinel_wrapper<int*>;
+    int a[] = {1, 2, 3};
+    using R = std::ranges::subrange<It, Sent, std::ranges::subrange_kind::unsized>;
+    R r = R(It(a), Sent(It(a + 3)));
+    ASSERT_SAME_TYPE(decltype(std::get<0>(r)), It);
+    ASSERT_SAME_TYPE(decltype(std::get<1>(r)), Sent);
+    ASSERT_SAME_TYPE(decltype(std::get<0>(static_cast<R&&>(r))), It);
+    ASSERT_SAME_TYPE(decltype(std::get<1>(static_cast<R&&>(r))), Sent);
+    ASSERT_SAME_TYPE(decltype(std::get<0>(static_cast<const R&>(r))), It);
+    ASSERT_SAME_TYPE(decltype(std::get<1>(static_cast<const R&>(r))), Sent);
+    ASSERT_SAME_TYPE(decltype(std::get<0>(static_cast<const R&&>(r))), It);
+    ASSERT_SAME_TYPE(decltype(std::get<1>(static_cast<const R&&>(r))), Sent);
+    assert(base(std::get<0>(r)) == a);                      // copy from It
+    assert(base(base(std::get<1>(r))) == a + 3);            // copy from Sent
+    assert(base(std::get<0>(std::move(r))) == a);           // copy from It
+    assert(base(base(std::get<1>(std::move(r)))) == a + 3); // copy from Sent
+  }
+  {
+    using It = int*;
+    using Sent = sentinel_wrapper<int*>;
+    int a[] = {1, 2, 3};
+    using R = std::ranges::subrange<It, Sent, std::ranges::subrange_kind::sized>;
+    R r = R(It(a), Sent(It(a + 3)), 3);
+    ASSERT_SAME_TYPE(decltype(std::get<0>(r)), It);
+    ASSERT_SAME_TYPE(decltype(std::get<1>(r)), Sent);
+    ASSERT_SAME_TYPE(decltype(std::get<0>(static_cast<R&&>(r))), It);
+    ASSERT_SAME_TYPE(decltype(std::get<1>(static_cast<R&&>(r))), Sent);
+    ASSERT_SAME_TYPE(decltype(std::get<0>(static_cast<const R&>(r))), It);
+    ASSERT_SAME_TYPE(decltype(std::get<1>(static_cast<const R&>(r))), Sent);
+    ASSERT_SAME_TYPE(decltype(std::get<0>(static_cast<const R&&>(r))), It);
+    ASSERT_SAME_TYPE(decltype(std::get<1>(static_cast<const R&&>(r))), Sent);
+    assert(base(std::get<0>(r)) == a);                      // copy from It
+    assert(base(base(std::get<1>(r))) == a + 3);            // copy from Sent
+    assert(base(std::get<0>(std::move(r))) == a);           // copy from It
+    assert(base(base(std::get<1>(std::move(r)))) == a + 3); // copy from Sent
+  }
+  {
+    // Test the fix for LWG 3589.
+    using It = cpp20_input_iterator<int*>;
+    using Sent = sentinel_wrapper<It>;
+    int a[] = {1, 2, 3};
+    using R = std::ranges::subrange<It, Sent>;
+    R r = R(It(a), Sent(It(a + 3)));
+    static_assert(!HasGet<0, R&>);
+    ASSERT_SAME_TYPE(decltype(std::get<1>(r)), Sent);
+    ASSERT_SAME_TYPE(decltype(std::get<0>(static_cast<R&&>(r))), It);
+    ASSERT_SAME_TYPE(decltype(std::get<1>(static_cast<R&&>(r))), Sent);
+    static_assert(!HasGet<0, const R&>);
+    ASSERT_SAME_TYPE(decltype(std::get<1>(static_cast<const R&>(r))), Sent);
+    static_assert(!HasGet<0, const R&&>);
+    ASSERT_SAME_TYPE(decltype(std::get<1>(static_cast<const R&&>(r))), Sent);
+    assert(base(base(std::get<1>(r))) == a + 3);            // copy from Sent
+    assert(base(std::get<0>(std::move(r))) == a);           // move from It
+    assert(base(base(std::get<1>(std::move(r)))) == a + 3); // copy from Sent
+  }
 
   return true;
 }
 
 int main(int, char**) {
   test();
   static_assert(test());
 
   return 0;
 }