[libcxx-commits] [PATCH] D106840: [libcxx][ranges] Add `std::ranges::single_view`.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/include/__ranges/copyable_box.h:182
+
+    _LIBCPP_HIDE_FROM_ABI constexpr const _Tp *operator->() const noexcept { return &__val_; }
+    _LIBCPP_HIDE_FROM_ABI constexpr _Tp *operator->() noexcept { return &__val_; }
----------------
`_VSTD::addressof(__val_)`

Also, would it be possible to add a test for it, like we do for `operator*`?


================
Comment at: libcxx/include/__ranges/single_view.h:1
+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
----------------
Could you add `views::single` at the same time? It's pretty small so it makes sense to implement both in the same go, I think.

Also, this makes me think that we don't have `views::empty`, right?


================
Comment at: libcxx/include/__ranges/single_view.h:40
+    _LIBCPP_HIDE_FROM_ABI
+    constexpr explicit single_view(const _Tp& __t) : __value_(__t) {}
+
----------------
Here you could use `__value_(std::in_place, __t)` instead, and you would not have to modify `__copyable_box`. Same for the rvalue ref below.


================
Comment at: libcxx/test/std/ranges/range.factories/range.single.view/size.pass.cpp:13
+
+// static constexpr size_t size() noexcept;
+
----------------
Since the spec documents this as being `static`, can you add a test that `std::ranges::single_view<int>::size()` works as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106840