What should I do to make my container work with ranges?

Question

I have a simple container:

template <class T, class Allocator = std::allocator<T>>
class ring
{
public:

    using value_type = T;
    using allocator_type = Allocator;
    using size_type = std::size_t;
    using difference_type = std::ptrdiff_t;
    using reference = T &;
    using const_reference = const T &;
    using pointer = T *;
    using const_pointer = const T *;

private:

    template <class E>
    class ring_iterator
    {
    public:

        using iterator_category = std::random_access_iterator_tag;
        using value_type = E;
        using difference_type = std::ptrdiff_t;
        using reference = E &;
        using pointer = E *;

        ring_iterator(const ring_iterator& other) = default;
        ring_iterator(ring_iterator&& other) = default;

        ring_iterator& operator = (const ring_iterator& other);

        pointer operator-> () const;

        reference operator* () const;

        ring_iterator& operator++ ();

        ring_iterator operator++ (int);

        ring_iterator& operator-- ();

        ring_iterator operator-- (int);

        ring_iterator& operator += (difference_type diff);

        ring_iterator& operator -= (difference_type diff);

        ring_iterator operator + (difference_type diff) const;

        ring_iterator operator - (difference_type diff) const;

        difference_type operator - (const ring_iterator& other) const;

        bool operator == (const ring_iterator& other) const;

        bool operator != (const ring_iterator& other)  const;

        bool operator < (const ring_iterator& other) const;

        operator ring_iterator<const E>() const;
    };

public:

    using iterator = ring_iterator<T>;
    using const_iterator = ring_iterator<const T>;
    using reverse_iterator = std::reverse_iterator<iterator>;
    using const_reverse_iterator = std::reverse_iterator<const_iterator>;

    ring(Allocator alloc = {});

    ring(size_type cap, Allocator alloc = {});

    ring(const ring& other);

    ring(ring&& other);

    ring& operator = (const ring& other);

    ring& operator = (ring&& other);

    ~ring();

    reference front();
    reference back();

    const_reference front() const;
    const_reference back() const;

    void push_front(const value_type& val);

    void push_front(value_type&& val);

    void push_back(const value_type& val);

    void push_back(value_type&& val);

    void pop_front();

    void pop_back();

    void reserve(size_t);

    void clear();

    size_type size() const;
    
    size_type capacity() const;
    
    bool empty() const;
    
    bool full() const;

    reference operator[](size_type index);

    const_reference operator[](size_type index) const;

    reference at(size_type index);

    const_reference at(size_type index) const;

    iterator begin();
    const_iterator begin() const;
    const_iterator cbegin() const;

    iterator end();
    const_iterator end() const;
    const_iterator cend() const;

    reverse_iterator rbegin();
    const_reverse_iterator rbegin() const;
    const_reverse_iterator crbegin() const;

    reverse_iterator rend();
    const_reverse_iterator rend() const;
    const_reverse_iterator crend() const;
};

What should I do to make the code below compile?

#include <vector>
#include <ranges>

//std::vector<int> v; //compiles
ring<int> v;     //does not compile

auto range = v | std::views::transform([](int n) { return n * n; });

MSVC compiler error:

error C2678: binary '|': no operator found which takes a left-hand operand of type 'ring<int,std::allocator<int>>' (or there is no acceptable conversion)

so... that's a **lot** of unnecessary code. First of all only the declarations are needed to see if your type works with ranges. Second of all all those methods besides the "Standard ones" (like begin, end) and data members are irelevant. — bolov, Sep 06 '21 at 06:59
`ring_iterator` doesn't need to be a template - `ring::ring_iterator` is already dependent on `T`, it doesn't need to be `ring::ring_iterator` — Useless, Sep 06 '21 at 08:33
I'm referring to `using iterator = ring_iterator` - the nested type is already dependent on the outer template parameter, it doesn't need to be templated again — Useless, Sep 06 '21 at 11:48
@Useless What about `using const_iterator = ring_iterator` ? — Dmitriano, Sep 06 '21 at 14:41

bolov · Accepted Answer · 2021-09-06T09:50:45.267

So... after a lot of investigation:

Your iterator must have a public default constructor.

What should I do to make my container work with ranges?

It should satisfy the concept std::ranges::range:

static_assert(std::ranges::range<ring<int>>);

but it doesn't and the error messages are not helpful. So we look at the concept itself:

template< class T >
concept range = requires(T& t) {
  ranges::begin(t); // equality-preserving for forward iterators
  ranges::end  (t);
};

ranges::begin(v) is well defined, but ranges::end(v) gives error "error: no match for call to '(const std::ranges::__cust_access::_End) (ring&)'" with, again, no helpful error messages.

So now we look over std::ranges::end:

template< class T >
    requires /* see below */
constexpr std::sentinel_for<ranges::iterator_t<T>> auto end(T&& t);

The documentation here is a bit iffy, but the the failed requirement here is:

static_assert(
    std::sentinel_for<ring<int>::iterator, ring<int>::iterator>
);

i.e. the end iterator should be a sentinel for the begin iterator.

It is here where we reach at our first useful error message:

error: static assertion failed
   89 |     std::sentinel_for<ring<int>::iterator, ring<int>::iterator>
      |     ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
note: constraints not satisfied

[...]

opt/compiler-explorer/gcc-trunk-20210906/include/c++/12.0.0/concepts:137:30: note: the expression is_constructible_v<_Tp, _Args ...> [with _Tp = ring<int, std::allocator<int> >::ring_iterator<int>; _Args = {}] evaluated to 'false'
  137 |       = destructible<_Tp> && is_constructible_v<_Tp, _Args...>;
      |                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

So there you have it ring<int>::ring_iterator<int> must have a publicly available default constructor.

eerorika · Answer 2 · 2021-09-06T07:42:45.377

What should I do to make my container work with ranges?

You should design the container to conform to the "Range" concept.

In short, the container should to provide member functions begin and end which should return iterator and a sentinel. The end sentinel must be reachable from begin iterator. The sentinel type may be the same as the iterator. The iterator type must conform to the "Iterator" concept.

What should I do to make the code below compile?

ring_iterator in your attempt isn't default initialisable, and as such it isn't an Iterator and thus the container isn't a range. Add a default constructor to solve the problem.

What should I do to make my container work with ranges?

2 Answers2