Cppcoreguidelines: C.128 Doesn't provide reasoning for rule exception with destructors

[adambadura]

C.128 Virtual functions should specify exactly one of virtual, override, or final makes an exception for destructors:

If a base class destructor is declared virtual, derived class destructors should neither be declared virtual nor override.

However no reasoning for this is provided.

I think it would be better to make it a separate paragraph, perhaps with some emphasis so that it clearly stands out as an exception to the general rule. And also provide there justification for such exception.

[cubbimew]

related #721

[adambadura]

What is the procedure here? Should I propose rewording to that point or someone else is reviewing such issues and applying (or not) changes to the text?

[strpbrk]

This is bothering me, too. I have also read discussions about it, but mostly only saw difference in opinions.

Given that C.35 allows (and requires) a non-virtual protected destructor to be used, when I see a destructor declaration such as ~Derived(), I would like to know whether it is virtual or not without having to look at the base class.

And what's even more important, if I switch the ~Base to protected non-virtual, the compiler won't flag anything on the ~Derived. If there was override, the derived destructor would be flagged. (In this case maybe Derived is yet another abstract class that augments the Base, and I want to use it with std::unique_ptr<Derived>).

Before someone says that pure abstract classes (interfaces) should only have public virtual destructors, I would like to point again at C.35. Does destruction always have to be exposed in an interface?

// Whatever the interface here may be...
Article * Newspaper::get_article();
Article & Newspaper::get_article();
shared_ptr<Article> Newspaper::get_article();

class Article {
public:
    // Yes, you may vandalize the text...
    virtual Text * get_text() = 0;

protected:
    // ... but don't think this will remove the article... use Newspaper::erase!
    ~Article() = default;
};

Hopefully I make some sense, and this was not a waste of your time. Thanks.

[gdr-at-ms]

Fixed.

[adambadura]

Now the point states:

If a base class destructor is declared virtual, one should avoid declaring derived class destructors virtual or override. Some code base and tools might insist on override for destructors, but that is not the recommendation of these guidelines.

How is this "fixing" the issue? Still no reasoning is provided for skipping override.

[SR-gh]

@adambadura I do agree.
In addition to a reasoning [1], what is the proposed replacement of these guidelines in order to have a way to express :

_This destructor is intended to already be virtual, and if it happens it is not yet, this is an error._

[1] The obvious reason could be that no base class function has the same name as the derived class destructor name. So no base class function with the derived class destructor name can be given a definition. Consequently no overriding of the definition of a base class fuction having the same name as the derived class destructor name can occur. But even if it seems logical, the definition of overriding has been modified to include the case of the destructor (13.3 - 6). Is it a defect according to these guidelines ?

[MikeGitb]

@SR-gh: The issue #721 that was linked by cubbimew contains a lengthy discussion on the subject and why the maintainers think this is the right way to go.

Still, it would be nice to have the reasoning in the guidelines and not just spread over some github issues.

[SR-gh]

@MikeGitb Issue #721 raises the problem. The problem is said to be fixed in this issue by adding a repetition of the rule (https://github.com/isocpp/CppCoreGuidelines/commit/918a5695c7c18c7c96a77b1e36fa2789470a813d#diff-feb71ecadc563b52e66838adbd6b8e30).
An explanation to the reason leading to this rule would be welcome.

[beinhaerter]

@strpbrk

I would like to know whether it is virtual or not without having to look at the base class.
Why? What difference does it make?

In the first place I thought that it is a good idea to enforce virtual/override/final for dtors. But since you pointed to C.35 I am in doubt and think that enforcing C.35 might be sufficient.

[barkatmoon]

@beinhaerter I would argue that the same can be asked about using "override" on any virtual function. It makes difference while I'm developing the class, while checking for consistency, and especially while and after refactoring it. I check for "protected" vs "public" on it, and I check whether it is virtual or not, and I consider that both of these features should be clearly expressed.

Putting "override" on the destructor makes compiler catch the anti-example given in C.35. To me, even this is enough of a reason to use it.