0

Aka: Is there any "Calling Virtuals During Deinitialization" idiom

I am cleaning up some old code and need to fix cases where virtual methods are called in constructors and destructors. I don't know the code base and it is huge. Major rewrite is not an option.

The fix for constructors was simple. I moved the virtual calls to a static Create template and made all the constructors protected. Then all I needed to do was to compile and change all location causing errors to use the Create template. Minimal chance for regressions. However there is no analog to this for destructors.

How would you solve this?

Example code

#include <iostream>

class Base
{
public:
    virtual ~Base()
    {
        DeInit();
    }
protected:
    virtual void DeInit()
    {
        std::cout << "Base" << std::endl;
    }
};

class Derived : public Base
{
protected:
    virtual void DeInit() override
    {
        std::cout << "Derived" << std::endl;
        Base::DeInit();
    }
};

int main()
{
    Derived d;
}

This code does not call Derived::DeInit (only prints "Base"). I need to fix this kind of issues.

Working example code

Mathias
  • 1,446
  • 2
  • 16
  • 31
  • 2
    What problem do you need to solve? Virtual destructors already work as intended. – Kerrek SB Jul 30 '15 at 08:11
  • @KerrekSB: OP wants to avoid virtual calls in constructor/destructor as the behaviour of calling virtual method there is not the same as in other context. – Jarod42 Jul 30 '15 at 08:19

4 Answers4

1
...
virtual Base::~Base()
{
    Base::DeInit();
}
...

...
Derived::~Derived()
{
    // de-initialization code
    // do not call Derived::DeInit() here as otherwise Base::DeInit()
    // will be called two times
}
...

and cleanup of virtual function calls from destructors when spotting them.

bobah
  • 18,364
  • 2
  • 37
  • 70
  • 1
    If you do this, why not remove `DeInit` altogether? Which often is the correct choice, BTW. – MSalters Jul 30 '15 at 08:31
  • @Msalters - it boils down to what author can and cannot touch and whether he can efficiently (=automatically) "inline" method calls done from destructors – bobah Jul 30 '15 at 08:37
1

This is quite tricky, as destructors are called automatically on leaving scopes, whether by normal flow, break, continue, return or throw. That's also why you can't pass arguments to a destructor.

The straightforward solution is to call Derived::DeInit from Derived::~Derived. This has the additional benefit of still having Derived members available.

Another is to create your own smart pointer class, which calls T::DeInit before T::~T. To prevent this from being bypassed, return this smart pointer from your Create.

MSalters
  • 173,980
  • 10
  • 155
  • 350
0

you dont need to have a virtual DeInit fun.

    #include <iostream>

    class Base
    {
    public:
        virtual ~Base()
        {
            DeInit(); //this calls Base version
        }
    protected:
        void DeInit()
        {
            std::cout << "Base" << std::endl;
        }
    };

    class Derived : public Base
    {

    public:
         ~Derived()
        {
            DeInit(); //this calls Derived version
        }
    protected:
        void DeInit() 
        {
        std::cout << "Derived" << std::endl;
        }
    };

    int main()
    {
        Derived d;
    }

Output: Derived Base

is this what you wanted?

Murugan P
  • 11
  • 5
  • This is a good solution if you can(dare to) change Derived. If that is the case I would even kill the `DeInit` (and `Init`) methods and go proper RAII. However, if code base is huge and `Base` is a common base class, then the number of `Derived` classes may be vast. Hundreds or more maybe. Making changes to all of those without proper analysis and testing may be a bad idea. – Mathias Jul 31 '15 at 05:42
0

A solution inspired by MSalters second idea.

This solution only requires changes to Base class and to instantiation of Derived classes. No changes needed to any Derived implementation.

#include <iostream>
#include <memory>

class Base
{
private:
    template <class T>
    class WithAutoDeInit : public T
    {
    public:
        virtual ~WithAutoDeInit() override
        {
            T::DeInit();
        }
    };

public:
    template <class T>
    static std::unique_ptr<typename std::enable_if<std::is_base_of<Base, T>::value, WithAutoDeInit<T>>::type> Create()
    {
        return std::make_unique<WithAutoDeInit<T>>();
    }

    virtual ~Base() = default;

protected:
    virtual void DeInit()
    {
        std::cout << "Base" << std::endl;
    }
};

class Derived : public Base
{
protected:
    virtual void DeInit() override
    {
        std::cout << "Derived" << std::endl;
        Base::DeInit();
    }
};

int main()
{
    Base::Create<Derived>();
}

Working example code

This is not a robust solution. You can still make instances of Derived directly. And if you update all your Derived classes with protected constructors an unknowing developer could still create a new class forgetting to make its constructors protected. I wonder if this could be enforced by some kind of assert in a strategic location?

static_assert(std::is_constructible<Derived>::value, "Derived class is constructable");

Btw: I finally choose to rewite the code. It is managable I think, and the resulting code will off cause be simpler (hence better).

Mathias
  • 1,446
  • 2
  • 16
  • 31