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 <iostream>
#include <memory>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<Derived> 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.
This sounds like a good idea to me. gcc behaves the same way as clang here
(in gcc, the warning is behind -Weffc++), and I've previously wished that
gcc behaves as you describe. I believe the main reason it behaves as it
does is to match 'Effective C++' item 7: "Declare destructors virtual in
polymorphic base classes".
In fact, I think two separate warnings might make sense here: a -Weffc++
warning which behaves as the current warning does, and a more conservative
warning which only warns on 'delete' in the conditions you describe below.
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.
In C++0x, we could also suppress the warning for a destructor in a final
class.
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 ?
No idea 
Richard