Constrain distance function using is_iterator type trait

[jeremy-murphy]

distance will match anything, which gets annoying when your geometry algorithm for distance is... distance.
(Same goes for advance.)

Use enable_if to constrain both functions to types that satisfy the library's definition of an iterator with boost::is_iterator.

[jeremy-murphy]

Looks like this works in principle, now to add a unit test. (haha, so much for TDD...)

[Lastique]

One potential issue is that is_iterator is a heuristic and may return false for unconventional iterators that don't expose their category. Though category/traversal is needed to select distance_impl internally, so maybe that's a moot point.

[Lastique]

We should use std::enable_if since Boost.Iterator requires C++11 now.

[Lastique]

The same restriction should probably be applied to advance.

[jeremy-murphy]

One potential issue is that is_iterator is a heuristic and may return false for unconventional iterators that don't expose their category. Though category/traversal is needed to select distance_impl internally, so maybe that's a moot point.

Yeah, that's what I figured -- if it unconditionally uses iterator_traversal then we should be able to use that expectation to conditionally enable it. However, in the brief testing I did (clang/libc++), it didn't actually disable it, so there might be more to it. In other words, I'm more concerned about false positives than false negatives.

[jeremy-murphy]

We should use std::enable_if since Boost.Iterator requires C++11 now.

OK, sure thing.

[amlel-el-mahrouss]

@jeremy-murphy, hey, I started working on the unit test for #96.

[jeremy-murphy]

Yeah, so I don't get why the unit test fails with:

In file included from ./boost/iterator/distance.hpp:14:
./boost/iterator/iterator_traits.hpp:41:1: error: no type named 'difference_type' in 'std::iterator_traits<Foo>'
using iterator_difference_t = typename std::iterator_traits< Iterator >::difference_type;
^~~~~
./boost/iterator/iterator_traits.hpp:46:18: note: in instantiation of template type alias 'iterator_difference_t' requested here
using type = iterator_difference_t< Iterator >;
^
./boost/iterator/distance.hpp:47:37: note: in instantiation of template class 'boost::iterators::iterator_difference<Foo>' requested here
typename iterator_difference< SinglePassIterator >::type >::type
^
libs/iterator/test/distance_test.cpp:94:23: note: while substituting deduced template arguments into function template 'distance' [with SinglePassIterator = Foo]
auto result = distance(Foo{}, Foo{});
^

when is_iterator is meant to disable it for non-iterators, right?

[jeremy-murphy]

I'll do advance tomorrow.

[amlel-el-mahrouss]

Isn't 'cstddef' redundant here?

[Lastique]

If you're referring to the test then it is needed for std::ptrdiff_t.

[Becheler]

Ooooh so THIS is where you've been hiding @jeremy-murphy ahaha 😆 ❤️
Such a neat weed nest ! Is everything going well, and can I help a way or another ?

[megyhazy]

Hi - any thoughts as to when I can/should pull this into my development branch?

[jeremy-murphy]

Hi - any thoughts as to when I can/should pull this into my development branch?

Sorry, @megyhazy, I've been really swamped the past few weeks. I still kind of am but this is one of my highest priorities. I'm just looking over it to check if I've missed anything.

[jeremy-murphy]

Are copyright comments like this actually still the done thing?

[Becheler]

From what I've seen and talked with joaquin, yes

[jeremy-murphy]

Thanks for merging, @Lastique, glad I could make Boost.Iterator just that little bit easier to use. 😅