[libcxx-commits] [PATCH] D142335: [libc++][ranges] Partially implement `ranges::to`.

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const added a comment.

@philnik Sorry, I uploaded just the last commit from my local branch last time. Should be fixed now.



================
Comment at: libcxx/include/deque:70
+    template<container-compatible-range<T> R>
+      void assign_range(R&& rg); // C++23
     void assign(size_type n, const value_type& v);
----------------
philnik wrote:
> throughout
This file uses this style, without `since`. I deliberately keep it consistent.


================
Comment at: libcxx/include/deque:668-674
+      if constexpr (ranges::forward_range<_Range> || ranges::sized_range<_Range>) {
+        auto __n = static_cast<size_type>(ranges::distance(__range));
+        __assign_with_size(ranges::begin(__range), ranges::end(__range), __n);
+
+      } else {
+        __assign_with_sentinel(ranges::begin(__range), ranges::end(__range));
+      }
----------------
philnik wrote:
> IMO it would be better to do this optimization in `__assign_whatever` to avoid having slightly different versions of this in different places.
The internal `__assign` functions don't have access to the range, only the iterators. It's possible to have a range where `size` takes constant time while `distance` is linear.


================
Comment at: libcxx/include/deque:792
+    void prepend_range(_Range&& __range) {
+      insert(begin(), std::forward<_Range>(__range));
+    }
----------------
philnik wrote:
> This looks completely wrong. Did you mean to use `insert_range`? I'm also not convinced this can be implemented this way. `prepend_range` has much stronger exception guarantees than `insert`. If we provide strong enough exception guarantees inside `insert` (which I don't think is the case), please add a comment.
Yes, it should be `insert_range`.

Where are the exception guarantees you're referring to specified? I was looking at https://eel.is/c++draft/deque.modifiers where `insert`, `insert_range` and `prepend_range` are grouped together and share the same `remarks` about exception safety.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142335