Re: [boost] [iterator] Code/documentation mismatch in iterator_facade

Subject: Re: [boost] [iterator] Code/documentation mismatch in iterator_facade
From: Jeffrey Lee Hellrung, Jr. (jeffrey.hellrung_at_[hidden])
Date: 2013-04-23 14:22:37

On Tue, Apr 23, 2013 at 11:00 AM, Gabriel Redner <gredner_at_[hidden]> wrote:

> Hi folks,
>
> I've stumbled upon an issue with iterator_facade.  When creating a
> random access facade, one must implement the distance_to member
> function in order for std::distance to work.  However, it looks like
> the arguments passed to this method are reversed from what the
> documentation states.
>
> Specifically, the docs [1] say (paraphrased):  a.distance_to(b) is
> equivalent to distance(a, b).
>
> However, when I call std::distance(a, b), I find that b.distance_to(a)
> is invoked in the facade - the mirror image of what the docs say!
> This is not a problem if just wrapping some other random access
> iterator, but in my case the calculation of distance is nontrivial and
> this issue led to a rather tiresome debugging session and the solution
> (negating before returning from distance_to, knowing that
> iterator_facade will re-negate my response) is annoying to explain.
>
> Should I file a bug?
>
> A test case is appended to this message.
>

This looks like it could potentially be a bug. File a trac ticket. I'll
have to look at the documentation a bit closer and also peruse the tests,
but at a first glance it doesn't look good :/

If it is a bug, I'm afraid the only viable thing to do is fix the
documentation to match the code (rather than fix the code to match the
documentation).

- Jeff

-Gabe
>
> [1]
> http://www.boost.org/doc/libs/1_53_0/libs/iterator/doc/iterator_facade.html#iterator-facade-requirements
>
> ====================
> #include <boost/iterator/iterator_facade.hpp>
>
> #include <vector>
> #include <iterator>
> #include <iostream>
>
> class vector_iterator
>   : public boost::iterator_facade<
>       vector_iterator,
>       int,
>       boost::random_access_traversal_tag>
> {
> public:
>
>   vector_iterator(char tag, std::vector<int>::iterator it) :
> _tag(tag), _it(it) {}
>
> private:
>   friend class boost::iterator_core_access;
>
>   int& dereference() const { return *_it; }
>   bool equal(vector_iterator const& other) const { return _it ==
> other._it; }
>   void increment() { ++_it; }
>   void decrement() { --_it; }
>   void advance(int n) { std::advance(_it, n); }
>
>   std::vector<int>::iterator::difference_type
>   distance_to(const vector_iterator& other) const
>   {
>     std::cerr << _tag << ".distance_to(" << other._tag << ")\n";
>     std::vector<int>::iterator::difference_type result =
> std::distance(_it, other._it);
>     std::cerr << "result = " << result << '\n';
>     return result;
>   }
>
> private:
>   const char _tag;
>   std::vector<int>::iterator _it;
> };
>
> int main()
> {
>   std::vector<int> v;
>   v.push_back(1);
>   v.push_back(2);
>
>   vector_iterator a('a', v.begin());
>   vector_iterator b('b', v.end());
>
>   std::cerr << std::distance(a, b) << '\n';
> }
> ====================
>
> _______________________________________________
> Unsubscribe & other changes:
> http://lists.boost.org/mailman/listinfo.cgi/boost
>