[libcxx-commits] [PATCH] D141699: [In Progress][libc++][ranges] Implement P2474R2(`views::repeat`).

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added a comment.

In D141699#4054724 <https://reviews.llvm.org/D141699#4054724>, @yronglin wrote:

> Hi, @Mordante , can you please help me confirm that it this a bug in clang? https://godbolt.org/z/Pcefr19Yh

I don't think so. You're missing a few constraints in the second declaration. That doesn't really matter though, see inline comments.



================
Comment at: libcxx/include/__ranges/repeat_view.h:31
+
+#define _LIBCPP_STD_VER 23
+
----------------
This seems wrong.


================
Comment at: libcxx/include/__ranges/repeat_view.h:35
+
+#if _LIBCPP_STD_VER > 17
+
----------------



================
Comment at: libcxx/include/__ranges/repeat_view.h:43
+private:
+  struct __iterator;
+
----------------
You can't make this a member of `repeat_view` because it makes specializing traits impossible. Instead, make it a class inside `namespace std` and add an alias here. You can look at `istream_view` for an example of this.


================
Comment at: libcxx/include/__ranges/repeat_view.h:117
+
+  constexpr __iterator& operator++() {
+    ++__current_;
----------------
These should all be `_LIBCPP_HIDE_FROM_ABI`.


================
Comment at: libcxx/include/__ranges/repeat_view.h:194
+    requires __can_unbound_repeat_view<_Tp>
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr auto operator()(_Tp&& __value) const {
+    return repeat_view(std::forward<_Tp>(__value));
----------------
This should be `_LIBCPP_NODISCARD_EXT`. Please add a test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141699