Derived class constructor taking rvalue ref to base class

Question

I've got the following classes: BaseWidget, ActiveWidget, and DetachedWidget. Both derived classes use resources provided by BaseWidget: vector and int. The current so-called "state" (represented by each derived class) is stored in Context as a unique ptr to base class.

Imagine that Context is like an entry point for the system. For simplicity Context's handle method does not take any parameter but in a real scenario, it would take some data.

Anyways, when one calls Contex's handle method, then the call is forwarded to the current "state" and then it processes the "command". As you can see, e.g. when the current state is represented by DetachedWidget, and when the handle method gets called, then DetachedWidget exchanges the current state to ActiveWidget passing to ActiveWidget's constructor one parameter which is an rvalue reference to BaseClass created by invoking std::move(*this). The purpose of this is that I want to pass all resources that DetachedWidget owns to the new instance. Of course, I could have passed e.g. vector, and int as separated parameters, but I knew that in every case I would like to exchange all resources, so taking the base class as a parameter and then passing it to move ctor of base class sounds like a quite good idea.

But I'm not 100% sure if there are any pitfalls that I'm not aware of yet. There should be no slicing issues nor dangling references and so on but... I'm especially interested in two aspects:

• A derived class that has a constructor which takes rvalue reference to BaseClass
• std::move(*this)

#include <memory>
#include <iostream>
#include <algorithm>
#include <random>
#include <vector>
#include <cassert>

class Context;
struct BaseWidget
{
  int x;
  std::vector<int> vec;

  virtual void handle(Context&) = 0;
};


struct Context
{
  std::unique_ptr<BaseWidget> widget;

  void set_widget(std::unique_ptr<BaseWidget> w)
  {
    widget = std::move(w);
  }

  void handle()
  {
    widget->handle(*this);
  }
};

struct ActiveWidget : BaseWidget
{
  explicit ActiveWidget(BaseWidget&& base)
    : BaseWidget(std::move(base))
  {
    assert(vec.size() == 2);
  }

  void handle(Context& ctx) override
  {

  }
};

struct DetachedWidget : BaseWidget 
{
  DetachedWidget()
  {
    vec.resize(2);
  }
  void handle(Context& ctx) override
  {
    ctx.set_widget(std::make_unique<ActiveWidget>(std::move(*this)));
  }
};


int main() 
{
  auto ctx = Context{};
  auto initial = std::make_unique<DetachedWidget>();
  ctx.set_widget(std::move(initial));
  ctx.handle();



    return 0;                                        
}

```

Answer 1

Never let an object delete itself

I've gone down a similar road myself, and there are pitfalls everywhere. Just do not do it. The problem is that while there is no explicit delete, the fact that DetachedWidget::handle() calls ctx.set_widget() while ctx is the owner of the DetachedWidget object means that in effect, DetachedWidget is deleting itself. Before set_widget() even returns, that means the DetachedWidget object is no longer in a valid state. Any further execution of its member functions is undefined behavior.

There might be ways around this. You could make a widget's handle() return a function object which Context::handle() will then call the moment it gets it, and that function object can then call set_widget(). For example:

struct Context {
    ...
    void handle() {
        widget->handle(*this)(); // Note the extra parentheses
    }
    ...
};

struct DetachedWidget : BaseWidget {
    ...
    std::function<void()> handle(Context& ctx) override {
        return [this](Context& ctx){
             ctx.set_widget(std::make_unique<ActiveWidget>(std::move(*this)));
        };
    }
    ...
};

But this looks very contrived. Which brings me to:

Polymorphism might be the wrong solution here

Do you need to have a separate ActiveWidget and DetachedWidget at all? If they are already so intertwined, why not just have one Widget class that has a bool active member variable?

If either one of those widgets has a lot of state that the other doesn't need, and you don't want to pay for the memory usage, then perhaps just store that state in a separate object held in a std::unique_ptr member variable in Widget.