call to 'advance' is ambiguous about iterator HOT 16 CLOSED

I don't think the test case is correct. You can't expect unqualified advance to resolve to std::advance because the type of the iterator is unspecified (i.e. its type is not required to be in namespace std).

I think the right thing to do is to fix user's code (or range-v3) so that it doesn't call unqualified advance.

from iterator.

I think the right thing to do is to fix user's code (or range-v3)

I'll note here that the test case has no range-v3 code in it. The right thing to do would be for Boost to not add an overload of advance that creates an ambiguity with the one in namespace std, and locate it such that ADL is likely to find it. One possible solution would be to turn boost::advance into a customization point object.

from iterator.

The right thing to do would be for Boost to not add an overload of advance that creates an ambiguity with the one in namespace std, and locate it such that ADL is likely to find it.

advance in Boost.Iterator is already protected from inadvertent ADL. It's defined in boost::iterators::advance_adl_barrier. boost::advance and boost::iterators::advance are the names for direct public use.

from iterator.

I don't think you're really understanding the issue. As @wickedmic pointed out above, the version of advance in the ADL-protecting namespace is brought into the boost:: namespace with a using declaration (which rather defeats the purpose of the ADL-protecting namespace) here:

So an unqualified call to advance will find both the versions in boost:: and in std:: if the iterator type has boost and std as associated namespaces. That is not an uncommon scenario, I would imagine, especially for users of Boost.Iterator.

The way to make this work is to turn boost::advance into a customization point object.

from iterator.

Sorry, I was thinking that ADL does not consider using-declarations, but apparently it only doesn't consider using-directives.

Making boost::advance a customization point object would solve the ambiguity but I'm not sure making it a customization point is what we really want. I don't think we want to dispatch between different implementations of advance. Perhaps just converting that using-declaration to a using-directive would suffice.

from iterator.

IIRC, Boost.Iterator has never advertised boost::advance/distance.
The documentation only says boost::iterators::advance/distance.
So it might be OK to remove using iterators::advance/distance;
(and submit PR's to users of boost::advance/distance).
But this makes a discrepancy between Boost.Iterator and Boost.Range;
Boost.Range has boost::distance, but Boost.Iterator doesn't have it.

Perhaps just converting that using-declaration to a using-directive would suffice.

+1 and #44.

from iterator.

Thanks Michael and Eric for reporting and pointing out the problem.
Also, thanks Andrey for merging the PR (and sorry for forgetting rebase & squash).

from iterator.

I had to revert the change as it broke building Boost.

from iterator.

It seems that Boost.Range's boost::distance(rng) hides Boost.Iterator's boost::distance(it1, it2), if we use using-directives. So, boost::distance(it1, it2) cannot be found when <boost/range/distance.hpp> is included.

This sample code fails to compile:

namespace NS1
{
    namespace NS2
    {
        template <typename T, typename U>
        void f(T, U) {}
    }
    
    using namespace NS2; // Error
    // using NS2::f; // OK (but using-declarations have ADL issue)
    
    template <typename T>
    void f(T) {}
}

int main(int argc, char* argv[])
{
    NS1::f(1, 2);
    
    return 0;
}

from iterator.

Iterator's boost::advance should be enable_if-ed on the category being one of Iterator's. (An object won't work - can't overload an object with Range's function.)

from iterator.

Looking at

from iterator.

... and it seems that we'll also have to using std::advance; in boost:: to support boost::advance for normal iterators.

Tricky.

from iterator.

A possible fix:
Define Boost.Range's distance(rng) in ADL barrier namespace (currently, it is defined in boost namespace) and pull the name by a using-directive. Note that documentation of Boost.Range explicitly forbids unqualified call to distance(rng). So this modification to Boost.Range should be OK.

This code compiles fine:

namespace NS1
{
    namespace NS2
    {
        template <typename T, typename U>
        void f(T, U) {}
    }
    using namespace NS2;
    
    namespace NS3
    {
        template <typename T>
        void f(T) {}
    }
    using namespace NS3;
}

int main(int argc, char* argv[])
{
    NS1::f(1, 2);
    NS1::f(1);
    
    return 0;
}

from iterator.

Regarding constraints on the first template parameter of advance: if ADL issues get resolved, I think "no constraints" would be OK (unless other libraries define boost::advance with two unconstrained template parameters).

from iterator.

Yes, I agree.

from iterator.

Nice. Thanks guys.

from iterator.