[libcxx-commits] [PATCH] D103335: [libcxx][ranges] Adds `common_iterator`.

[Casey Carter via Phabricator via libcxx-commits]

CaseyCarter added inline comments.


================
Comment at: libcxx/test/std/iterators/predef.iterators/iterators.common/iterator_traits.compile.pass.cpp:56
+    static_assert(std::same_as<IterTraits::difference_type, std::ptrdiff_t>);
+    static_assert(std::same_as<IterTraits::pointer, const Iter&>); // TODO: is this right?
+    static_assert(std::same_as<IterTraits::reference, int&>);
----------------
tcanens wrote:
> zoecarver wrote:
> > zoecarver wrote:
> > > ldionne wrote:
> > > > This doesn't look right. According to the spec (http://eel.is/c++draft/iterators.common#common.iter.types-1.3), this should either be the type of `operator->()` or `void`, but never `Iter const&`. This means you most likely have a bug in the implementation.
> > > > 
> > > > Also, please add a test where `pointer` ends up being `void`.
> > > > This doesn't look right. According to the spec (http://eel.is/c++draft/iterators.common#common.iter.types-1.3), this should either be the type of operator->() or void, but never Iter const&. This means you most likely have a bug in the implementation.
> > > 
> > > Turns out this is actually correct. This is supposed to be the return type of **common_iterator**'s `operator->()`. And if `_Iter` has an `operator->()` then it's supposed to simply return `get<Iter>(v)`. (However, I'd argue that should maybe return `get<Iter>(v).operator->()`, if you agree, maybe I'll file an LWG issue.)
> > > 
> > > Sound good now? This threw me off too :P
> > > Also, please add a test where pointer ends up being void.
> > 
> > Done.
> > > This doesn't look right. According to the spec (http://eel.is/c++draft/iterators.common#common.iter.types-1.3), this should either be the type of operator->() or void, but never Iter const&. This means you most likely have a bug in the implementation.
> > 
> > Turns out this is actually correct. This is supposed to be the return type of **common_iterator**'s `operator->()`. And if `_Iter` has an `operator->()` then it's supposed to simply return `get<Iter>(v)`. (However, I'd argue that should maybe return `get<Iter>(v).operator->()`, if you agree, maybe I'll file an LWG issue.)
> 
> Producing `decltype(a.operator->())` matches http://eel.is/c++draft/iterator.traits#1.sentence-3 but not the actual wording in http://eel.is/c++draft/iterators.common#common.iter.types-1.3 which says that it is the type of `a.operator->()`, aka `remove_reference_t<decltype(a.operator->())>`. What's the intent here @CaseyCarter?
http://eel.is/c++draft/iterator.traits#1.sentence-3 is authoritative; http://eel.is/c++draft/iterators.common#common.iter.types-1.3 should be corrected to agree. Presumably the writer of the latter paragraph was under the impression that `a.operator->()` is always a prvalue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103335