0

As an exercise, I'm trying to write a custom iterator to be used by std::sort. From its doc, I read:

[My iterator] must meet the requirements of ValueSwappable and
RandomAccessIterator.

Without fully implementing those1, I've come to this MCVE:

#include <iostream>
#include <algorithm>

struct mcve_random_access_iterator
{
    using size_type  = std::size_t;
    using value_type = int;

    size_type _index;
    value_type* _values;
    mcve_random_access_iterator(value_type* values) : _index(0), _values(values) {}

    mcve_random_access_iterator& operator-(const mcve_random_access_iterator& rhs) { _index-=rhs._index ; return *this; }
    value_type& operator*() { return _values[_index]; }
    friend bool operator==(mcve_random_access_iterator& lhs, mcve_random_access_iterator& rhs) { return lhs._index == rhs._index; }
    friend bool operator!=(mcve_random_access_iterator& lhs, mcve_random_access_iterator& rhs) { return !(lhs == rhs); }
};

void swap(mcve_random_access_iterator& lhs, mcve_random_access_iterator& rhs)
{
    std::swap(*lhs, *rhs);
}

struct mcve_container
{
    int _values[3];
    mcve_container() : _values{2, 3, 1} {}
    mcve_random_access_iterator begin() { return {_values}; }
    mcve_random_access_iterator end()  { auto b = begin(); b._index = sizeof(_values)/sizeof(_values[0]); return b; }
};

int main()
{
    mcve_container data;
    std::sort(data.begin(), data.end());
    for (auto n : data._values)
        std::cout << n << ", ";
    std::cout << "\n";
}

Compiling with g++ 7.2.0, I get the following error:

/usr/local/include/c++/7.2.0/bits/stl_algo.h:1969:14: error: no matching function for call to '__lg(mcve_random_access_iterator&)'
std::__lg(__last - __first) * 2,

demo from coliru

Why do I get this error and how to address it?


1) I've stripped all requirements of RandomAccessIterator from mcve_random_access_iterator which still reproduce the error.

YSC
  • 38,212
  • 9
  • 96
  • 149
  • 1
    Nice job at reducing the example. Made me realise the actual problem way more quickly. – milleniumbug Dec 20 '17 at 10:41
  • You are not *any kind of* Iterator if you don't provide *all of* `value_type`, `difference_type`, `reference`, `pointer`, `iterator_category` (either directly or by providing a specialisation of `std::iterator_traits`) – Caleth Dec 20 '17 at 10:46

2 Answers2

2

A difference between two RandomAccessIterators is a number of type std::iterator_traits<It>::difference_type (specifically, a number describing how many elements are between one iterator and the other). Yours isn't.

std::__lg is a red herring, the relevant part is that it expects a different type than your __last - __first provides.

http://en.cppreference.com/w/cpp/concept/RandomAccessIterator (see the requirement corresponding to the expression b - a)

milleniumbug
  • 15,379
  • 3
  • 47
  • 71
  • You're right, it was indeed the `operator-` missing in my complete implementation. See minimal _working_ example: http://coliru.stacked-crooked.com/a/b9243d89e2b4c34e – YSC Dec 20 '17 at 12:20
1

To complete the answer from @milleniumbug: Here is a minimal working implementation of a custom iterator suitable for std::sort.

struct random_access_iterator
{
    using size_type  = std::size_t;
    using value_type = int;
    using difference_type = std::size_t;
    using reference = int&;

    size_type _index;
    value_type* _values;


    random_access_iterator(value_type* values) : _index(0), _values(values) {}
    random_access_iterator(random_access_iterator const& other) : _index(other._index), _values(other._values) {}
    random_access_iterator& operator=(random_access_iterator const& other) { _index = other._index ; _values = other._values; return *this;}

    // Iterator
    value_type& operator*() { return _values[_index]; }
    random_access_iterator& operator++() { ++_index; return *this; }

    // InputIterator
    friend bool operator==(random_access_iterator& lhs, random_access_iterator& rhs) { return lhs._index == rhs._index; }
    friend bool operator!=(random_access_iterator& lhs, random_access_iterator& rhs) { return !(lhs == rhs); }
    value_type& operator->() { return _values[_index]; }
    random_access_iterator operator++(int) { const auto tmp = *this; ++*this; return tmp; }

    // ForwardIterator
    random_access_iterator() : _index(0), _values(nullptr) {}    

    // BidirectionalIterator
    random_access_iterator& operator--() { --_index; return *this; }
    random_access_iterator operator--(int) { const auto tmp = *this; --*this; return tmp; }

    // RandomAccessIterator
    random_access_iterator& operator+=(difference_type rhs) { _index+=rhs; return *this; }
    friend random_access_iterator operator+(random_access_iterator const& lhs, difference_type rhs) { auto tmp = lhs; tmp+=rhs; return tmp; }
    friend random_access_iterator operator+(difference_type lhs, random_access_iterator const& rhs) { return rhs + lhs; }

    random_access_iterator& operator-=(difference_type rhs) { _index-=rhs; return *this; }
    friend random_access_iterator operator-(random_access_iterator const& lhs, difference_type rhs) { auto tmp = lhs; tmp-=rhs; return tmp; }
    friend random_access_iterator operator-(difference_type lhs, random_access_iterator const& rhs) { return rhs - lhs; }
    friend difference_type operator-(random_access_iterator const& lhs, random_access_iterator const& rhs) { return lhs._index - rhs._index; }

    reference operator[](size_type i) const { return *(*this + i); }

    friend bool operator<(random_access_iterator const& lhs, random_access_iterator const& rhs) { return lhs._index < rhs._index; }
    friend bool operator<=(random_access_iterator const& lhs, random_access_iterator const& rhs) { return lhs._index <= rhs._index; }
    friend bool operator>(random_access_iterator const& lhs, random_access_iterator const& rhs) { return lhs._index > rhs._index; }
    friend bool operator>=(random_access_iterator const& lhs, random_access_iterator const& rhs) { return lhs._index >= rhs._index; }
};

namespace std
{
    template<>
    struct iterator_traits<random_access_iterator>
    {
        using size_type         = random_access_iterator::size_type;
        using value_type        = random_access_iterator::value_type;
        using difference_type   = random_access_iterator::difference_type;
        using reference         = random_access_iterator::reference;
        using iterator_category = random_access_iterator_tag;
    };
}

void swap(random_access_iterator& lhs, random_access_iterator& rhs)
{
    std::swap(*lhs, *rhs);
}

I've written a simple live demo to see this iterator in action with std::sort.

YSC
  • 38,212
  • 9
  • 96
  • 149