9
std::string &func(int vlu)
{
    std::string str;
    str = std::to_string(vlu) + "something";

    return str;
}

the function above is unsafe clearly.
Following is another version.

std::string &func(int vlu)
{
    return std::to_string(vlu) + "something";
}  

I have some questions:
the compiler(gcc), in the second version, doesn't give me any warning. Is it safe? I just think that compiler(or something?) will create a temporary variable to hold the return of expression std::to_string(vlu) + "something". So the second version is unsafe too. and I right?

schorsch312
  • 5,553
  • 5
  • 28
  • 57
umbreLLaJYL
  • 185
  • 1
  • 10
  • 1
    clang and gcc(8.2) do emit warnings: `prog.cc:5:12: error: non-const lvalue reference to type 'basic_string<...>' cannot bind to a temporary of type 'basic_string<...>' return std::to_string(vlu) + "something"; ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1 error generated.` – hellow Oct 29 '18 at 10:54
  • 4
    Yes, the second version is unsafe also. A compiler is not obligated to warn you about undefined behavior. If you get a warning, consider it to be no more than an unexpected surprise that your compiler detected your own bug for you. – Sam Varshavchik Oct 29 '18 at 10:54
  • The 2nd version is definitely unsafe. I think it should be a gcc bug for not emitting warning, or maybe, it involves RVO and thus affecting the analysis – Kingsley Chen Oct 29 '18 at 10:55
  • 2
    Possible duplicate of [C++ Returning reference to local variable](https://stackoverflow.com/questions/4643713/c-returning-reference-to-local-variable) – Lanting Oct 29 '18 at 10:56
  • its not just "unsafe". unsafe sounds as if there is a chance that it could to something wrong, but in fact there is no chance that the function could do something right... – 463035818_is_not_an_ai Oct 29 '18 at 11:00
  • Yes, both examples are unsafe, in the sense that the caller will exhibit undefined behaviour if it uses the returned reference as an object (e.g. calling non-static member functions). However, the code in both functions does not contain any diagnosable error (in the sense that the *standard* requires a diagnostic). If your compiler gives, or can be configured to give, a warning or error for either example, count your lucky stars. Such behaviour by a compiler is a quality-of-implementation matter, not a requirement of the C++ standard. – Peter Oct 29 '18 at 11:04
  • You should generally not use references to std:string, because they are implemented as a header pointing to the data, with only the header being copied. This from memory from the TC++PL. Anyone have an up to date reference for that? – hkBst Oct 29 '18 at 11:09
  • @user463035818 - there is actually a possibility that the function will "do something right". The functions are both syntactically correct, and the behaviour is undefined if the caller uses the returned reference. However, undefined behaviour simply means the standard doesn't limit what can happen as a result, it doesn't mean that "wrong" behaviour is guaranteed. The nastiest cases of undefined behaviour occur with code that *seems* to work correctly, only to malfunction at some later time (such as following a compiler update, change of optimisation settings). – Peter Oct 29 '18 at 11:10
  • @Peter The function itself has no undefined behaviour. It just returns an invalid reference (always), I would call this broken rather than unsafe ;) – 463035818_is_not_an_ai Oct 29 '18 at 11:13
  • @hkBst that sounds strange. no references to `std::string` ?? huh? – 463035818_is_not_an_ai Oct 29 '18 at 11:15
  • @user463035818 - You'll note that my previous comments said nothing about the functions having undefined behaviour - I referred only to the caller. It is unsafe if the caller uses that reference - because there is no way the caller can avoid undefined behaviour, no consistent way in standard C++ to even detect the presence of that undefined behaviour, and no predictable or reliable outcome. A certainty of an unknown outcome is generally considered a high risk, or a lack of safety. – Peter Oct 29 '18 at 11:43
  • Both are bad - but more to the point, why would you want to do that? – Konchog Oct 29 '18 at 12:00
  • @Peter I did note it, but the comment was adressed at me and I was also not refering to undefined behviour. Lets make a compromise and say it is unsafe to use the function because it is broken – 463035818_is_not_an_ai Oct 29 '18 at 12:17
  • really there're no reasons for all this. for a mutable string from ended scope you didn't make any attempts to deploy it right. It could be done with smart pointers or with `new` but all stuff you'd posted looks like a bad plan part. – Алексей Неудачин Oct 29 '18 at 12:40

1 Answers1

13

No, neither program is safe. In both cases the returned reference is dangling and the behaviour of using the reference will be undefined.

The second program is also ill-formed, because it attempts to bind an lvalue reference (that is returned) to an rvalue (the temporary). The second program might be considered "less unsafe", since a compiler might choose to not compile it.

To fix the function: Don't attempt to return a reference, when the purpose is to return a new object. Return an object instead.

eerorika
  • 232,697
  • 12
  • 197
  • 326