Thursday 14 September 2023

C++: I was today years old when I learned this about shared_ptr

I was on a review for a colleague and there was this piece of code which stood out to me, I could not figure out what was being achieved.  Without being specific it was essentially

static std::unique_ptr<B> Create(const std::shared_ptr<A>& referenceToA);

Just the function prototype didn't sit right with me and the use case was even more strange to my eye for the "B" structure here being created has the shared pointer in the members.

struct B
{
    B(const std::shared_ptr<A>& referenceToA)
        : mMyA(referenceToA)
    {
    }

public:
   static std::unique_ptr<B> Create(const std::shared_ptr<A>& referenceToA)
    {
        return std::unique_ptr<B>(new B(referenceToA));
    }
};

private:
    std::shared_ptr<A> mMyA;

And my head just lot it.  To my eye this API, the create function here, is given a constant reference to the shared_ptr and what can be constant about a shared_ptr?  Well the internal reference count, or so I thought.  I believed this was a mistake, I believed the compiler would throw this one out and say "No, you can't change the reference count of this const object".

Never had I ever thought that the shared_ptr is actually referencing some other controlling block elsewhere.  My understanding was therefore flawed and I'm happy to admit naive.

So what would happen here?  Well, the B constructor, actually copies the shared_ptr control block, it therefore does and can increment the reference counter to the shared_ptr.

As counter intuitive as the const is therefore.  We aren't actually saying the shared_ptr itself is constant, rather the reference to it is, we should be reading the parameter type as std::shared_ptr<A>const &  referenceToA.

I felt rather silly for not realising this earlier, not least as I did once write a C program to switch out the qualifiers on a function call to make things like this stand out in formatting my code!!

But it has slipped my mind! Okay! I'm old, shut up.

Here is the full code of what I put together to understand this: 

#include <memory>
#include <mutex>
#include <queue>
#include <string>
#include <cstdio>
struct A
{
A()
:
mIndex(GetNextIndex())
{
}
~A() = default;

const int mIndex;
private:
static int GetNextIndex()
{
static int index{ 0 };
return ++index;
}
};
struct B
{
public:
B(const std::shared_ptr<A>& parent)
: mParent(parent)
{
}
std::shared_ptr<A> mParent;
static std::unique_ptr<B> Create(std::shared_ptr<A>const &  referenceToA)
{
return std::make_unique<B>(referenceToA);
}
~B()
{
mParent.reset();
}
const int& GetParentIndex() const { return mParent->mIndex; }
};
int main()
{
std::shared_ptr<A> original{ std::make_shared<A>() };
printf("Original %i: %i\n", original->mIndex, original.use_count());
auto createdB{ B::Create(original) };
printf("Copy %i: %i\n", createdB->GetParentIndex(), createdB->mParent.use_count());
printf("Original %i: %i\n", original->mIndex, original.use_count());
}

However, I have to say, I still don't like this; clever as it is, there's a hidden copy going on, in the B constructor the copy of the shared_ptr control block and it's reference count incrementing from 1 to 2.

It does quite neatly move ownership of the pointer, but the const and the reference just make my brain spin.  So where this can only gain 10/10 plaudits for C++ smarts, it only gets a mere 3/10 on the maintainability ladder for me, and only when we have the const& together, when written as at the top of the page this drops to 1/10.

Performance is also a factor here, if we wanted better performance we would have to consider whom owns the original A here.  If no-one and it is always added into the shared member in B, well move it... Create once and move it, this code doesn't communicate that as an option, but it certainly could be.