Page MenuHomePhabricator

[clang-tidy] Special member functions check should allow sole dtors by default
AbandonedPublic

Authored by steakhal on Jun 2 2021, 3:15 AM.

Details

Summary

The cppcoreguidelines-special-member-functions check has the AllowSoleDefaultDtor checker option.
It is false by default, and now I'm proposing to change it to true.
Our CodeChecker users frequently find it surprising that the check reports for cases like this by default:

struct A {
  virtual ~A() = default;
};

Quoting from the Cpp CoreGuidelines C.21: If you define or =delete any copy, move, or destructor function, define or =delete them all:

Example, good When a destructor needs to be declared just to make it virtual, it can be defined as defaulted.

class AbstractBase {
public:
    virtual ~AbstractBase() = default;
    // ...
};

Diff Detail

Event Timeline

steakhal created this revision.Jun 2 2021, 3:15 AM
steakhal requested review of this revision.Jun 2 2021, 3:15 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 2 2021, 3:15 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript

I think the C++ Core Guideline wording is... confusing. The rule title is C.21: If you define or =delete any copy, move, or destructor function, define or =delete them all, which I take literally. Using = default defines the function, so:

struct S {
  virtual ~S() = default; // This defines the dtor
};

Because this defines the dtor, all the rest need to be defined as well per the rule. The enforcement on the rule says: Enforcement (Simple) A class should have a declaration (even a =delete one) for either all or none of the copy/move/destructor functions. and so it seems like the rule is requiring us to diagnose this case by default (pun retroactively intended), and so the option should remain as it is. In the code example you posted from the rule, I imagine that to be a load-bearing // ... that defines all the rest of the members.

Perhaps the C++ Core Guideline authors can be enticed to update the rule to be more clear?

I think the C++ Core Guideline wording is... confusing. The rule title is C.21: If you define or =delete any copy, move, or destructor function, define or =delete them all, which I take literally. Using = default defines the function, so:

Okay, then I'm going to abandon this in a few days if nothing interesting shows up.
Thanks for the quick response!

steakhal abandoned this revision.Fri, Nov 12, 8:34 AM

I forgot to abandon this earlier.