[libcxx-commits] [PATCH] D103840: [libcxx][ranges][nfc] Factor out common logic in the copy algorithms.

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone requested changes to this revision.
Quuxplusone added inline comments.


================
Comment at: libcxx/include/__algorithm/copy.h:83
+copy(_InputIterator __first, _InputIterator __last, _OutputIterator __result) {
+  return __copy_impl(__first, __last, __result).__get_out();
+}
----------------
`_VSTD::` — but also I worry a little bit about the codegen here. As you've written it so far, `ranges::copy` could just be
```
return __in_out_result{__last, _VSTD::copy(__first, __last, __result)};
```
and save a lot of modifications. Which is obviously not C++20-compliant. So for C++20, we're going to have to write a new `ranges::copy` algorithm that returns //the computed iterator, generated by incrementing `__first`, which ended up comparing equal to `_Sentinel __last`.// That's a much different-looking algorithm from what we've got today for `std::copy`, and might not even be able to share any code with it.

I strongly strongly strongly ask that you continue working on this PR until it succeeds at implementing the C++20 `ranges::copy` semantics, and then see if you still think this is a good direction. I strongly strongly strongly oppose merging this PR in its current state.


================
Comment at: libcxx/test/std/algorithms/robust_against_adl.pass.cpp:15
 
+// TODO: figure out what this is testing and how we can continue to test it.
+
----------------
This is testing that your library code is robust against ADL.
I assume it's failing at the moment because you didn't ADL-proof your call to `__copy` above.
Remember, every time you call a function in namespace `_VSTD` that would be subject to ADL, you need to qualify it. (The one common exception we make is for `declval`, because it takes no arguments.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103840