Re: [lmi] depr.impldec

[Vadim Zeitlin]

On Thu, 14 Jul 2022 13:11:05 +0000 Greg Chicares <gchicares@sbcglobal.net> 
wrote:

GC> On 7/13/22 23:39, Vadim Zeitlin wrote:
GC> > On Wed, 13 Jul 2022 21:52:55 +0000 Greg Chicares 
<gchicares@sbcglobal.net> wrote:
GC> > 
GC> > GC> I propose that we follow Sutter's suggestion here:
GC> > GC>   
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rc-dtor-virtual
GC> > GC> | A base class destructor should be either public and virtual,
GC> > GC> | or protected and non-virtual
GC> > 
GC> >  This looks like a good rule.
GC> 
GC> Would it be generally regarded as a good rule if Sutter hadn't said it?

 Yes, I think so -- I did think about it before writing the above and I
still think it's generally good. I didn't say it was perfect and had to be
applied in all circumstances, but it seems like a good general guidelines.

GC> When you say
GC> > It's a bit unusual to
GC> > have the dtor become virtual in the middle of the inheritance chain
GC> one might add "to put it mildly". I've never seen such a thing.

 Me neither, but it doesn't seem like a good reason to dismiss it out of
hand, otherwise we'd never change anything that we do. To be honest, I did
hope to find some good reason not to do it like this, but after thinking
about it for a few minutes I just couldn't find any.

GC> I'd be inclined to reject it in code review without a demonstration of
GC> some actual benefit.

 It's true that I don't see much of it. From efficiency point of view, the
gains are certainly negligible. From the correctness point of view, it
prevents from using objects via pointers to the base class, but it's not
really clear why is it an advantage.

 In fact, maybe it's worth taking a step back and asking ourselves why
exactly do we want to prevent this from being done. I don't see anything
really wrong with having a vector<unique_ptr<datum_base>> for example, is
there?

GC> But I see no benefit at all. The illustrative patch below [0] compares
GC> the 'sizeof' each class in two hierarchies that are minimal examples
GC> of what 'datum_base' does--viz., it presents an interface with pure
GC> virtuals. In one hierarchy, the base class's dtor is protected and
GC> nonvirtual; in the other, it's protected and virtual, contrary to
GC> the GoTW#5 advice AIUI. The classes are the same size (8 bytes here)
GC> as opposed to an empty class (1 byte). They're the same size because
GC> every one of them has a vtable, because every one has a pure virtual;
GC> and making the dtor virtual or not doesn't change that.

 It does change the size of the vtable itself, but this is negligible (I am
not quite sure about this when thinking of adding virtual functions to
wxWindow, as this adds an extra pointer to vtables of all classes deriving
from wxWindow, of which there are quite a few, but there are not nearly as
many classes deriving from datum_base).

GC> For a base class like the erstwhile std::unary_function, a non-virtual
GC> dtor is just fine. For a base class like 'datum_base', a non-virtual
GC> dtor must change its nature to virtual along the inheritance chain,
GC> which, lacking any benefit, is just gratuitous weirdness.

 Maybe the real problem is that it becomes virtual, i.e. perhaps there is
something wrong with our inheritance hierarchy and we should avoid having a
class (datum_string) which is both a concrete class and a base class?

GC> Instead of GoTW#5, I propose:
GC> 
GC> A base class destructor should generally be protected and virtual.
GC> Exceptions: make it
GC>  - public iff it is necessary to call the derived class's
GC>    dtor though a pointer to base.
GC>  - non-virtual iff that prevents the base from needing a vtable.

 So, equivalently:

- For a class with virtual functions, make the dtor virtual.
- Make it public by default or protected if using objects via the base
  class pointer is forbidden.

 But I still struggle to come up with any good example of a situation when
we need to forbid doing this, to be honest. So maybe we should just always
make the base class dtor public (and then, unavoidably, virtual) and forget
about all the complications?

VZ

pgpe7VvdH8vDO.pgp
Description: PGP signature