ADL not working as expected

Question

struct S
{
    vector<int> v;
    void method()
    {
        begin(v);
    }
};

The above code snippet compiles fine, because of ADL until I add

auto begin() { return begin(v); }

to the class declaration. At that point C++ forgets about ADL and instead prefers S::begin that doesn't even have a viable overload, producing the error

error: no matching function for call to ‘S::begin(std::vector<int>&)’ begin(v);

Is there any fix to this? I am asking, because after reading Why use non-member begin and end functions in C++11?, I started using begin() and end() free functions everywhere for consistency, but now I am getting conflicts after defining my own begin() and end() methods.

It's because your function *hides* the `std::begin` function. — Some programmer dude, Aug 14 '17 at 13:29
@VTT I'd rather change my existing code to `container.begin()` and `std::begin`, where that is not possible. — Post Self, Aug 14 '17 at 13:30
Since you're apparently `using namespace std;`, the first snippet does not involve ADL. If you didn't, it would. — molbdnilo, Aug 14 '17 at 13:57

score 4 · Answer 1 · answered Aug 14 '17 at 13:37

As mentioned in comments S::begin hides std::begin. You can bring std::begin into S's scope by typing using std::begin or explicitly call std::begin.

struct S
{
    std::vector<int> v;
    void method()
    {
        using std::begin;
        begin(v);
    }

    auto begin() {  using std::begin; return begin(v); }
};

Useless · Accepted Answer · 2017-08-14T14:57:50.503

You're using begin as an unqualified name, and unqualified name lookup

... examines the scopes as described below, until it finds at least one declaration of any kind, at which time the lookup stops and no further scopes are examined.

reference.

From your member function's perspective, the first scope providing a name begin is the class scope, so it populates the overload set from that scope, and stops looking.

Only after this name lookup stage, does it try to choose one of the overload set, and decide nothing matches. The compiler doesn't then go back and start searching from the next scope, it just gives up.

Your options are:

use the existing container member function instead of the free function (this is slightly less verbose than the explicitly-qualified version below)
```
auto begin() { return v.begin(); }
```

use qualified names instead

auto begin() { return ::std::begin(v); }

add the correct overload to class scope, with using std::begin;

Ignore that one, I forgot you can't introduce non-member names at class scope with using.
inject the correct overload into the narrower scope of the member function body itself, so the search stops there
```
auto begin() { using std::begin; return begin(v); }
```
stop providing a begin member function in the first place, and instead add a non-member begin overload to the enclosing namespace. This is more modern and avoids the lookup problem.
```
namespace N {
  struct S { std::vector<int> v; };
  std::vector<int>::iterator begin(S& s) { return s.v.begin(); }
  // and end, cbegin, etc. etc.
}
```

That's not class scope, that's file scope. However, you're right, I completely forgot you can't `using` non-members into a class scope! — Useless, Aug 14 '17 at 14:40
How would I do 4.? I can define it as a friend method, but a function usually won't have access to the container inside the class — Post Self, Aug 14 '17 at 14:45
Currently your container is public anyway. If not, you can always provide a reference-to-container method. — Useless, Aug 14 '17 at 14:47
Okay. This is semanticly getting quite bulky, but I guess that's what I have to do. — Post Self, Aug 14 '17 at 14:52
I remembered a shorter alternative, and added it at the beginning. — Useless, Aug 14 '17 at 14:58

score 0 · Answer 3 · answered Apr 16 '23 at 01:11

Useless's answer is very complete; but just to simplify it a bit... There are exactly two things you could have meant with your original code. Either you meant to invoke ADL on begin, or you didn't.

If you don't mean to invoke ADL, then you should use a qualified name. You wouldn't write sort(v.begin(), v.end()) when you meant std::sort, so you shouldn't write begin(v) when you mean std::begin(v), either. In fact, you shouldn't write std::begin(v) when you mean v.begin()!

template<class T>
auto test(const std::vector<T>& v) {
    return std::begin(v);  // Right (no ADL)
}

template<class T>
auto test(const std::vector<T>& v) {
    return v.begin();  // Even better (no ADL)
}

If you do mean to invoke ADL, then you must enable ADL by adding a using-declaration. Typically you use ADL when you're writing generic code (templates), and you want the author of T to have some control over what happens in your templatey code. In your actual code snippet, v is just a std::vector<int> with no templatey bits; but for the sake of argument let's pretend that its type is some unknown container type T (perhaps even a native array type, like int[10]). Then we should do the std::swap two-step:

template<class T>
auto test(const T& v) {
    using std::begin;
    return begin(v);  // Right (ADL)
}

These ("qualified" and "ADL two-step") are the only two ways of making non-member function calls in C++ that you need to know; they are designed to work 100% of the time. Your original snippet deviated from the best practice by trying to use ADL without the two-step; that's why it sometimes failed for you.

Note that in C++20 you can (if you like) make a qualified call to std::ranges::begin, which will itself do the ADL two-step for you:

template<class T>
auto test(const T& v) {
    return std::ranges::begin(v);  // Right (ADL happens internally)
}

ADL not working as expected

3 Answers3