Discussion: Make base class destructors protected and non-virtual (always)

[purpleKarrot]

For many years, the guideline was:

A base class destructor should be either public and virtual, or protected and non-virtual

However, with C++26, it can acutally be simplified to:

A base class destructor should be protected and non-virtual

class Base
{
public:
  virtual void foo() = 0;
protected:
  ~Base();
};

class Derived : public Base
{
  void foo() override;
};

{
  Derived d;                                               // OK
  std::polymorphic<Base> p1 = Derived{};                   // OK
  std::shared_ptr<Base> p2 = std::make_shared<Derived>();  // OK
  std::unique_ptr<Base> p3 = std::make_unique<Derived>();  // FAILS to compile, should not be used.
  Base* p4 = new Derived; delete p4;                       // FAILS to compile, should not be used.
}

The simplified guideline is easier to learn, easier to teach, and easier to remember.
For experienced programmers, it requires accepting a new mindset.

[jwakely]

Why?

Are you saying std::polymorphic<Base> should be used in preference to std::unique_ptr<Base>? Why? That would be a massive breaking change for many code bases, and if they previously had a public virtual ~Base() then they were correct and safe and met the guidelines.

[jwakely]

And std::polymorphic doesn't support move-only types.

[hsutter]

If the base class allows polymorphic destruction, its destructor must be public (so it can be called) and virtual (so it's correct). There are many examples; @jwakely gives one.

Is the suggested restriction documented as a known best practice somewhere? The Guidelines aim to codify existing known best practices. The current "public and virtual or protected and non-virtual" is documented by me here, and googling finds a number of similar guidelines. I can't remember whether I was the first to formulate it that way or not.

What problem would banning public virtual base destructors improve? The Guidelines sometimes are opinionated to prefer one legal style over another, but only when banning something that is dangerous or error-prone.

[jwakely]

I very much doubt there are any best practices based on std::polymorphic which is new in C++26 (which is not even published yet!) and only implemented in one compiler (GCC 16, not even released yet!)

Since the core guidelines do not use C++26 yet, this is a non-starter (and IMHO would still be wrong in a few years when the guidelines use C++26).

[purpleKarrot]

Are you saying std::polymorphic<Base> should be used in preference to std::unique_ptr<Base>?

Yes, exactly that. This also implies that Base should no longer be artificially restricted to be move only.

Why?

Because the guideline is simpler, more straight forward (no more either or) and hence easier to learn, easier to teach, and easier to remember. I am well aware of the origin of the original guideline.

That would be a massive breaking change for many code bases.

That is the case for any new guideline in a code base that does not follow it already.

Is the suggested restriction documented as a known best practice somewhere?

I started teaching it already for C++26, and I expect it to gain popularity when C++26 becomes widely available, for the reasons given above.

[jwakely]

Yes, exactly that. This also implies that Base should no longer be artificially restricted to be move only.

polymorphic requires the derived type to be copyable.

[purpleKarrot]

Yes, exactly that. This also implies that Base should no longer be artificially restricted to be move only.

polymorphic requires the derived type to be copyable.

Yes, I know.

In most code bases that I have seen, base classes = delete their copy constructor and copy assignment operator (I have seen = delete in derived classes much less). I think this approach should be discouraged with C++26. Having a virtual function is no longer a reason for making a class non-copyable.

Your resistance actually proves my point: It requires accepting a new mindset. It is hard to break with old habits.

[jwakely]

I have no problem with new mindsets, but that doesn't belong here (yet), because as Herb said:

The Guidelines aim to codify existing known best practices.