8

The following bit of code fails to compile. The error seems to be some kind of ambigous call to the merge routine. My understanding is that STL has a merge routine found in the std namespace, but as far as I can tell the name merge in the code below should be unique.

If I rename merge to xmerge, everything works. What could the problem be? where is the name clash coming from?

http://codepad.org/uAKciGy5

#include <iostream>
#include <iterator>
#include <vector>

template<typename InputIterator1,
         typename InputIterator2,
         typename OutputIterator>
void merge(const InputIterator1 begin1, const InputIterator1 end1,
           const InputIterator2 begin2, const InputIterator2 end2,
           OutputIterator out)
{
   InputIterator1 itr1 = begin1;
   InputIterator2 itr2 = begin2;
   while ((itr1 != end1) && (itr2 != end2))
   {
      if (*itr1 < *itr2)
         *out = *itr1, ++itr1;
      else
         *out = *itr2, ++itr2;
      ++out;
   }
   while (itr1 != end1) *out++ = *itr1++;
   while (itr2 != end2) *out++ = *itr2++;
}

int main()
{
   std::vector<int> l1;
   std::vector<int> l2;
   std::vector<int> merged_list;

   merge(l1.begin(),l1.end(),
         l2.begin(),l2.end(),
         std::back_inserter(merged_list));

   return 0;
}
Matthieu M.
  • 287,565
  • 48
  • 449
  • 722
  • What's the error message? My Compiler does not complain, it compiles the code just fine. – Oswald Feb 02 '11 at 10:14
  • Compiles fine here. (gcc 4.4.3) – Eddy Pronk Feb 02 '11 at 10:15
  • 3
    Why should i give it a different name? i'm not using std namespace. –  Feb 02 '11 at 10:16
  • 5
    But `std::merge` will be visible via ADL because you are using (e.g.) `std::back_insert_iterator`. – CB Bailey Feb 02 '11 at 10:18
  • 2
    @Charles: I think you should re-read koenig lookup semantics. I don't think you understand them correctly. –  Feb 02 '11 at 10:20
  • 2
    @Reyzooti: You could be well right, I'm not an expert on ADL. I thought that as `std::back_insert_iterator< std::vector >` was a class type in the `std` namespace `std` would be search in the lookup for `merge`. I know you haven't included `` but I don't know if it's required that a declaration for `std::merge` must not be visible. – CB Bailey Feb 02 '11 at 10:22
  • http://ideone.com/RHNJM , using gcc-4.3.4 has no issues. Are you sure you're including the full sample? – johnsyweb Feb 02 '11 at 10:47
  • 2
    @CharlesBailey: You have it right. Std::merge is allowed to be visible (any stdlib header can include another), but it is not required that it is visible without . – Fred Nurk Feb 02 '11 at 10:49
  • 4
    @Reyzooti: Remember that codepad.org has a large selection of headers, which include , included before the listed code; codepad does this because it precompiles them for speed. Additionally, codepad has an (evil) global using directive for std, though it shouldn't matter here. This is why people's local compilers (Oswald's, Eddy's comments) and ideone.com (Johnsyweb's comment) do not show the error (they don't have this precompiled prelude) while codepad does. – Fred Nurk Feb 02 '11 at 10:59
  • 4
    @Reyzooti: [Is that so?](http://codepad.org/K9ZExkBj) – Fred Nurk Feb 02 '11 at 12:16
  • 9
    @Reyzooti: I would advise you to be a little less aggressive in the assertion of your opinions. Charles and Fred are trying to help, and offer valuable advices, that you disagree with them does not entitle you to demean their contributions, whatever the "truth". – Matthieu M. Feb 02 '11 at 13:14
  • 3
    @Reyzooti: After reading some of the comments seems to me that you don't consider or know that in C++, unfortunately, while it's specified which names each header MUST declare (e.g. if you include `` you can be SURE that `std::` will be available) there is nothing that specifies that ONLY THOSE NAMES will be made visible. So even after including just `` it's still possible that an implementation will bring in the whole ``. This is unfortunate because code can compile just because of an unportable dependency of include files. And here because ADL bites you. – 6502 Feb 04 '11 at 17:50

1 Answers1

17

Compiler is getting confused between your merge function and the std::merge defined in the algorithm. Use ::merge to remove this ambiguity. This call is ambiguous as compiler is using Argument Dependendent Lookup to search for the function when unqualified function name is used.

Martin B
  • 23,670
  • 6
  • 53
  • 72
Naveen
  • 74,600
  • 47
  • 176
  • 233
  • 1
    That is what i think is happening but shouldn't that routine be in the std namespace? –  Feb 02 '11 at 10:19
  • 7
    @Reyzooti: The issue is that Argument Dependent Lookup implies that since the types passed to the function call are defined in `std`, then the `std` namespace enters the search space. Basically this is what enables you to write `std::string("Hi ")+std::string("there")` outside of the `std` namespace. The `operator+` for strings is defined inside `std`, but you surely want that code to work without having to write `std::operator+( str1, str2 )` or `str1.operator+(str2)`, depending on how the operator is defined, which I don't quite care. – David Rodríguez - dribeas Feb 02 '11 at 10:49
  • 5
    @Reyzooti: Yes, the standard library's version of `merge` is in the `std` namespace -- but ADL still causes it to be considered as a candidate because the arguments to the function are from the `std` namespace (for details of how ADL works see the Wikipedia article). – Martin B Feb 02 '11 at 10:49
  • 1
    Martin B and David Rodrigues: You're both wrong. If its an ADL issue, it would have to be in regards to the iterator translation unit std namespace. If the iterator translation unit is leaking an std::merge symbol into the anonymous namespace then the problem is with the STL definition, is it not? –  Feb 02 '11 at 12:14
  • 5
    @Reyzooti: I am almost positive that I am not wrong in this particular regard. If the standard library headers where declaring `merge` in the global namespace, it could be considered to be a problem, but the issue is that ADL will bring identifiers from the namespaces of the arguments, so even if `std::merge` is only declared inside the `std` header, it will be considered in the lookup because of the arguments. The simple case I wrote in my first comment here should demonstrate it. Else try this: `namespace X { struct test {}; void foo( test const & ){} }; int main() { X::test t; foo(t); }` – David Rodríguez - dribeas Feb 02 '11 at 12:53
  • 1
    @Reyzooti: I even typed that for you http://www.ideone.com/nby0P (note that ideone is a little less aggressive in adding headers that you have not requested and that means the `#include ` is required here) – David Rodríguez - dribeas Feb 02 '11 at 12:57