[libcxx-commits] [libcxx] [libc++] Merge the implementations of ranges::copy_n and std::copy_n (PR #157444)

[Louis Dionne via libcxx-commits]

================
@@ -13,48 +13,51 @@
 #include <__config>
 #include <__iterator/iterator_traits.h>
 #include <__type_traits/enable_if.h>
-#include <__utility/convert_to_integral.h>
+#include <__utility/move.h>
+#include <__utility/pair.h>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
 #endif
 
+_LIBCPP_PUSH_MACROS
+#include <__undef_macros>
+
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-template <class _InputIterator,
-          class _Size,
-          class _OutputIterator,
-          __enable_if_t<__has_input_iterator_category<_InputIterator>::value &&
-                            !__has_random_access_iterator_category<_InputIterator>::value,
-                        int> = 0>
-inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _OutputIterator
-copy_n(_InputIterator __first, _Size __orig_n, _OutputIterator __result) {
-  typedef decltype(std::__convert_to_integral(__orig_n)) _IntegralSize;
-  _IntegralSize __n = __orig_n;
-  if (__n > 0) {
+template <class _InIter,
+          class _DiffType,
+          class _OutIter,
+          __enable_if_t<__has_random_access_iterator_category<_InIter>::value, int> = 0>
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 pair<_InIter, _OutIter>
+__copy_n(_InIter __first, _DiffType __n, _OutIter __result) {
+  return std::__copy(__first, __first + __n, std::move(__result));
+}
+
+template <class _InIter,
+          class _DiffType,
+          class _OutIter,
+          __enable_if_t<!__has_random_access_iterator_category<_InIter>::value, int> = 0>
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 pair<_InIter, _OutIter>
+__copy_n(_InIter __first, _DiffType __n, _OutIter __result) {
+  while (__n != 0) {
     *__result = *__first;
+    ++__first;
     ++__result;
-    for (--__n; __n > 0; --__n) {
-      ++__first;
-      *__result = *__first;
-      ++__result;
-    }
+    --__n;
   }
-  return __result;
+  return std::make_pair(std::move(__first), std::move(__result));
 }
 
-template <class _InputIterator,
-          class _Size,
-          class _OutputIterator,
-          __enable_if_t<__has_random_access_iterator_category<_InputIterator>::value, int> = 0>
-inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _OutputIterator
-copy_n(_InputIterator __first, _Size __orig_n, _OutputIterator __result) {
-  typedef typename iterator_traits<_InputIterator>::difference_type difference_type;
-  typedef decltype(std::__convert_to_integral(__orig_n)) _IntegralSize;
-  _IntegralSize __n = __orig_n;
----------------
ldionne wrote:

The code seems to very purposefully converting to an integral type *and then* to the iterator's difference type. I wonder if that's necessary for correctness or whether it just happened to be written that way. However, I vaguely recall having a discussion about that on an old PR.

I think we should figure out whether this matters before we can land this part of the patch. Alternatively, I'm also happy to progress on this patch if you go back to the previous usage of `__convert_to_integral`. In fact, since we use `__convert_to_integral` in a bunch of places, it would probably be better to tackle that change as an orthogonal patch.

https://github.com/llvm/llvm-project/pull/157444