C++ ownership of a function parameter

Question

I wrote a function which has this form:

Result f( const IParameter& p);

My intention is that this signature will make it clear that the function is not taking ownership of the parameter p.

Problem is that Result will keep a reference to IParameter:

class Result
{  
    const IParameter& m_p;

public: 
    Result( const IParameter& p ) 
        : m_p( p ){ } 
};

But then it happened that somebody called the function like this:

const auto r = f(ConcreteParameter{});

Unfortunately the temporary can be bound to const reference, and this caused a crash.

Question is: how can I make it clear that the function is not supposed to be called with temporaries, and maybe have a nice compilation error when that happens? Is it actually wrong in this case to state that it is not taking the ownership, since is passing it to result that will be propagated outside the function call scope?

Answer 1

The easiest way to make it clear is to overload the function with an rvalue reference parameter. Those are prefered to const references for temporaries so they will be chosen instead. If you then delete said overload, you'll get a nice compiler error. For your code that would look like:

Result f( const IParameter&& ) = delete;

You can also do the same thing with Result to gaurd it as well and that would look like:

class Result
{  
    const IParameter& m_p;

public: 
    Result( const IParameter& p ) 
        : m_p( p ){ } 
    Result( const IParameter&& ) = delete;
};

Answer 2

In general, if a function receives value by const&, it's expected, that the function will use the value, but won't hold it. You do hold the reference to value so you should probably change argument type to use shared_ptr (if the resource is mandatory) or weak_ptr (if resource is optional). Otherwise you'll run into that kind of problems from time to time, as no one reads documentation.

Answer 3

It's hard to tell. The best way would be to document that Result must not live longer than the IParameter used to construct it.

There are valid cases of temporaries sent as constructor that is perfectly valid. Think about this:

doSomethingWithResult(Result{SomeParameterType{}});

Deleting the constructor taking temporaries would prevent such valid code.

Also, deleting the rvalue constructor won't prevent all cases. Think about this:

auto make_result() -> Result {
    SomeParameterType param;
    return Result{param};
}

Even if the constructor with temporary is deleted, invalid code is still really easy to make. You will have to document the lifetime requirement of your parameters anyways.

So if you have to document such behavior anyways, I would opt for what the standard library does with string views:

int main() {
    auto sv = std::string_view{std::string{"ub"}};
    std::cout << "This is " << sv;
}

It won't prevent constructing string views from temporary strings since it can be useful, just like my first example.