CODEWITHSUNDEEP
c++ownership
nyarlathotep108
nyarlathotep108

Reputation: 5523

C++ ownership of a function parameter

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?

Upvotes: 5

Views: 1216

Answers (5)

nyarlathotep108
nyarlathotep108

Reputation: 5523

I already voted @NathanOliver answer as the best one, because I really think it is given the information I provided. On the other hand I would like to share what I think is a better solution to solve this very specific scenario when the function is more complex than the one in my initial example.

The problem with the delete solution is that it grows exponentially with the number of parameters, assuming that all the parameters needs to stay alive after the function call ends and you want a compile time check that the user of your API is not trying to give the ownership of those parameters to the function:

void f(const A& a, const B& b)
{
    // function body here
}

void f(const A& a, B&& b) = delete;
void f(A&& a, const B& b) = delete;
void f(A&& a, B&& b) = delete;

We need to delete all the possible combination, and this will be hard to maintain on the long run. So my proposed solution is to take advantage of the fact that the reference_wrapper constructor which wraps T by move is already deleted in STD, and then write this:

using AConstRef = reference_wrapper<const A>;
using BConstRef = reference_wrapper<const B>;

void f(AConstRef a, BConstRef b)
{
    // function body here
}

In this way all the invalid overload will be automatically deleted. I do not see any drawback with this approach so far.

Upvotes: 0

lubgr
lubgr

Reputation: 38277

You can manually remove a constructor accepting an IParameter&& rvalue from the overload set:

class Result
{
    // ...

    public:
    Result( IParameter&& ) = delete; // No temporaries!
    Result( const IParameter& p );
};

When client code tries to instantiate an object via

Result f(ConcreteParameter{}); // Error

the constructor taking a const-qualified reference is no match because of the missing const-ness, but the rvalue constructor exactly matches. As this one is = deleted, the compiler refuses to accept such an object creation.

Note that as pointed out in the comments, this can be circumvented with const-qualified temporaries, see @NathanOliver's answer for how to make sure this doesn't happen.

Also note that not everyone agrees that this is good practice, have a look here (at 15:20) for example.

Upvotes: 3

Guillaume Racicot
Guillaume Racicot

Reputation: 41780

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.

Upvotes: 3

Radosław Cybulski
Radosław Cybulski

Reputation: 2992

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.

Upvotes: 3

NathanOliver
NathanOliver

Reputation: 180630

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;
};

Upvotes: 9

Related Questions