[libcxx-commits] [PATCH] D101404: [libc++] Comment preconditions for __wrap_iter; make it work with C++20 to_address

[Casey Carter via Phabricator via libcxx-commits]

CaseyCarter added inline comments.


================
Comment at: libcxx/include/iterator:1280
     typedef typename iterator_traits<iterator_type>::pointer           pointer;
+    typedef typename pointer_traits<pointer>::element_type             element_type;
     typedef typename iterator_traits<iterator_type>::reference         reference;
----------------
Quuxplusone wrote:
> tcanens wrote:
> > zoecarver wrote:
> > > Quuxplusone wrote:
> > > > If my comments are correct, then this could+should use `pointer_traits<_Iter>` instead of `pointer_traits<pointer>`...
> > > > ...and the `_If` on line 1284 is unconditionally true, and can be simplified.
> > > > Please try that simplification (at least locally) and see if it passes the test suite.
> > > Alternatively we could add `element_type` with an `_If` (or maybe inheritance?) to make this work for non-pointer-like iterators. 
> > It's surprising to me that this is even needed. What breaks without this change?
> In the old code, `pointer_traits<std::vector<int>::iterator>::element_type` would be `int*`, not `int`. https://godbolt.org/z/PTPvn11bf
> Providing a user-provided `element_type` fixes that (in both C++17 and C++20).
> 
> Interestingly, libstdc++ and MSVC STL //also// report `pointer_traits<std::vector<int>::iterator>::element_type` as something other than `int`. I don't know if this is a conscious decision ("contiguous iterators needn't support pointer_traits") or if they just haven't gotten around to that part of C++20 yet. @CaseyCarter ?
https://godbolt.org/z/ba6z9cEbe demonstrates fairly clearly that `pointer_traits<vector<T>::iterator>::element_type` is `T` for MSVC STL. It looks like GCC didn't need to specialize `pointer_traits` for the `to_address` requirement: presumably `operator->` always works with their pointer-wrapper type. This is not the case for our `_Vector_iterator` which asserts in debug mode if you call `operator->` on a non-dereferenceable iterator.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101404