[libcxx-commits] [PATCH] D129040: [libc++] Fix unwrapping ranges with different iterators and sentinels

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/include/__algorithm/copy.h:13
 #include <__algorithm/unwrap_iter.h>
+#include <__algorithm/unwrap_range.h>
 #include <__config>
----------------
Mordante wrote:
> philnik wrote:
> > Mordante wrote:
> > > Please also add this to the top level `algorithm` header.
> > Why should we do that? It's an implementation detail, and adding the include to the top level header just exposes that implementation detail (even for modules users).
> Are other header not allowed to use this? `unwrap_iter` is also in `<algorithm>` so IMO this should be there too.
Other headers are allowed to use this, but they should include `__algorithm/unwrap_range.h` and not `algorithm`.


================
Comment at: libcxx/include/__algorithm/unwrap_range.h:48
+
+  _LIBCPP_HIDE_FROM_ABI static constexpr auto __rewrap(_Iter, _Iter __iter) { return __iter; }
+};
----------------
ldionne wrote:
> This would avoid a copy?
It turns out that this causes the overloads to be ambiguous. I'll try to improve it later, since this code probably still needs sum debugging.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129040