[libcxx-commits] [PATCH] D99855: [libcxx] makes `iterator_traits` C++20-aware

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver added inline comments.


================
Comment at: libcxx/include/iterator:725
+  using difference_type    = typename _Ip::difference_type;
+  using pointer            = typename __iterator_traits_member_pointer<_Ip>::type;
+  using reference          = typename _Ip::reference;
----------------
ldionne wrote:
> For the primary template, `pointer` should be (http://eel.is/c++draft/iterator.traits#3.1):
> 
> > If the qualified-id `I​::​pointer` is valid and denotes a type, then `iterator_­traits<I>​::​pointer` names that type; otherwise, it names `void`.
> 
> We shouldn't check for `operator->()`, so using `__iterator_traits_member_pointer<_Ip>` (which checks for `operator->()`) doesn't sound correct to me. We should also add a test that catches that: define a type `T` from which we could deduce `iterator_traits<T>::pointer` using `operator->()` but not otherwise, and confirm that `iterator_traits<T>::pointer` is `void`. Before fixing this comment, the result would have been whatever's deduced by `operator->`.
> 
Yes, @ldionne you're right. Here's an example (from Arthur's godbolt) which catches this:
```
struct NoPointerMember {
  struct iterator_category {};
  struct value_type {};
  struct difference_type {};
  struct reference {};
  value_type* operator->() const;  // ignored, because B is not a LegacyInputIterator
};
static_assert(std::same_as<NoPointerMemberTraits::pointer, void>);
```

On both MSVC and GCC this compiles just fine, but we (currently) think the pointer type should be `value_type*`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99855