isocpp / isocpp/CppCoreGuidelines

C.35 First enforcement is too broad, should only apply to base classes

Open
#1,841 5 comments 0 reactions 1 assignee View on GitHub

@GabrielDosReis is already working on this.

Since Nov 11, 2021.

Dominant language
CSS
Stars
45.3k
Forks
5.6k
PR merge metrics
No merged PRs in 30d

Description

Hi,

I believe the first bullet of the enforcement section of C.35 is too broad. The rule title says:

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

Note that it only talks about base classes.

However, the enforcement says:

A class with any virtual functions should have a destructor that is either public and virtual or else protected and non-virtual.

So it talks about any class instead of a base class. This is rather strict, in my opinion.

Consider the following example:

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

    protected:
        // We don't want Base to be used polymorphically. 
        // We just want to enforce that Derived classes implement the public interface.
        // Therefore, use protected non-virtual destructor
        ~Interface() = default;
};

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

In this example, C.35 would be violated for Derived - it contains a virtual function foo, and the destructor is public non-virtual. Does that make sense or should the enforcement only apply to base classes?

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.