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

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

Quuxplusone added a subscriber: CaseyCarter.
Quuxplusone 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;
----------------
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 ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101404