Expressing Ownership Transfer of pointers in function signatures


#1

Since the conversion from ScopedPointer to std::unique_ptr in the framework, I’ve started to look at a lot of code I’ve been writing and making it more visually apparent that ownership is being transferred when it comes to functions that take pointers. an example of this would be

SynthesiserVoice* Synthesiser::addVoice (SynthesiserVoice* const newVoice)
{
    const ScopedLock sl (lock);
    newVoice->setCurrentPlaybackSampleRate (sampleRate);
    return voices.add (newVoice);
}

Now, there’s nothing in that function that actually tells me (the user) that ownership of newVoice is being transferred, other than the documentation. if I didn’t know that voices was an OwnedArray<SynthesiserVoice> voices; and didn’t know how OwnedArray worked, I would not know that the passed-in pointer would become owned by the class.

So, I’m wondering if this small change to functions like this would help benefit expressing ownership transfer for those of us who don’t read the documentation as deeply as we should:

SynthesiserVoice* Synthesiser::addVoice (std::unique_ptr<SynthesiserVoice> newVoice)
{
    const ScopedLock sl (lock);
    newVoice->setCurrentPlaybackSampleRate (sampleRate);
    return voices.add (newVoice.release());
}

now, when you’re adding a voice to your synth, you have to declare that the synth will own the voice:

    for( int i = 0; i < numVoices; ++i )
    {
        auto voice = std::make_unique<MyVoice>();
        synth->addVoice( std::move(voice) );
    }

Thoughts? Comments? Ever since I started doing this style in my own code, i’ve had to write less documentation because the function signatures express exactly what happens to the pointers being passed to the function.


#2

It’s great for the reasons you mention: Self-documenting, statically checked and removes room for error - all important properties of code quality.
I would hesitate to call it a pattern / be limited to function signatures, it should be the default modus operandi in mordern C++ :slight_smile:

I would also just model the OwnedArray as an array of unique_ptr’s, further ensuring that addVoice() itself isn’t buggy.


#3

well, OwnedArray has some pretty awesome member functions that are a pain in the ass to implement via , so OwnedArray is worth keeping around. it’s one of the best tools in JUCE, imho.

oh, and Synthesiser is a JUCE class, not my own class.


#4

Or only used pointer when you are transferring ownership and use reference otherwise.


#5

Or only used pointer when you are transferring ownership and use reference otherwise.

Pointers are often used to express “non-owned, optional” (whereas reference is non-optional).

SynthesiserVoice* Synthesiser::addVoice (std::unique_ptr<SynthesiserVoice> newVoice)

I’m in favor of that style :slightly_smiling_face: But shouldn’t it be unique_ptr<SynthesiserVoice>&& ?


#6

:+1:

Pointers are often used to express “non-owned, optional ” (whereas reference is non-optional).

I don’t remember seeing anything that gets a reference and takes its ownership, while I’ve seen many constructors and functions taking pointers with the ownership.


#7

in c++17, there are a bunch of new compiler tricks that let you get away with pass-by-value arguments that the compiler automatically converts to r-values or something weird like that. the point being that you don’t need to use unique_ptr<>&& as the function argument. the folks here know way more than me about it: https://discord.gg/J5hBe8F


#8

I agree with everything here, API should handle memory management by contract!

And indeed, the prototype of the function should pass the smart pointer by value, and not rvalue. The reason is also that by value allows you more options, and also tells you that you really sink the object, not just that you “could”.


#9

Ah, good to know, thanks. I just noticed it’s also preferred in the C++ core guidelines.