[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:1649
+// Furthermore, we can't do this for user-defined iterators because their
+// `to_address` may be non-constexpr and/or have observable side effects.
+// Furthermore, some algorithms (e.g. std::copy, but not std::sort) need
----------------
It's been pointed out to me that this is fine; `std::copy` is required to have the `constexpr` keyword on the front, but it's never required to actually //be// constexpr. libc++ can say "`std::copy` is constexpr unless you give us a contiguous iterator with a non-constexpr `to_address` and/or `operator->`."


================
Comment at: libcxx/include/algorithm:1653
+// is no standard API: we rely on implicit conversion to do the right thing.
+// For user-defined iterators this isn't a conforming assumption.
+// So the only type we actually unwrap is our own __wrap_iter<T*>.
----------------
I still lack any great way to work around this one, though. I think if we want to use `__unwrap_iter(first)` on arbitrary user-provided contiguous iterators in `copy`, then we'll have to separately compute the return value using something like
```
template<class _OrigIter, class _UnwrappedIter>
_Iter __rewrap_iter(_OrigIter __first, _UnwrappedIter __result)
{
    static_assert(__is_cpp17_contiguous_iterator<_OrigIter>, "");  // otherwise we won't get here
    return __first + (__result - _VSTD::__unwrap_iter(__first));
}

template<class _OrigIter>
_Iter __rewrap_iter(_OrigIter, _OrigIter __result)
{
    return __result;
}
[...]
return _VSTD::__rewrap_iter(__result,
     _VSTD::__copy(_VSTD::__unwrap_iter(__first),
                   _VSTD::__unwrap_iter(__last),
                   _VSTD::__unwrap_iter(__result)));
```

This mostly destroys our hopes of "reducing the number of template instantiations"; but it does permit us to keep using `memmove` for user-defined contiguous iterators.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95983