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

[Christopher Di Bella via Phabricator via libcxx-commits]

cjdb added inline comments.


================
Comment at: libcxx/include/iterator:712
 
+template<class> struct __iterator_traits_member_pointer;
+template<class> struct __iterator_traits_member_reference;
----------------
Mordante wrote:
> Does this hunk affect the synopsis?
Apparently not. Our synopsis for `iterator_traits` is far more in-depth than [iterator.synopsis], so I'm not sure if we want to go the extra mile and add it. If anything, I'd prefer to delete some of our header's synopsis to match the standard.


================
Comment at: libcxx/include/iterator:741
+  using reference          = typename _Ip::reference;
+  using __primary_template = iterator_traits;
+};
----------------
Mordante wrote:
> Will `__primary_template` be used in a later patch?
Yes (next patch), but it also is a part of the standard to make this the primary template :-)


================
Comment at: libcxx/include/iterator:755
+// [iterator.traits]/3.2
+// Otherwise, if `I` satisfies the exposition-only concept _`cpp17-input-iterator`_,
+// `iterator_­traits<I>` has the following publicly accessible members:
----------------
Mordante wrote:
> I rather remove the underscores in _`cpp17-input-iterator`_,". I don't think they add clarity. The same for the other places where you used these concepts.
Sure, this isn't Markdown anyway.


================
Comment at: libcxx/test/std/iterators/iterator.primitives/iterator.traits/cxx20_iterator_traits.pass.cpp:16
+
+#include <iterator>
+
----------------
Mordante wrote:
> I would like some tests using the `contiguous_iterator_tag` see whether it acts as a `random_access_iterator_tag`.
Is that in scope for this test? I need a bit of convincing :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99855