[libcxx-commits] [PATCH] D117656: [libc++] [ranges] Implement move_sentinel and C++20 move_iterator

[Joe Loser via Phabricator via libcxx-commits]

jloser added inline comments.


================
Comment at: libcxx/include/__iterator/move_iterator.h:99
+#if _LIBCPP_STD_VER > 17 && !defined(_LIBCPP_HAS_NO_CONCEPTS)
+    _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX14
+    move_iterator() requires is_constructible_v<_Iter> : __current_() {}
----------------
`s/_LIBCPP_CONSTEXPR_AFTER_CXX14/constexpr` here and below since we're already guarded with `#if _LIBCPP_STD_VER > 17`.


================
Comment at: libcxx/include/__iterator/move_iterator.h:196
+    bool operator==(const move_iterator& __x, const move_sentinel<_Sent>& __y)
+        requires __move_iter_comparable<_Iter, _Sent>
+    {
----------------
Quuxplusone wrote:
> I found that writing the naïve `requires requires { __x.base() == __y.base(); }` here did //not// work; Clang complains something about accessing into incomplete type `move_iterator<X>` when you go to instantiate it. Pulling the constraint out into a concept //outside// the body of the class seems to fix the issue.
I'll buy that. I've also run into this several times in other codebases with `requires requires` expressions in member functions of a class template. It's not great, but the usual "pull it out to a named concept" works.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117656