Avoid returning by-reference argument

Question

Suppose I have a class Option:

template<typename T>
class Option {
public:
    Option() noexcept
    {}

    Option(T val) noexcept : val_(std::make_shared<T>(std::move(val)))
    {}

    const T & get() const
    {
        if (val_ == nullptr) {
            throw std::out_of_range("get on empty Option");
        }
        return *val_;
    }

    const T & getOrElse(const T &x) const
    {
        return val_ == nullptr ? x : *val_;
    }

private:
    std::shared_ptr<T> val_;
};

The argument passed to Option::getOrElse is the default value to return when this Option is empty:

Option<int> x;  // empty
int y = 123;
x.getOrElse(y);  // == 123

However, I think the following code is not safe:

Option<int> x;
x.getOrElse(123);  // reference to temporary variable!

A safer way would be to return by value from Option::getOrElse, but that would be wasteful when the Option is non-empty. Can I work around this somehow?

UPDATE: I'm thinking about perhaps overloading on the argument type (lvalue/rvalue) of getOrElse, but haven't figured out exactly how to do so.

UPDATE 2: Maybe this?

T getOrElse(T &&x) const { ... }

const T & getOrElse(const T &x) const { ... }

But I think this might be ambiguous because both lvalue and rvalue arguments fit the second version.

Answer 1

However, I think the following code is not safe:

Option<int> x;
x.getOrElse(123);  // reference to temporary variable!

You are correct. This is why std::optional::value_or() returns a T and not a T& or T const&. As per the rationale in N3672:

It has been argued that the function should return by constant reference rather than value, which would avoid copy overhead in certain situations:

void observe(const X& x);

optional<X> ox { /* ... */ };
observe( ox.value_or(X{args}) );    // unnecessary copy

However, the benefit of the function value_or is only visible when the optional object is provided as a temporary (without the name); otherwise, a ternary operator is equally useful:

optional<X> ox { /* ... */ };
observe(ox ? *ok : X{args});            // no copy

Also, returning by reference would be likely to render a dangling reference, in case the optional object is disengaged, because the second argument is typically a temporary:

optional<X> ox {nullopt};
auto&& x = ox.value_or(X{args});
cout << x;                              // x is dangling!

I suggest you follow the same guidelines. If you really need to avoid the copy, use a ternary. This is safe and copyless:

Optional<int> ox = ...;
const int& val = ox ? *ox : 123;

If you really don't, or the Optional is an rvalue anyway, getOrElse() is more concise.