2

I have a pure virtual class Base and some derived classes ChildA (A/B/C etc):

class Base {
    ...
}

class ChildA : public Base {
    ...
}

I need specific control of the ownership of these child classes, so I spawn them via factory functions and std::unique_ptr.

At some point during their creation/setup, I need to modify them (not make copies or change ownership) in a manner that is common to all of the derived classes, so I want to use a function that accepts their base type and takes by reference:

void modifyInstance(std::unique_ptr<Base>& baseInst);

But when I try to use this function like so:

bool childAFactory(std::unique_ptr<ChildA>& buffer)
{
    buffer = std::make_unique<ChildA>();
    modifyInstance(buffer);
}

I get this error:

error: non-const lvalue reference to type 'unique_ptr<Base>' cannot bind to a value of unrelated type 'unique_ptr<ChildA>'

Is it not possible to take advantage of polymorphism in this particular fashion? The only similar threads I can find are where one is trying to pass by value in which case you obviously have to use std::move() and give up ownership.

I would have the function take a regular Base pointer and use unique_ptr::get() but I also need to be able to reset the pointer within the function conditionally (so the pointer and its contents need to be able to be modified, hence passing a unique_ptr by reference).

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
oblivioncth
  • 315
  • 3
  • 11
  • 1
    If not passing ownership, I'd use `void modifyInstance(Base* baseInst);` or `void modifyInstance(Base& baseInst);` – Eljay Jan 22 '21 at 20:20
  • 2
    Why not to use `void modifyInstance(Base& baseInst);` why do you need to modify `std::unique_ptr` ? – Slava Jan 22 '21 at 20:20
  • 1
    I'll forefront this with how much I think it's a dreadful code smell for why you think you need this, but if you really want to do it you could always template the modifier. I.e. `std::enable_if_t::value > modifyInstance(std::unique_ptr& sp)` – WhozCraig Jan 22 '21 at 20:22
  • If your code was valid, there would be nothing stopping me from doing this in `modifyInstance`: `baseInst = std::make_unique();`. Now suddenly your `std::unique_ptr` in `childAFactory` is pointing to a `ChildB`! Of course this code isn't valid, so you can't do that :) – Kevin Jan 22 '21 at 21:02
  • You should not be passing round smart pointers unless you need to change ownership. Just pass the raw pointer or the reference: `void modifyInstance(Base& baseInst);` – Galik Jan 22 '21 at 21:46
  • This may be relevant from the CppCoreGuidelines: [F.7](https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#f7-for-general-use-take-t-or-t-arguments-rather-than-smart-pointers) – Galik Jan 22 '21 at 21:51
  • Possible duplicate https://stackoverflow.com/questions/35543520/how-to-pass-shared-ptr-to-class-with-lower-lifetime – Galik Jan 22 '21 at 21:55
  • I need to modify unique_ptr because its possible for the function to "fail" in which case I want to set unique_ptr to null (I can just do this in the outer function I guess). Yes this setup overall is a bit clunky but its purpose is because these classes act as a hook to a file that there should only ever be one of for each and I don't want them to be copyable. Once changes are made the ownership is then handed back to the factory class where the file is then saved and released. The extra semantics issues come from me preferring to return vs a buffer and have the return value be a status. – oblivioncth Jan 22 '21 at 22:13
  • @Kevin That does make sense, not something I thought of. Thanks for pointing it out. – oblivioncth Jan 22 '21 at 22:14

1 Answers1

2

I'd do this:

bool modifyInstance(Base* baseInst);

bool childAFactory(std::unique_ptr<ChildA>& buffer)
{
    buffer = std::make_unique<ChildA>();
    if (!modifyInstance(buffer.get()))
        buffer.reset();
}

That is, let the factory remain responsible for lifetimes, and just return an error result from modifyInstance() when the object needs to be destroyed.

John Zwinck
  • 239,568
  • 38
  • 324
  • 436
  • Agreed. Use raw unowning pointers when ownership is not being transferred around. – Remy Lebeau Jan 22 '21 at 23:01
  • This ends up requiring a bit of extra redundant feeling code but is still fairly minimal and is definitely sufficient. I actually ended up doing this in the interim and am now keeping this setup until I rework the whole interface to be a little less particular about managed vs. unmanaged pointers. I agree that otherwise this is the best option. – oblivioncth Jan 23 '21 at 17:56