[libcxx-commits] [PATCH] D105753: [libcxx][ranges] Add `ranges::common_view`.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

Please make sure you go through https://libcxx.llvm.org/Contributing.html (I think you've got everything except the visibility macros, which I added recently to the list, but it might be worth double-checking).

This basically LGTM. You can update this and ping me again, but it should be a no-brainer.



================
Comment at: libcxx/include/__ranges/common_view.h:39
+  requires (!common_range<_View> && copyable<iterator_t<_View>>)
+class common_view : public view_interface<common_view<_View>> {
+  _View __base_ = _View();
----------------
Missing `_LIBCPP_HIDE_FROM_ABI` everywhere. Also missing from `common_iterator.h`, sorry for not noticing earlier.


Note: Yes, that's a pain. If someone wants to work on solving this a better way, let me know, I've got some ideas with inline namespaces that might be worth exploring.


================
Comment at: libcxx/test/std/ranges/range.adaptors/range.common.view/ctor.view.pass.cpp:54
+    std::ranges::common_view<ContiguousView> common(ContiguousView{buffer});
+    if (!std::is_constant_evaluated())
+      assert(*common.begin() == 1);
----------------
Same comment as above with `std::is_constant_evaluated()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105753