26

Is it ok to return default argument's value by const reference like in the examples below:

https://coliru.stacked-crooked.com/a/ff76e060a007723b

#include <string>

const std::string& foo(const std::string& s = std::string(""))
{
    return s;
}

int main()
{
    const std::string& s1 = foo();
    std::string s2 = foo();

    const std::string& s3 = foo("s");
    std::string s4 = foo("s");
}
curiousguy
  • 8,038
  • 2
  • 40
  • 58
FrozenHeart
  • 19,844
  • 33
  • 126
  • 242
  • 3
    Simple test: replace `std::string` with a class of your own so you can track construction and destruction. – user4581301 Nov 01 '19 at 17:45
  • 1
    @user4581301 If the sequence is correct it wouldn't prove that the construct is ok though. – Peter - Reinstate Monica Nov 01 '19 at 17:46
  • 6
    @user4581301 "It appeared to work when I tried it" is the absolute worst thing about undefined behavior – HerrJoebob Nov 01 '19 at 17:52
  • It should be noted that the question is a tidbit misleading in its wording. You're not returning a default argument's value by const reference, but you are returning a const reference to a const reference (... to a default argument). – Damon Nov 01 '19 at 18:02
  • 2
    @HerrJoebob Agree with the statement 100%, but not the context you're using it in. The way I read it this question resolves to "When is object's lifetime over?" figuring out when the destructor is called is a good way to do that. For an Automatic variable the destructor should be called right on time or you've got big problems. – user4581301 Nov 01 '19 at 18:06
  • @user4581301 There's your problem, it's not an automatic variable - it's a *temporary* and its lifetime is not defined enough to be subject to 'try and see what happens' testing. In any case 'try and see' shouldn't be encouraged. – HerrJoebob Nov 03 '19 at 04:03
  • @HerrJoebob: it is defined. Check Brian's answer – Andriy Tylychko Nov 04 '19 at 23:43

3 Answers3

18

In your code, both s1 and s3 are dangling references. s2 and s4 are ok.

In the first call, the temporary empty std::string object created from the default argument will be created in the context of the expression containing the call. Therefore, it will die at the end of the definition of s1, which leaves s1 dangling.

In the second call, the temporary std::string object is used to initialize s2, then it dies.

In the third call, the string literal "s" is used to create a temporary std::string object and that also dies at the end of the definition of s3, leaving s3 dangling.

In the fourth call, the temporary std::string object with value "s" is used to initialize s4 and then it dies.

See C++17 [class.temporary]/6.1

A temporary object bound to a reference parameter in a function call (8.2.2) persists until the completion of the full-expression containing the call.

Brian Bi
  • 111,498
  • 10
  • 176
  • 312
  • 1
    The interesting part of the answer is the assertion that the default argument will be created in the caller's context. That seems to be supported by Guillaume's standard quote. – Peter - Reinstate Monica Nov 01 '19 at 17:56
  • 2
    @Peter-ReinstateMonica See [expr.call]/4, "... The initialization and destruction of each parameter occurs within the context of the calling function. ..." – Brian Bi Nov 01 '19 at 17:59
8

It is not safe:

In general, the lifetime of a temporary cannot be further extended by "passing it on": a second reference, initialized from the reference to which the temporary was bound, does not affect its lifetime.

Oblivion
  • 7,176
  • 2
  • 14
  • 33
  • So do you think `std::string s2 = foo();` is valid (after all, no reference is passed on explicitly)? – Peter - Reinstate Monica Nov 01 '19 at 17:51
  • 1
    @Peter-ReinstateMonica that one is safe as new object will be constructed. My answer is merely about lifetime expansion. The other two answers already covered everything. I wouldn't repeat in mine. – Oblivion Nov 01 '19 at 18:15
5

It depends on what you do with the string afterwards.

If your question is is my code correct? then yes is it.

From [dcl.fct.default]/2

[ Example: The declaration

void point(int = 3, int = 4);

declares a function that can be called with zero, one, or two arguments of type int. It can be called in any of these ways:

point(1,2);  point(1);  point();

The last two calls are equivalent to point(1,4) and point(3,4), respectively. — end example]

So your code is effectively equivalent to:

const std::string& s1 = foo(std::string(""));
std::string s2 = foo(std::string(""));

All your code is correct, but there is no reference lifetime extension in any of these cases, since the return type is a reference.

Since you call a function with a temporary, the lifetime of the returned string won't extend the statement.

const std::string& s1 = foo(std::string("")); // okay

s1; // not okay, s1 is dead. s1 is the temporary.

Your example with s2 is okay since you copy (or move) from the temporary before the end of the satement. s3 has the same problem than s1.

Guillaume Racicot
  • 39,621
  • 9
  • 77
  • 141