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

Nikolas Klauser via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Sun Apr 16 12:35:57 PDT 2023


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

Did you upload the wrong patch?



================
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);
----------------
throughout


================
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));
+      }
----------------
IMO it would be better to do this optimization in `__assign_whatever` to avoid having slightly different versions of this in different places.


================
Comment at: libcxx/include/deque:792
+    void prepend_range(_Range&& __range) {
+      insert(begin(), std::forward<_Range>(__range));
+    }
----------------
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.


================
Comment at: libcxx/include/deque:970
 
+    template <class _Iterator, class _Sentinel>
+    _LIBCPP_HIDE_FROM_ABI
----------------
Why not `_Iter` and `_Sent` like we use elsewhere?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142335



More information about the libcxx-commits mailing list