cppcoreguidelines-virtual-class-destructor ... complains when Base has default protected destructor

Question

We are using clang-tidy to do static analysis, and we've enabled cpp-core-guidelines.

As per previous advice (by one of the authors of cpp-core-guidelines), we have a abstract base class with a protected default destructor, and a derived class (simple hierarchy) with a public non-virtual destructor.

struct Base {
  public: 
    // all functions here are pure virtual
    virtual void allFunctionsHereArePureVirtual() = 0; 
  protected: 
    ~Base() = default; 
};

// NOT that final matters here... but cpp-core-guidelines still complains!!!!
struct Derived final : Base {
  public:
    ~Derived(); //implementation irrelevant
    void allFunctionsHereArePureVirtual() override; 
};

Is there any other good reason why clang-tidy when enforcing [cpp-core-guidelines], complains about this?

Destructor of 'Derived' is public and non-virtual (fix available)clang-tidycppcoreguidelines-virtual-class-destructor

It should not be possible to delete via Base polymorphically (compiler error), so what is the issue in this instance (bad linting tool? bad guidelines?). Would the new guideline be... even protected destructors of abc's should be virtual, as deleting via derived will optimize the base destructor virtualness away in any case?

For the sake of bad linters (and compiler warnings that can't discern), would it not be better to just make destructors protected and virtual (instead of selectively removing warnings)?

Thanks in advance

Answer 1

Clang-tidy correctly implements the Core guideline C.35 which states:

Enforcement

• A class with any virtual functions should have a destructor that is either public and virtual or else protected and non-virtual.
• If a class inherits publicly from a base class, the base class should have a destructor that is either public and virtual or else protected and non-virtual.

Your Derived has virtual methods, hence, according to the first bullet, its destructor should be either public and virtual or protected and non-virtual. It is not.

However, I think your code is ok. And it seems that final is not considered for the guideline. When Derived is final and you can only destroy it via ~Derived its ok to not declare the destructor as virtual.

In a discussion of C.35 an exception is mentioned where they write:

In this rare case, you could make the destructor public and non-virtual but clearly document that further-derived objects must not be used polymorphically as B’s. This is what was done with std::unary_function.

It seems like an oversight to me that also here final is not mentioned. If a class is final there is no need to "clearly document" what further derived classes must not do.

Clang-tidys example here https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/virtual-class-destructor.html is also in line with the Core Guideline.