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

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

Quuxplusone added inline comments.


================
Comment at: libcxx/test/support/test_iterators.h:170
+public:
+    typedef          std::forward_iterator_tag                 iterator_category;
+    typedef typename std::iterator_traits<It>::value_type      value_type;
----------------
zoecarver wrote:
> ldionne wrote:
> > Shouldn't this be `std::iterator_traits<It>::iterator_category`? Actually, we could perhaps do this instead:
> > 
> > ```
> > template<class It>
> > struct std::iterator_traits<no_default_constructor<It>> : std::iterator_traits<It> { };
> > ```
> > 
> > Then, could we simply drop all the typedefs in `no_default_constructor`?
> No, because we only provide members such that this is a forward iterator. For example, this is used mostly with pointer types. If we did what you're suggesting these would report that they were contiguous_iterators, but they are not (they're not even bidirectional). 
Turn this inside out.
```
template<class T>
class NonDefCtablePtr {
    T *p_;
public:
    using iterator_category = std::random_access_iterator_tag;
    using value_type = T;
    using difference_type = std::ptrdiff_t;
    [...]
    constexpr NonDefCtablePtr(T *p) : p_(p) {}
    constexpr T& operator*() const { return *p_; }
    [...]
    friend constexpr T *base(NonDefCtablePtr p) { return p.p_; }
};
```
and then in your test cases, use
```
#include "test_iterator.h"

test<bidirectional_iterator<int*>>();  // bidirectional, default-constructible
test<bidirectional_iterator<NonDefCtablePtr<int>>>();  // bidirectional, non-default-constructible
```
That is, we already have the iterator wrapper types in order to "downgrade" `int*` to forward/bidirectional/random-access level. So your `NonDefCtable` type should not "downgrade." It should just be contiguous, and then if someone wants a downgraded version, they can wrap it in one of the downgrade wrappers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103335