One VS2010 bug ? Allowing binding non-const reference to rvalue WITHOUT EVEN a warning?

Question

string foo() { return "hello"; }
int main() 
{
    //below should be illegal for binding a non-const (lvalue) reference to a rvalue
    string& tem  = foo();   

    //below should be the correct one as only const reference can be bind to rvalue(most important const)
    const string& constTem = foo();   
}

1. GCC is the good one to give a compile error: invalid initialization of non-const reference of type std::string& from a temporary of type std::string
2. VS2008 is not too bad as at least it gives a compile warning: warning C4239: nonstandard extension used : 'initializing' : conversion from std::string to std::string & A non-const reference may only be bound to an lvalue
3. Here comes the problematic one - VS2010(SP1) comples fine WITHOUT any error or warning, WHY ??!! I know rvalue reference in VS2010 can be used to bind with rvalue but I am NOT using &&, instead in the demo code, I was just using non-const lvalue reference !

Can somone help me explain the behavior of VS2010 here? Is it a bug !? Thanks

Answer 1

That is a known issue/feature of the VS compilers. They have always allowed that and there does not seem to be any push into removing that extension.

Answer 2

The compiler will issue an error with Disable Language Extensions turned on, and a warning at /W4. However, removing this code will break previously compiling code, and Microsoft is very reluctant to do that. This is also why they won't fix their SFINAE support.

Answer 3

Several years and many versions of Visual Studio later, we still have this "extension" causing surprises and headaches. Sigh...

The fix is to simply turn warning C4239 into an error. This will prevent MSVC from compiling code that attempts to bind a non-const lvalue reference to a temporary, and give you a nice clear compiler error. Simply add /we4239 to your compiler definitions or cl command line arguments.

In Visual Studio: Project Properties > C/C++ > All Options > Treat Specific Warnings As Errors > add 4239, and make sure to separate any other numbers with a semicolon.

In CMake:

if(MSVC)
    add_definitions("/we4239")
endif()

This seems to work far better than disabling all language extensions with /Za, which officially not recommend. On my large code base, adding /Za caused over 1500 compiler errors from Microsofts's own winnt.h header.

Answer 4

There is a much nastier variant of this problem:

class Foo {
  int _val;
public:
  Foo(int v) : _val(v) {}
  void F() { std::cout << _val << std::endl; }
};

class Bar {
  Foo& f;
public:
  Bar(Foo& f) : f(f) {}
  void F() { f.F(); }
};

int main() {
  Bar b(Foo(3));
  b.F();
}

So: to what does b.f point during the call to b.F()? The above example, compiled with VS2013 default Debug settings, runs without crashing and prints 3, but I'd suspect that any much more complex example will lead to stack corruption. If it doesn't and the compiler is doing something 'clever' to make it work, then I guess what it is really doing is this:

class Foo {
  int _val;
public:
  Foo(int v) : _val(v) {}
  void F() { std::cout << _val << std::endl; }
};

class Bar {
  Foo f;
public:
  Bar(Foo&& f) : f(f) {}
  void F() { f.F(); }
};

int main() {
  Bar b(Foo(3));
  b.F();
}