[libcxx-commits] [PATCH] D117961: [libc++] Fix LWG3589 "The const lvalue reference overload of get for subrange..."

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

Quuxplusone marked an inline comment as done.
Quuxplusone added inline comments.


================
Comment at: libcxx/include/__ranges/subrange.h:228
   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
----------------
Mordante wrote:
> I don't mind `(_Index == 1)`, but the current code doesn't match the LWG issue.
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.


================
Comment at: libcxx/test/std/ranges/range.utility/range.subrange/get.pass.cpp:49
+    assert(base(std::get<0>(std::move(r))) == a);
+    assert(base(base(std::get<1>(std::move(r)))) == a + 3);
+  }
----------------
Mordante wrote:
> Use after move of `r`, likewise in the two blocks below.
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.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117961/new/

https://reviews.llvm.org/D117961