[libcxx-commits] [PATCH] D122982: [libc++] Implement ranges::copy{, _n, _if, _backward}

Mark de Wever via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Wed Apr 6 09:52:45 PDT 2022

Mordante added inline comments.

Comment at: libcxx/include/__algorithm/copy.h:47
+  if (__libcpp_is_constant_evaluated()
+// TODO: Remove this once GCC supports __builtin_memmove during constant evaluation
Do you know whether this is available in GCC-12?

Comment at: libcxx/include/__algorithm/copy.h:61
+                     && is_copy_constructible<_Sent>::value
+                     && is_copy_constructible<_OutIter>::value> >
philnik wrote:
> Mordante wrote:
> > Do we need to have an additional specialization? where
> > ```
> >          __enable_if_t<is_copy_constructible<_InIter>::value
> >                       && is_copy_constructible<_Sent>::value
> >                       && !is_copy_constructible<_OutIter>::value> >
> > ```
> > Just to make sure that a copy of a `std::string` to a non-copyable output iterator is still unwrapped.
> > This is will be used in `std::format`.
> What would you expect to change between it being wrapped and not? It's not like we can optimize that in any way.
The cpp20 output iterator isn't copy constructible. But `std::basic_string<CharT>::iterator` can still be unwrapped. So I wondered whether we want an overload for that case. Something like
template <class _InIter, class _Sent, class _OutIter,
                      && is_copy_constructible<_Sent>::value> >
  pair<_InIter, _OutIter> __copy(_InIter __first, _Sent __last, _OutIter __result) {
  auto __ret = std::__copy_impl(std::__unwrap_iter(__first), std::__unwrap_iter(__last), std::move__result));
  return std::make_pair(std::__rewrap_iter(__first, __ret.first), _ret.second);

Comment at: libcxx/include/__algorithm/copy_backward.h:56
-        __result -= __n;
-        _VSTD::memmove(__result, __first, __n * sizeof(_Up));
-    }
philnik wrote:
> Mordante wrote:
> > Did you have a look at the codegen for this change?
> What change are you asking for exactly? The change from `std::memmove` to `__builtin_memmove`?
No the removal of this overload. Does that affect the generated code or not.

Comment at: libcxx/include/__algorithm/ranges_copy_if.h:49
+    }
+    return {std::move(__first), std::move(__result)};
+  }
philnik wrote:
> Mordante wrote:
> > The original is correct, but this is closer to the wording of the Standard.
> The problem is that your version isn't correct. `_Sent` doesn't have to be the same type as `_InIter`.
I trusted the wording of the Standard, but it seems that's wrong ;-)
  {last, result + N} for the overloads in namespace ranges.
Do you want to file an LWG issue?

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list