Wrong logic used for virtual destructor check

[Tim Yorke]

Hi, I've noticed that the wrong logic is being used when performing the virtual destructor check.

void CheckClass::virtualDestructor()
{
    // This error should only be given if:
    // * base class doesn't have virtual destructor
    // * derived class has non-empty destructor
    // * base class is deleted
    // unless inconclusive in which case:
    // * base class has virtual members but doesn't have virtual destructor

in particular this condition:

* derived class has non-empty destructor 

It doesn't consider member variables in the derived class.

Here's some example code that will be passed by cpp-check:

#include <iostream>
#include <memory>

using namespace std;

struct A
{
    A()  { cout << "A is constructing\n"; }
    ~A() { cout << "A is destructing\n"; }
};

struct Base {};

struct Derived : Base
{
    A a;
};

int main()
{
    Base* p = new Derived();
    delete p;
    std::cout << "Bye!\n";
}

Output:

A is constructing
Bye!

These kind of missing virtual destructor errors are very important to detect correctly as they can cause silent memory corruption, resource leaks etc.

[Daniel Marjamäki]

I created ticket:
https://trac.cppcheck.net/ticket/9104

Feel free to implement a fix for this. It seems to me you have understood the cppcheck source code.