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

[Louis Dionne via Phabricator via libcxx-commits]

ldionne 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;
+
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95983