0

Consider the following hierarchy:

class BaseICannotChange {};

class DerivedIControl: public BaseICannotChange {
    private:
    int _Value;
    
    public:
    DerivedIControl(int value): _Value{value} {}
    int getValue () const {return _Value;}
};

I have a std::unique_ptr to a DerivedIControl. I want to retain the ability to get the value after the pointer has been moved, but while I am sure that the underlying object still exists:

int main () {
  auto myDerived = std::make_unique<DerivedIControl>(42);
  std::cout << "My value is: " << myDerived->getValue() << std::endl;
  std::unique_ptr<BaseICannotChange> myBase = std::move(myDerived);
  
  // This would crash as myDerived has been moved.
  // std::cout << "My value is: " << myDerived->getValue() << std::endl;
  return 0;
}

As I have control of DerivedIControl, is it legit to swap the getter for a std::function that I can then copy before moving the pointer?

class BaseICannotChange {};

class DerivedIControl: public BaseICannotChange {
    private:
    int _Value;
    
    public:
    DerivedIControl(int value): _Value{value} {}
    std::function<int(void)> getValue = [this] () {return _Value;};
};

Now I can write this:

int main () {
  auto myDerived = std::make_unique<DerivedIControl>(42);
  std::cout << "My value is: " << myDerived->getValue() << std::endl;
  auto myGetter = myDerived->getValue;
  std::unique_ptr<BaseICannotChange> myBase = std::move(myBase);
  std::cout << "My value is: " << myGetter() << std::endl;
  return 0;
}

I have tried this on a couple of online compilers, and it seems to work.

Is there a catch I am not seeing that makes it undefined behavior?

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
Antonio
  • 355
  • 1
  • 2
  • 13
  • 1
    [std::unique_ptr::get](https://en.cppreference.com/w/cpp/memory/unique_ptr/get) – Eljay Mar 08 '23 at 18:53
  • 1
    Why do you need to move it to `std::unique_ptr` anyway? Can you print before moving? – Aykhan Hagverdili Mar 08 '23 at 18:59
  • @Eljay Can you elaborate on the suggestion? As far as I understand `get` would release ownership, which is not what I want to do. Perhaps this is not clear in the question, but a constraint is that I need to end up with a working `std::unique_ptr` to `BaseICannotChange`. – Antonio Mar 08 '23 at 19:01
  • 3
    `get()` does not release ownership. That would be `release()`. – sweenish Mar 08 '23 at 19:07
  • 2
    Your code is basically equivalent to: `DerivedIControl *tmp = myDerived.get(); ...; tmp->getValue();` There is no need to invent some elaborate mechanisms when you can just store plain pointer. Or you can cast `myBase.get()` to `DerivedIControl *` either with `static_cast` or `dynamic_cast`. – sklott Mar 08 '23 at 19:08
  • If the constraint is to have a valid `BaseICannotChange`, why not just call the function from that pointer after the move, or call what you need before the move? You're trying to get around the whole idea behind `unique_ptr`. Do both pointers need to be valid? Do they both actually require ownership of the object? Maybe a `shared_ptr` is what you need. – sweenish Mar 08 '23 at 19:10
  • All other topic aside, hopefully the base class has virtual destructor in the actual code. – alagner Mar 08 '23 at 19:14
  • @AyxanHaqverdili, in my actual project, this happens inside a larger class that needs to let downstream code query the value. It doesn't need to print it. – Antonio Mar 08 '23 at 19:14
  • @Antonio There's about a million ways to get this working in C++ (hold a regular pointer to the object, use dynamic/static cast to access the subclass after the fact, use shared_ptr instead, etc etc). We need more details to help you the right way. – Aykhan Hagverdili Mar 08 '23 at 19:34
  • *As far as I understand get would release ownership, which is not what I want to do.* I am not sure where you got that misunderstanding. Have you read the information in the link to cppreference I provided? – Eljay Mar 08 '23 at 19:55

1 Answers1

1

Yes, your code is valid. Moving a unique_ptr does not move the object it points to, so the captured this pointer in the std::function always remains valid. (To be pedantic: the only issue with your code is that BaseICannotChange needs a virtual destructor.)

However, I think you're overthinking this. There's no reason to change the derived class's interface. Keep it simple and just store an extra reference to the object at the place where you need to.

int main () {
  auto myDerived = std::make_unique<DerivedIControl>(42);
  std::cout << "My value is: " << myDerived->getValue() << "\n";
  auto &stillMyDerived = *myDerived; // get a non-owning reference to object
  std::unique_ptr<BaseICannotChange> myBase = std::move(myDerived); // and pass ownership to something else
  // object hasn't gone anywhere, this is fine
  std::cout << "My value is still: " << stillMyDerived.getValue() << "\n";
  return 0;
}

Note that you can't use a reference as a data member of a class (or, you can, but it makes a huge mess). Contrary to all the comments, I would not use std::unique_ptr::get to get a non-owning pointer in that case. This is not even to "avoid raw pointers", the issue is simply that a raw pointer is logically allowed to be nullptr, but it appears that you're not allowed to not have a valid reference in this case. So you should use a type that makes this clear:

class TemporaryOwner {
    std::unique_ptr<DerivedIControl> ownership;
    std::reference_wrapper<DerivedIControl> object;
public:
    TemporaryOwner(int value)
      : ownership(std::make_unique<DerivedIControl>(value))
      , object(*ownership)
      { }
    DerivedIControl &peekObject() { return object; }
    void printValue() { std::cout << "My value is: " << peekObject().getValue() << "\n"; }
    std::unique_ptr<BaseICannotChange> takeOwnership() { return std::move(ownership); }
};
// objects of this class become invalid if you take ownership and end up destroying the underlying object too early
// it would be *safer* to use shared_ptr everywhere, but I would consider it okay to do it this way *if* you're careful
HTNW
  • 27,182
  • 1
  • 32
  • 60
  • While this works, 9/10 times this is a wrong/ugly/error-prone way to work around a poor design decision and you probably have much better ways to do whatever you're trying to do. – Aykhan Hagverdili Mar 08 '23 at 23:23