[libcxx-commits] [PATCH] D95983: [libc++] Essentially revert D94807, to avoid calling __unwrap_iter in constexpr contexts.

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

Quuxplusone added inline comments.


================
Comment at: libcxx/include/algorithm:1656
 
-template <class _Iter, bool = __is_cpp17_contiguous_iterator<_Iter>::value>
+template <class _Iter> class __wrap_iter;
+
----------------
ldionne wrote:
> I don't understand this change - didn't you work around the `constexpr` problem by *not* calling `unwrap_iter` on iterators when we're in a constexpr context?
> 
> Edit: Ah, I think you're trying to work around the *second* problem you pointed out, which is that you can't automatically re-create an arbitrary user-defined iterator from a raw pointer. Right? A solution for that could be to:
> 
> 1. Introduce an indirection like `__rewrap_iter_impl` with a nested `__apply` that creates an iterator from a raw pointer. That can be specialized as needed.
> 2. Provide a default implementation for all iterators that are implicitly constructible from a pointer to their `value_type`.
> 
> That way, if the iterator isn't implicitly constructible from the pointer, a user like Chrome can still get the optimization by specializing the struct to tell libc++ how to get an iterator back from a pointer. And by default, it'll work for most iterators out of the box. Thoughts on that solution?
> 
> The only issue I can see is if implicit construction from a raw pointer is syntactically valid but doesn't do what we think it should do. I'm not sure how likely or dangerous that could be, but I would argue that if a contiguous iterator is constructible from a pointer but that constructor doesn't do the right thing, then you really voided your warranty with the Standard Library.
> That way, if the iterator isn't implicitly constructible from the pointer, a user like Chrome can still get the optimization by specializing the struct to tell libc++ how to get an iterator back from a pointer.

I don't think we need `__rewrap_iter` to be a customization point at all. A contiguous iterator (even a user-defined one) is always a random-access iterator too, so we can use `__first + (__result - _VSTD::__unwrap_iter(__first))` as I indicated in my Phab comment a few lines above this. (If `__unwrap_iter` is a no-op, meaning that `_OrigIter` wasn't a contiguous iterator, just return `__result` unmodified.)

> I would argue that if a contiguous iterator is constructible from a pointer but that constructor doesn't do the right thing, then you really voided your warranty with the Standard Library.

I strongly agree in fantasy-everything-as-it-should-be-land, but I'm less sure in actual-C++-land. Remember this is the language that invented `std::pointer_traits<It>::to_address(it)` and `std::pointer_traits<It>::pointer_to(ptr)` instead of just using `static_cast<T*>(it)` and `static_cast<It>(ptr)`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95983