23

I've been badly bitten by the following code, on which I wasted many hours of precious time.

#include<string>

int next(std::string param){
    return 0;
}

void foo(){
    next(std::string{ "abc" });
}

This produces the following compiler error (on Visual Studio 2013):

1>------ Build started: Project: sandbox, Configuration: Debug Win32 ------
1>  test.cpp
1>c:\program files (x86)\microsoft visual studio 12.0\vc\include\xutility(371): error C2039: 'iterator_category' : is not a member of 'std::basic_string<char,std::char_traits<char>,std::allocator<char>>'
1>          c:\users\ray\dropbox\programming\c++\sandbox\test.cpp(8) : see reference to class template instantiation 'std::iterator_traits<std::basic_string<char,std::char_traits<char>,std::allocator<char>>>' being compiled
1>c:\program files (x86)\microsoft visual studio 12.0\vc\include\xutility(371): error C2146: syntax error : missing ';' before identifier 'iterator_category'
1>c:\program files (x86)\microsoft visual studio 12.0\vc\include\xutility(371): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
1>c:\program files (x86)\microsoft visual studio 12.0\vc\include\xutility(371): error C2602: 'std::iterator_traits<std::basic_string<char,std::char_traits<char>,std::allocator<char>>>::iterator_category' is not a member of a base class of 'std::iterator_traits<std::basic_string<char,std::char_traits<char>,std::allocator<char>>>'
1>          c:\program files (x86)\microsoft visual studio 12.0\vc\include\xutility(371) : see declaration of 'std::iterator_traits<std::basic_string<char,std::char_traits<char>,std::allocator<char>>>::iterator_category'
1>c:\program files (x86)\microsoft visual studio 12.0\vc\include\xutility(371): error C2868: 'std::iterator_traits<std::basic_string<char,std::char_traits<char>,std::allocator<char>>>::iterator_category' : illegal syntax for using-declaration; expected qualified-name
========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========

I found out later that if I change my function name from next() to something else, all is fine. To me, this indicates that there is a name conflict, specifically of the name next. I find this strange because I didn't use anything like using namespace std. As far as I know, next is not a built-in C++ keyword (is it?). I looked up next here, but it's std::next and as I said I didn't using namespace std. So how did this conflict happen? How do I prevent similar things in the future? What other names might cause a conflict like this?

Rapptz
  • 20,807
  • 5
  • 72
  • 86
Ray
  • 7,833
  • 13
  • 57
  • 91
  • 1
    Due to the way ADL can work, in my workplace we tend to always explicitly specify a namespace when we mean to call a very specific free function, even when we refer to the global namespace. In your case, it would be `::next(std::string{abc});`. – Giulio Franco Apr 12 '15 at 22:29
  • I just realized that `std::next("abc")` is legal and returns `"bc"`. ADL doesn't apply to string literals so `next("abc")` would depend on `using namespace std`. – MSalters Apr 13 '15 at 09:28
  • 1
    @PiotrS. ADL took place because the call is unqualified, and it finds `std::next` because it's in an associated namespace of `std::string`. Just doing a qualified call as `::next` is likely to be more understandable to most people. – Jonathan Wakely Apr 13 '15 at 09:48
  • I just had a similar set of errors with trying to construct a vector with a list of its elements `return std::vector(a,b);`, which of course does not work (but should just give an error like there-is-no-such-consructor). – masterxilo May 25 '15 at 07:47

4 Answers4

26

There are several things going on here, interacting in subtle ways.

Firstly, an unqualified call to next with an argument of type std::string means that as well as your own next function, the standard function template std::next is found by Argument-dependent lookup (ADL).

After name lookup has found your ::next and the standard library's std::next it performs overload resolution to see which one is a better match for the arguments you called it with.

The definition of std::next looks like:

template <class ForwardIterator>
  ForwardIterator next(ForwardIterator x,
  typename std::iterator_traits<ForwardIterator>::difference_type n = 1);

This means that when the compiler performs overload resolution it substitutes the type std::string into std::iterator_traits<std::string>.

Prior to C++14 iterator_traits is not SFINAE-friendly which means that it is invalid to instantiate it with a type that is not an iterator. std::string is not an iterator, so it's invalid. The SFINAE rule does not apply here, because the error is not in the immediate context, and so using iterator_traits<T>::difference_type for any non-iterator T will produce a hard error, not a substitution failure.

Your code should work correctly in C++14, or using a different standard library implementation that already provides a SFINAE-friendly iterator_traits, such as GCC's library. I believe Microsoft will also provide a SFINAE-friendly iterator_traits for the next major release of Visual Studio.

To make your code work now you can qualify the call to next so ADL is not performed:

::next(std::string{ "abc" });

This says to call the next in the global namespace, rather than any other next that might be found by unqualified name lookup.

Community
  • 1
  • 1
Jonathan Wakely
  • 166,810
  • 27
  • 341
  • 521
  • Lwg 2408 was applied after C++14 shipped.. alhough I suppose it can be seen as a DR that 14 compilers should implement. – Cubbi Apr 14 '15 at 14:54
  • @Cubbi, that was the intent of the committee, that's why the change was made as a DR not an N-numbered paper (which is how it was originally proposed). – Jonathan Wakely Apr 14 '15 at 16:00
  • OK I will have to read up later on SFINAE. What does it stand for? I'm just surprised that the compiler looked for a `next` that is "further away" and "more obscure" than the one that right in front of its eyes. But it's probably because I don't understand SFINAE. – Ray Apr 14 '15 at 21:34
  • 2
    No, it's because of ADL. Read the answer again. The compiler tries to find _all_ the possible overloaded `next` functions, and as well as your one ADL finds the "further away" one. Then the compiler tries to decide which of all the `next` overloads it found is the best one to call in this case, because that's how overloading works. The compiler _should_ choose your one, but while trying to decide whether `std::next` should even be considered there is an error that stops compilation.. I linked to the Wikipedia page on SFINAE in my answer, which tells you what it stands for and what it means. – Jonathan Wakely Apr 14 '15 at 22:03
  • @JonathanWakely Ah I see it's much clearer now. Thank you very much! – Ray Apr 15 '15 at 18:20
8

(Updated per Jonathan's comments) There are two views here, the C++11 and the C++14 view. Back in 2013, C++11's std::next was not properly defined. It is supposed to apply to iterators, but due to what looks like an oversight it will cause hard failures when you pass it a non-iterator. I believe the intention was that SFINAE should have prevented this; std::iterator_traits<X> should cause substitution failures.

In C++14, this problem is solved. The definition of std::next hasn't changed, but it's second argument (std::iterator_traits<>) is now properly empty for non-iterators. This eliminates std::next from the overload set for non-iterators.

The relevant declaration (taken from VS2013) is

template<class _FwdIt> inline
 _FwdIt next(_FwdIt _First,
 typename iterator_traits<_FwdIt>::difference_type _Off = 1)

This function should be added to the overload set if it can be instantiated for the given arguments.

The function is found via Argument Dependent Lookup and Microsoft's header structure. They put std::next in <xutility> which is shared between <string> and <iterator>

Note: _FwdIt and _Off are part of the implementation namespace. Don't use leading underscores yourself.

MSalters
  • 173,980
  • 10
  • 155
  • 350
  • 2
    Prior to C++14 [`iterator_traits` is not SFINAE-friendly](http://cplusplus.github.io/LWG/lwg-defects.html#2408) so the error is not in the immediate context, and so it's expected (although annoying) that `iterator_traits::difference_type` for any non-iterator `T` will produce a hard error not a substitution failure. – Jonathan Wakely Apr 13 '15 at 09:32
  • Ouch, this is painful. I wouldn’t have expected MSVC to get this wrong. Did you file a bug report yet? (Edit: oops, never mind, this comment is partly made obsolete with Jonathan’s comment; I still think that a report should be filed, although they are probably already tracking it internally somehow.) – Konrad Rudolph Apr 13 '15 at 09:44
  • 1
    @KonradRudolph, they didn't get anything wrong, their library is conforming to C++11 in this respect. The problem is the definition of `iterator_traits`, and that's why we fixed it in the C++14 standard. I believe MS plan to update (or already have updated) `iterator_traits` to match the C++14 definition (which libstdc++ has done for years). – Jonathan Wakely Apr 13 '15 at 09:46
  • @Jonathan The fact remains that the `std::next` implementation should *not* lead to a conflict here. If `iterator_traits` isn’t SFINAE friendly then they’d need to take other precautions, don’t you agree? Even if the standard failed to state this unambiguously, OP’s code clearly shouldn’t break, regardless of other concerns. – Konrad Rudolph Apr 13 '15 at 09:50
  • @KonradRudolph, it's a quality of implementation issue. As written, the C++11 standard makes the OP's code ill-formed. – Jonathan Wakely Apr 13 '15 at 09:53
5

Actually std::next() is a function defined in <iterator> which returns the next iterator passed to std::next(). Your code is running on my computer with gcc-4.9.2. More : http://en.cppreference.com/w/cpp/iterator/next

The code I used:

#include<string>
#include <iostream>
int next(std::string param){
    std::cout<<param<<std::endl;
    return 0;
}

void foo(){
    next(std::string{ "abc" });
}

int main()
{
    foo();
    return 0;
}

Also on ideone : http://ideone.com/QVxbO4

0x6773
  • 1,116
  • 1
  • 14
  • 33
  • OK you're the second person to report that this is running fine on your computer. Maybe Visual Studio 2013 is implicitly `using namespace std`? – Ray Apr 12 '15 at 21:53
  • 5
    @Ray It’s almost certainly not. The fact of `std::next` being found via argument-dependent lookup does *not* necessarily mean that `using namespace std;` is in effect. It just means that the MSVC standard library implementation of `std::next` does not properly specify its arguments. – Konrad Rudolph Apr 12 '15 at 21:58
  • @mnciittbhu I understand your answer but I'm afraid you're not answering my question. You are `using namespace std`, and my question is why the compiler still thinks I'm using `std::next()` even though I'm not `using namespace std`. Your link is the same link I wrote in my question. I know about `std::next()`, as I said very clearly in my question. – Ray Apr 13 '15 at 01:34
  • @Ray even if `using namespace std;` (which doesn't happen here), the code should still be OK, as your `next` is a different overload, so there should be no ambiguity. – vsoftco Apr 13 '15 at 02:21
  • @KonradRudolph: The actual problem appears to be a SFINAE failure. The library implementation is correct (I checked), but the failing template instantiation should just have been taken out of the overload set. – MSalters Apr 13 '15 at 09:16
  • @MSalters, SFINAE doesn't apply because the error is not in the immediate context – Jonathan Wakely Apr 13 '15 at 09:44
  • @Ray I removed ```using namespace std``` and the program still runs fine. – 0x6773 Apr 13 '15 at 10:50
  • 1
    @mnciitbhu, that's because GCC's library has provided a SFINAE-friendly `iterator_traits` for many years, even though that wasn't required by the standard until C++14. – Jonathan Wakely Apr 13 '15 at 11:04
4

The compiler found standard function std::next due to the so-called Argument Dependent Lookup because the argument used in the call - std::string - is declared in namespace std.

Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
  • The question is: is this correct? The code compiles fine on clang and gcc. `std::next` should in fact only be defined for iterators. – Konrad Rudolph Apr 12 '15 at 21:51
  • But I'm not `using namespace std`. – Ray Apr 12 '15 at 21:51
  • 1
    @Ray When unqualified name is used the compiler takes into account arguments and seeks also the function name in namespace where the arguments themselves are declared. In your case you use arguments of type std::string. Type std::string is defined in namespace std and the compiler finds the corresponding function., The other question is why the compiler does bot use your function. You should provide a ready to compile example that reproduce the error. – Vlad from Moscow Apr 12 '15 at 21:54
  • @Konrad Rudolph The question why the MS VC++ compiler does not see the user defined function requeres additional investigation or at least a ready to compile example that reproduces the error. – Vlad from Moscow Apr 12 '15 at 21:56
  • 2
    @VladfromMoscow the example I gave in my question is the ready-to-compile example that produces an error in Visual Studio 2013. – Ray Apr 12 '15 at 21:59
  • @VladfromMoscow, it does see it, but `std::next` is found by ADL and added to the overload set, and performing overload resolution on `std::next` causes an error. See my answer for full details. – Jonathan Wakely Apr 13 '15 at 09:42
  • @onathan Wakely O'k. I think it is simply a bug of MS VC++ 2010. – Vlad from Moscow Apr 13 '15 at 09:56