non-virtual-destructor warnings and false positives

Hello,

I noticed that -Wnon-virtual-destructor is not activated by default. In fact, it is not activated either by -Wall or -Wextra.

This is a bit annoying, because it’s an easy error to get into a scenario where one would needed, and the lack of it incurs Undefined Behavior.

However, the way the warning currently is implemented, it reports many false positives. The common case I could think of was:

#include
#include

class Base {
public:
virtual int getId() const = 0;
protected:
~Base() {}
};

class Derived: public Base { // expect-warning {{‘Derived’ has virtual functions but non-virtual destructor [-Wnon-virtual-dtor]}}
public:
explicit Derived(int i): _id(i) {}
virtual int getId() const { return _id; }
private:
int _id;
};

void print(Base const& b) { std::cout << b.getId() << “\n”; }

int main(int argc, char* argv[]) {
{
Derived d(argc);
print(d);
}
{
std::auto_ptr b(new Derived(argc));
print(*b);
}
return 0;
}

Here, there is absolutely no need for a virtual destructor, yet a warning is emitted.

This check is difficult. There are two main issues as I see it:

  • Policy Based design encourages you to inherit (privately, but still) from the policies in case they might be empty, to trigger the empty base optimization, thus you may inherit from non-polymorphic classes
  • It is impossible to know, at the point of delete, whether a class is used polymorphically or not (in another TU)

I was wondering if it would be possible to implement this check more conservatively, at the point of use: that is, on delete. Note that the array version does not need to be checked since arrays are not polymorphic.

More specifically, warning for a delete called on an object with virtual functions but no virtual destructors. Of course, this would still be imprecise. In the above case, the destruction of std::auto_ptr<Derived> is perfectly fine, yet would trigger the warning.

Still it would be less noisy, I think. It would notably nicely interact with Inversion of Control technics (such as Dependency Injection) where the object is built on the stack and passed by reference to an interface.

Also I believe the check would not be much more expensive that it is now (I don’t think there would be much calls to delete on a given class in a single TU) and would silence many false positives.

Has it been attempted already ?

Is it doomed to fail ?

– Matthieu