7

I have written a small function template that joins different containers in a new container:

#include <vector>
#include <unordered_set>
#include <string>
#include <iostream>
#include <iterator>

namespace impl
{
    template <typename OutIterator, typename Container, typename ...Containers>
    void join(OutIterator iterator, const Container& container, const Containers& ...containers)
    {        
        for (const auto& item : container)
            *iterator++ = item;

        join(iterator, containers...);  // gcc and clang cannot resolve this call
    }

    template <typename OutIterator, typename Container>
    void join(OutIterator iterator, const Container& container)
    {        
        for (const auto& item : container)
            *iterator++ = item;
    }
}

template <typename OutContainer, typename ...Containers>
OutContainer join(const Containers& ...containers)
{
    OutContainer container;
    auto it = std::inserter(container, container.end());
    impl::join(it, containers...);
    return container;
}

int main()
{
    using namespace std;
    vector<string> a = {"one"s, "two"s, "three"s};
    unordered_set<string> b = {"four"s, "five"s };
    auto res = join<unordered_set<string>>(a, b);

    for (auto& i : res)
        cout << i << "\n";

    return 0;
}

Using MSVC (cl.exe) with /std:c++17 the code compiles and works just fine. But when compiling with clang-6.0 or gcc-7.3, a compiler error is thrown. I.e. gcc says

no matching function for call to 'join(std::insert_iterator<std::unordered_set<std::__cxx11::basic_string<char> > >&)'

So obviously a function with this signature is not defined. But I don't get why it tries to call such a function. Shouldn't it be resolved like this

// in main()
join<unordered_set<string>>(a, b);

unordered_set<string> join(const vector<string>& a, const unordered_set<string>& b);
void impl::join(std::insert_iterator<unordered_set<string>> iterator, const vector<string>& a, const unordered_set<string>& b);
void impl::join(std::insert_iterator<unordered_set<string>> iterator, const unordered_set<string>& b);

Why does gcc try to instantiate join(std::insert_iterator<std::unordered_set<std::__cxx11::basic_string<char>>>&) ?

Here is an example using the compiler explorer.

Barry
  • 286,269
  • 29
  • 621
  • 977
Timo
  • 9,269
  • 2
  • 28
  • 58
  • Another nice thing about re-ordering them, is that you can avoid duplicating logic (that for loop). – StoryTeller - Unslander Monica Apr 03 '18 at 16:21
  • This is a very dangerous implementation, same problems as the good old C functions like strcat have! What, if the output iterator reaches the end iterator of the target container??? – Aconcagua Apr 03 '18 at 16:22
  • @Aconcagua - The output iterator is an inserter. I imagine these two functions being in an `impl` namespace is why they are dangerous "in general". – StoryTeller - Unslander Monica Apr 03 '18 at 16:25
  • @Aconcagua well, the functions in the `impl` namespace are intended to be used from the global scope function `join` only, which uses a insert_iterator, which means it inserts into the container every assignment. – Timo Apr 03 '18 at 16:26
  • OK... I personally still would reflect this in the template parameters. Maybe I am a little safety-fanatic? Side note: A little shorter: `auto it = std::back_inserter(container);`... – Aconcagua Apr 03 '18 at 16:41
  • @Aconcagua how would you reflect this in the template parameters? I cannot use `back_inserter` because the container can be (like shown in my example) something else than a vector (like a set), which does not have a `push_back` function. `back_inserter_iterator` and `insert_iterator` use different mechanics. – Timo Apr 03 '18 at 16:52
  • @Timo `template void join(std::insert_iterator i, Container const& c);`; `back_inserter`: sorry, oversaw necessity for more flexibility... Could have been nice in other situations, though. – Aconcagua Apr 03 '18 at 19:19

1 Answers1

12

gcc and clang are correct. MSVC still has issues with proper template name lookup (i.e. "two-phase lookup").

join, in join(iterator, containers...), is a dependent name. The candidates for finding that name are:

  • All names at the point of the template definition. This lookup just finds itself (the variadic overload), not the other overload (the 2-arg one), because it hasn't been declared yet.
  • All names that can be found by ADL on its arguments. None of those arguments have impl as an associated namespace, so that wouldn't find the other overload either.

In this situation, the fix is trivial: just reorder the two join() overloads. This ensures that the 2-arg join() will be found by that first bullet point.


Note that in C++17, you don't even need two overloads. Just one is fine:

template <typename OutIterator, typename Container, typename... Containers>
void join(OutIterator iterator, Container const& container, Container const&... containers) {
    for (auto const& item : container) {
        *iterator++ = item;
    }

    if constexpr(sizeof...(Containers) > 0) {
        join(iterator, containers...);
    }
}

Also consider using std::copy() instead of the loop. Which would actually allow:

template <typename OutIterator, typename... Containers>
void join(OutIterator iterator, Container const&... containers) {
    using std::begin;
    using std::end;
    (iterator = std::copy(begin(containers), end(containers), iterator), ...);
}
Barry
  • 286,269
  • 29
  • 621
  • 977
  • 1
    MSVC exhibits the correct behavior if you use the /permissive- flag (found on the c/c++->language node of project settings, labelled as "Conformance mode") – SoronelHaetir Apr 03 '18 at 16:34