CODEWITHSUNDEEP
c++referencetemporary
user2304458
user2304458

Reputation: 387

How to create a temporary variable in C++

I have a function returning a reference to an instance of my class "record".

record& get_record(int key) {
    return lookup(key);
}

That is effective it returns a reference and not by value. Now I modify it a bit.

record& get_record(int key) {
    if (valid(key))
        return lookup(key);
    else {
        record x;
        x.valid=false;
        return x; //Here I really want to return a temporary variable
                  // and not a reference to a local variable.      
    }
}

Is it correct that returning a reference to a local variable is not a good idea? and how do I return x in such a way that it becomes a temporary variable?

Upvotes: 4

Views: 12751

Answers (5)

YSC
YSC

Reputation: 40160

This is worse than a bad idea, it is undefined behavior and result in most of the cases to a crash. This is bad (TM).

What you could do is changing the return type of get_record so it returns a smart pointer. If key is valid, it returns an observer pointer to it. Otherwise, it returns an owning smart pointer to a freshly created object:

#include <memory>
#include <iostream>

struct record { int n; } some_record{42};

std::shared_ptr<record> get_record(bool b)
{
    if (b == true) {
        return std::shared_ptr<record>{&some_record, [](record*){}}; // see explanation ^1
    }
    return std::shared_ptr<record>{new record{0}};
}

int main()
{
    std::cout << get_record(true)->n << "\n";  // this is some_record
    std::cout << get_record(false)->n << "\n"; // this is a temporary
}

1) About [](record*){}: this no-op lambda given as the second argument to std::shared_ptr::shared_ptr() is invoked when the smart pointer is destroyed. It replaces the default deleter of std::shared_ptr whose behavior is to call delete on the owned pointer.

About why your design is flawed. In fact, making get_record return a reference makes it not consistent. What you want is:

  • if key is valid return a reference to an existing/permanant object, and
  • return a temporary object otherwise.

Those two are mutually exclusive, and your function doesn't make sense: what does get_record return semantically?

Upvotes: 7

Mark Ingram
Mark Ingram

Reputation: 73713

Returning a reference in itself doesn't produce undefined behaviour, but if you attempt to modify it, then you will.

Accessing an object outside of its lifetime is undefined behavior.

int* foo(void) {
    int a = 17; // a has automatic storage duration
    return &a;
}  // lifetime of a ends
int main(void) {
    int* p = foo(); // p points to an object past lifetime ("dangling pointer")
    int n = *p; // undefined behavior
}

http://en.cppreference.com/w/c/language/lifetime

If you have access to C++17, you could implement it using std::optional. Note the use of std::reference_wrapper, because use of a reference in std::optional makes your program ill-formed.

std::optional<std::reference_wrapper<record>> get_record(int key) {
    if (valid(key))
        return std::optional<std::reference_wrapper<record>>(lookup(key));
    else
        return std::nullopt;
}

Without C++17, you could just return a pointer to your record:

record* get_record(int key) {
    if (valid(key))
        return &lookup(key);
    else
        return nullptr;
}

Or if you prefer, you can keep the reference return type, and throw an exception to indicate a missing record. Though this is my least preferred approach as it makes it easy to call get_record without wrapping in a try / catch.

record& get_record(int key) {
    if (valid(key))
        return &lookup(key);
    else
        throw std::out_of_range("Invalid record key!");
}

Upvotes: 0

Guido Niewerth
Guido Niewerth

Reputation: 156

If you are allowed to modify the get_record function you can change the return type to pointer to record instead of reference to record.

record* get_record( int key )
{
    if( valid( key ) ) return &lookup( key ); 
    else               return nullptr;
 }

However, this approach needs two guarantees:

  • the lookup function must return a reference to record
  • the record returned by lookup must still live when lookup returns (e.g. record is an element of some sort of container and lookup returns its reference)

Upvotes: 1

Lanting
Lanting

Reputation: 3068

As others already detail why returning a reference to a local is bad, ill just provide an alternative: Exceptions. Though you could write a custom exception, arguably an std::out_of_range exception would seem in place (as the key lies outside of the range of valid keys, which is exactly what std::map::at does).

Have a look at: How to throw a C++ exception

record& get_record(int key)
{
  if (valid(key))
  {
    return lookup(key);
  } else {
    throw std::out_of_range("The provided key is invalid");
  }
}

You will now, obviously, have to catch the exception in the code calling get_record, otherwise your program will still terminate.

Upvotes: 0

songyuanyao
songyuanyao

Reputation: 173044

Is it correct that returning a reference to a local variable is not a good idea?

Yes. The local object will be destroyed when get out of the function so the returned reference is always dangled.

You might make x a static variable.

record& get_record(int key) {
    if (valid(key))
        return lookup(key);
    else {
        static record x;
        x.valid=false;
        return x;
    }
}

Note that the returned reference will always refer to the same object, i.e. x.

Upvotes: 7

Related Questions