8

I was just building one of our projects at work and I see a new function was added:

const std::string& ClassName::MethodName() const
{
   return "";
}

The compiler gives a warning:

Warning C4172: returning address of local variable or temporary

I think the compiler is right. How safe is this function?

Note that the function doesn't return const char* which would be OK inasmuch as string literals have static storage duration. It returns a reference to const std::string

Armen Tsirunyan
  • 130,161
  • 59
  • 324
  • 434

4 Answers4

8

Yes it is not safe.
Returning address of a local variable or temporary and dereferencing it results in Undefined Behavior.

As you commented:
Yes, the lifetime of the temporary bound to a constant reference increases till the lifetime of constant. But that needs the caller to accept the return value in a const reference, So by itself the function won't be safe.

From the C++ Standard:
C++03 12.2 Temporary objects:

The second context is when a reference is bound to a temporary. The temporary to which the reference is bound or the temporary that is the complete object to a subobject of which the temporary is bound persists for the lifetime of the reference except as specified below...

A temporary bound to a reference member in a constructor’s ctor-initializer (12.6.2) persists until the constructor exits. A temporary bound to a reference parameter in a function call (5.2.2) persists until the completion of the full expression containing the call.A temporary bound to the returned value in a function return statement (6.6.3) persists until the function exits

Alok Save
  • 202,538
  • 53
  • 430
  • 533
  • I was thinking along the lines of extending the lifetime of a temporary bound to a const reference... I mean, maybe it ***is*** safe? I want to make sure before speaking to the developer that added the function – Armen Tsirunyan Aug 12 '11 at 11:21
  • @Armen Tsirunyan, it is safe until you call any function. and even if you dont call function, you can't use it, because everything that can use it is a function. and even if you find a way to use it without any function, the compiler might reorder your code and put a function in the middle. so it is not safe in any way – Daniel Aug 12 '11 at 11:25
  • @Als: I can't say I am satisfied with the answer. If the lifetime of a local temporary ***cannot*** be extended outside its scope (which I don't know if it is the case) then you're right, **but** in this case binding the return value of the function to a const reference won't help either. If it can be extended outside the function, then it's irrelevant whether I bind it or copy it... – Armen Tsirunyan Aug 12 '11 at 11:27
  • @Armen Tsirunyan, it is relevant. if you copy it, the copy happens *before* the function returns so it is still in the lifetime of the object. the lifetime of the object will always end when the method returns unless you allocate it on the heap. – Daniel Aug 12 '11 at 11:31
  • @Als: Yeah, so the lifetime isn't extended after the function regardless of whether it's bound to a const reference or not – Armen Tsirunyan Aug 12 '11 at 11:40
  • @Armen Tsirunyan: It is not safe. The reference binding cannot extend the lifetime of the temporary across a function call. The temporary is allocated in the function stack frame, and that is dropped when the function completes (by either the function or the caller, depending on the calling convention). [warning: self promotion] As to the actual binding of the reference and what it means I wrote about it in [this article](http://definedbehavior.blogspot.com/2011/08/value-semantics-copy-elision.html) Conceptually it is the same as `std::string __detail = ""; return __detail;` – David Rodríguez - dribeas Aug 12 '11 at 12:25
  • @David: Very nice article. Thank you :) – Armen Tsirunyan Aug 12 '11 at 12:27
4

This is an example that made things clear for me:

#include <iostream>
using std::cout;
struct A{
   A()   {
      cout << "Ctor\n";
   }
   ~A()   {
      cout << "Dtor\n";
   }
};

const A& f(){
   return A();
}

int main(){
   const A& ref = f();
   cout << "1\n";
   {
      const A& ref1 = A();
      cout << "2\n";
   }
   cout << "3\n";
}

Outputs

Ctor
Dtor
1
Ctor
2
Dtor
3
Armen Tsirunyan
  • 130,161
  • 59
  • 324
  • 434
3

There are circumstances when this code is safe. See GotW #88: A Candidate For the “Most Important const”.

Synxis
  • 9,236
  • 2
  • 42
  • 64
user1585719
  • 31
  • 1
3

Doing what you did actually does this internally in the compiler:

const std::string* ClassName::MethodName() const
{
   std::string temp = "";
   return &temp;
}

And returning references or pointers to local variables is bad.

Daniel
  • 30,896
  • 18
  • 85
  • 139