[libcxx-commits] [PATCH] D132505: [libc++] Refactor deque::iterator algorithm optimizations

[Hui via Phabricator via libcxx-commits]

huixie90 added inline comments.


================
Comment at: libcxx/include/__algorithm/copy.h:33
+          __enable_if_t<!(is_copy_constructible<_InIter>::value
+                       && is_copy_constructible<_Sent>::value
+                       && is_copy_constructible<_OutIter>::value), int> = 0>
----------------
philnik wrote:
> huixie90 wrote:
> > sentinel is always copyable.
> > this applies for other places
> Since this is pre-existing I'd rather fix it in a follow-up. This patch is already quite large.
sounds good to me


================
Comment at: libcxx/include/__algorithm/copy.h:86
+          class _OutIter,
+          __enable_if_t<__segmented_iterator_traits<_InIter>::__is_segmented_iterator::value, int> = 0>
+inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 pair<_InIter, _OutIter>
----------------
philnik wrote:
> huixie90 wrote:
> > As we get more and more optimisations for different types, it is harder to make sure all of these overloads are mutually exclusive. Do you think this is (or will be) a problem?
> I think this is a problem. But I don't really have a good idea how to fix it. Using `if constexpr` would probably do the job, but we don't have that option.
I feel that someone is going to promote his priority_tag thing


================
Comment at: libcxx/include/__iterator/segmented_iterator.h:17-25
+// - `static <undefined> __get_forward_range(_Iterator __first, _Iterator __last);`
+//   where <undefined> is a range which contains the segments to iterate over. The segments are iterated front to back.
+// - `static <undefined> __get_backward_range(_Iterator __first, _Iterator __last);`
+//   where <undefined> is a range which contains the segments to iterate over. The segments are iterated back to front.
+// - `static <undefined> __get_range_to_segment_begin(_Iterator __end)`
+//   where <undefined> is a struct where [__begin_, __end_) is a range from begin of the segment to
+//   the passed-in iterator
----------------
philnik wrote:
> huixie90 wrote:
> > It might be a good idea to put these functions in the primary template `__segmented_iterator_traits` and `=delete` them. This way it is more self documentary. What do you think?
> I didn't define the return type more specifically here on purpose, so what should I return from the deleted functions?
perhaps just return `auto`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132505