[iterator] Code/documentation mismatch in iterator_facade
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. -Gabe [1] http://www.boost.org/doc/libs/1_53_0/libs/iterator/doc/iterator_facade.html#... ==================== #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'; } ====================
On Tue, Apr 23, 2013 at 11:00 AM, Gabriel Redner <gredner@gmail.com> 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#...
==================== #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
Thanks - I feared that that would be the case! However, as long as the docs match the code (and preferably call out this unexpected behavior) I don't think it's a major issue. Bug filed: https://svn.boost.org/trac/boost/ticket/8488 Thanks, -Gabe On Tue, Apr 23, 2013 at 2:22 PM, Jeffrey Lee Hellrung, Jr. <jeffrey.hellrung@gmail.com> wrote:
On Tue, Apr 23, 2013 at 11:00 AM, Gabriel Redner <gredner@gmail.com> 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#...
==================== #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
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
participants (2)
-
Gabriel Redner -
Jeffrey Lee Hellrung, Jr.