Is there a C++ warning for returning a reference into a temporary?

Question

There's an error for this case:

const int& foo() {
    const int x = 0;
    return x;
}

and even

const int& foo() {
    const std::pair<int,int> x = {0,0};
    return x.first;
}

but not this:

const int& foo() {
    const std::array<int,1> x = {0};
    return x[0];
}

and (less surprisingly) not this:

const int& foo() {
    const std::vector<int> x = {0};
    return x[0];
}

Particularly in the std::vector case, I get that this warning would be pretty tricky, since it's not obvious to the compiler that the const int& returned by std::vector<int>::operator[](size_t) const is a reference into the temporary. I'm actually a little surprised that std::array doesn't fail, though, since this similar case does give me an error:

struct X {
    int x[0];
};

const int& foo() {
    X x;
    return x.x[0];
}

Do any of the popular compilers have a warning/error that can catch these cases? I could imagine a conservative version that would warn about returning a reference that came from a member-function call on a temporary.

I tripped over this with something like the following, in which I inlined a chained series of calls, but because C++ lets you assign locals to const&, the verbose version works while the superficially-identical version deletes the temporary right away, leaving a dangling reference:

#include <iostream>

struct A {
    int x = 1234;
    A() { std::cout << "A::A " << this << std::endl; }
    ~A() { x = -1; std::cout << "A::~A " << this << std::endl; }
    const int& get() const { return x; }
};

struct C { 
    C() { std::cout << "C::C " << this << std::endl; }
    ~C() { std::cout << "C::~C " << this << std::endl; }
    A a() { return A(); }
};

int foo() {
    C c;
    const auto& a = c.a();
    const auto& x = a.get();
    std::cout << "c.a(); a.get() returning at " << &x << std::endl;
    return x;
}

int bar() {
    C c;
    const int& x = c.a().get();
    std::cout << "c.a().get() returning at " << &x << std::endl;
    return x;
}

int main() {
    std::cout << foo() << std::endl;
    std::cout << bar() << std::endl;
}

That outputs

C::C 0x7ffeeef2cb68
A::A 0x7ffeeef2cb58
c.a(); a.get() returning at 0x7ffeeef2cb58
A::~A 0x7ffeeef2cb58
C::~C 0x7ffeeef2cb68
1234
C::C 0x7ffeeef2cb68
A::A 0x7ffeeef2cb58
A::~A 0x7ffeeef2cb58
c.a().get() returning at 0x7ffeeef2cb58
C::~C 0x7ffeeef2cb68
-1

Depends on your compiler but yes most do have such a warnings, e.g. [MSVC has C4172](https://msdn.microsoft.com/en-us/library/d0ws2xs1.aspx) — Cory Kramer, May 24 '18 at 19:00
FWIW a easy rule to use to prevent problems is always return by value unless you are returning a static object or class member variable. — NathanOliver, May 24 '18 at 19:03
Does C4172 actually catch this? The example on that page looks like the examples that GCC & clang both see. — Ben, May 24 '18 at 19:03
Remember to turn on the optimiser most compilers only detect threse sort of errors when optimising. live example: https://godbolt.org/g/96DDu8 Remove `-O3` and the error is not detected. — Richard Critten, May 24 '18 at 19:04
@RichardCritten, that's fascinating. It doesn't catch the `std::vector` version, but sure enough, `-O3` changes the static error checking. That kinda blows my mind. — Ben, May 24 '18 at 19:06
Even better look at the code generated in the `-O3` case it doesn't even attempt to return the address as it would be Undefined Behaviour (it's a reference) and as UB can't happen in a well formed program ... — Richard Critten, May 24 '18 at 19:10
@NathanOliver OP returns reference to a class member variable indeed — Slava, May 24 '18 at 19:22
@Richard Critten UB can happen in a well formed program. well-formed program is a C++ program constructed according to the syntax rules, diagnosable semantic rules, and the one-definition rule. Something undefined doesn't get covered by that definition. Well-formed program is not synonym of correctly working one. Anyway, it's a UB. Anything can happen. — Swift - Friday Pie, May 24 '18 at 19:25
@Swift-FridayPie I don't think that is right. Sure, well formed code doesn't have to work as you expect it to (logic errors for instance) that doesn't the behavior is undefined, it's just you don't understand what you have defined. Undefined behavior is code that can do different things every time it is run for the same input. — NathanOliver, May 24 '18 at 19:29
@NathanOliver well, I mostly picked up definition of ISO. Also, ISO got distinction between _undefined_ and _unspecified_ behavior. _Undefined behavior_ is one that isn't affected by standard. _Undetermined_ is one that is defined by implementation. UB in this case is undefined, because result might be affected by final code, structure of program, level of optimization and random factors (state of uninitialized memory that got accessed, for example).Neither is listed as reason of causing program to be _ill-formed_. — Swift - Friday Pie, May 24 '18 at 19:35
@NathanOliver Swift is correct. `int main() { const int n=0; *(int*)&n = 1; }` is a well-formed program, but every execution of that program has undefined behavior. — aschepler, May 24 '18 at 23:39
@aschepler That is not well formed. You cast away `const`and write to it which is UB per the standard. — NathanOliver, May 24 '18 at 23:41
@NathanOliver The point here is that there is no direct relationship between the definitions of UB and well-formed. — aschepler, May 24 '18 at 23:44

Slava · Answer 1 · 2018-05-24T23:28:35.947

the verbose version works while the superficially-identical version deletes the temporary right away, leaving a dangling reference

Your code is not identical at all. At the first case:

const auto& a = c.a();
const auto& x = a.get();

lifetime of temporary extended for lifetime of const reference, so x is valid as long as a is valid, but in the second:

const int& x = c.a().get();

you have dangling reference x. And the case you have here is unrelated to examples you shown before - when you return a dangling reference to a local variable hence warnings you are looking in examples almost unrelated if a compiler would detect situation you described in real code.

Solution for your case though can be made by designer of the class A:

struct A {
    int x = 1234;
    A() { std::cout << "A::A " << this << std::endl; }
    ~A() { x = -1; std::cout << "A::~A " << this << std::endl; }
    const int& get() const & { return x; }
    int get() && { return x; } // prevent dangling reference or delete it to prevent compilation
};

I know they aren’t the same, I was just showing how I slipped into this error mode that could probably statically detected. Can you explain the `&&` suffix? I’m amazed I haven’t seen it. — Ben, May 24 '18 at 21:19
It is unclear what you are showing, as your examples are unrelated to the real case. In your examples you return a const reference to local variable which is easy to detect either by developer or by compiler. Real case is more complicated by both. For ref-qualifiers read this https://akrzemi1.wordpress.com/2014/06/02/ref-qualifiers/ — Slava, May 24 '18 at 21:24

score 1 · Answer 2 · answered May 24 '18 at 20:26

Using a value whose life has ended is undefined behavior, see [basic.life]/6.1. The standard does not require the compiler to output any diagnostics for UB.

So the diagnostics you're seeing are just a courtesy of the compiler. It's nice to see you're getting some of those, but they are certainly far from watertight as you have noticed.

And yes, lifetime extension isn't chainable. That makes it very dangerous and unreliable.

You can try Clang's Address Sanitizer (ASAN).

In fact ASAN seems to be catching your issue (-fsanitize-address-use-after-scope):

==35463==ERROR: AddressSanitizer: stack-use-after-scope on address 0x7fffffffe970 at pc 0x000000498d53 bp 0x7fffffffe910 sp 0x7fffffffe908
READ of size 4 at 0x7fffffffe970 thread T0

Is there a C++ warning for returning a reference into a temporary?

2 Answers2