views:

286

answers:

2

I have a pet project with which I experiment with new features of the upcoming C++0x standard. While I have experience with C, I'm fairly new to C++. To train myself into best practices, (besides reading a lot), I have enabled some strict compiler parameters (using GCC 4.4.1):

-std=c++0x -Werror -Wall -Winline -Weffc++ -pedantic-errors

This has worked fine for me. Until now, I have been able to resolve all obstacles. However, I have a need for enable_shared_from_this, and this is causing me problems. I get the following warning (error, in my case) when compiling my code (probably triggered by -Weffc++):

base class ‘class std::enable_shared_from_this<Package>’ has a non-virtual destructor

So basically, I'm a bit bugged by this implementation of enable_shared_from_this, because:

  • A destructor of a class that is intended for subclassing should always be virtual, IMHO.
  • The destructor is empty, why have it at all?
  • I can't imagine anyone would want to delete their instance by reference to enable_shared_from_this.

But I'm looking for ways to deal with this, so my question is really, is there a proper way to deal with this? And: am I correct in thinking that this destructor is bogus, or is there a real purpose to it?

+11  A: 

A destructor of a class that is intended for subclassing should always be virtual, IMHO.

A virtual destructor in a base class is only needed if an instance of the derived class is going to be deleted via a pointer to the base class.

Having any virtual function in a class, including a destructor, requires overhead. Boost (and the TR1 and C++0x standard library) doesn't want to force you to have that overhead just because you need to be able to obtain a shared_ptr from the this pointer.

The destructor is empty, why have it at all?

If you don't have a user-defined constructor, the compiler provides one for you, so it doesn't really matter.

I can't imagine anyone would want to delete their instance by reference to enable_shared_from_this.

Exactly.

As for the compiler warning, I would ignore the warning or suppress it (with a comment in the code explaining why you are doing so). Occasionally, especially at "pedantic" warning levels, compiler warnings are unhelpful, and I'd say this is one of those cases.

James McNellis
+3  A: 

I agree with Jame's description, but would add

A virtual destructor is only required if you want to destroy an instance of that class virtually. This is not always the case, however if a base class is not intended to be destroyed virtually it should protect against it

So I would change

A destructor of a class that is intended for subclassing should always be virtual, IMHO.

this to be:

A destructor of a class that is intended for subclassing should always be virtual or protected.

iain
Hmm.. good point. The destructor of `enable_shared_from_this` actually is protected. So perhaps this is a bug in GCC's `-Weffc++`?
Shtééf
@Shtééf: Effective C++ says that polymorphic bases should have virtual dtors, and classes not intended for polymorphism should not have virtual dtors. Since the compiler can't tell which is which, I'd say that yes it is a bug to offer this warning. So either your implementation of IIRC the advice about protected dtors in non-polymorphic base classes if from GotW / Exceptional C++, not Effective.
Steve Jessop
Further, my version of the gcc man page says that the `-Wnon-virtual-dtor` warning is only for classes with a virtual function. So either `enable_shared_from_this` has a virtual function (my implementation doesn't), or else the warning isn't following the documentation (which would be a bug), or else the docs have changed (check yours, maybe they explain the warning further). Actually, it's possible that *Effective C++* has been updated - the gcc man page quotes titles which don't match my 3rd edition. So arguably it's a bug in Scott Meyers' book ;-)
Steve Jessop