c++pass-by-const-reference

Should compiler warn about unsafe behaviour when using temporary object on const ref type parameters?


Let's consider following improper code:

#include <string>
#include <iostream>

struct C {
   C(const std::string &_s): s(_s) { }
   void run() {
      std::cout << "Here we are using the string " << s << "\n";
   }
   const std::string &s;
};

int main() {
   C a("/tmp/example.log");
   a.run();
   return 0;
}

Either g++ 6.3.0 as clang 3.8.1 compiles this code without warning, even with -W -Wall -Werror, but even for unwary programmer the behaviour simply says something is wrong here:

Here we are using the string Here we are usin

Why the results is not as one could naively expect? The lifetime of std::string object created from const char * is limited to the scope of constructor. So, when foo() constructor returns, the foo::s field has a reference to non existent object. Kaboom.

If someone understands the idea, it's easy to introduce the fix. It could be either explicit creation of std::string object and passing it as a parameter:

std::string s("/tmp/example.log");
C a(s);

The other one, which I consider a little bit safer is to change a foo::s field declaration to:

const std::string s;

making a local unmodifiable copy of the parameter, but it adds a cost of copying the object. Well, in this example copying of std::string doesn't automatically causes making a deep copy, but for own implemented class it could be completely different matter.

For C++11 there are few other methods available, as specified in this answer, but - for example - in embedded Linux environment when you are stuck to old compiler and some legacy code it's not necessarily a considerable option.

However, whatever solution would be chosen by programmer, one must know there is a problem. Even if I write my code properly, I dumbly assumed the compiler would be wise enough to detect such situations as obviously dangerous, if some other developer would fall into the trap of passing a const char * parameter. However, I decided to write some code to check the behaviour of compiler and I was disappointed - one can easily fall into the trap of having the reference to already non-existed object.

Passing a temporary object as a const reference is a recognized danger and it has been somehow addressed in C++11, but still passing the onus onto programmer to actively counteract such behaviour. So, is that a reasonable to expect a warning from the compiler, or rather one should rely on external static analysis tools?


Solution

  • Bind temporary to const reference works by design and should not issue any warnings. And it is not the source of problem in your code. Problem is that you use const reference as a member and not managing it's correctness. And even if you pass std::string as you suggested, that would not eliminate the problem - that string must outlive your object for your program to be correct. And creating member as std::string or const std::string is not little safer, but safe and proper way to do it in this situation. There are cases when you may want to store const or non const reference as a member, but you have to understand what you are doing and make sure that reference would not become dangled. Compiler unlikely to detect corner cases and issue you warning in this case.