[PATCH] D103511: [clang-tidy] Special member functions check should allow sole dtors by default

[Balázs Benics via Phabricator via cfe-commits]

steakhal created this revision.
steakhal added reviewers: aaron.ballman, njames93, steveire.
Herald added subscribers: martong, kbarton, xazax.hun, whisperity, nemanjai.
steakhal requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

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 <https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c21-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;
>       // ...
>   };


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103511

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
  clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
  clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-special-member-functions-allow-missing-move-when-copy-is-deleted.cpp
  clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-special-member-functions.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-special-member-functions.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-special-member-functions.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-special-member-functions.cpp
@@ -1,5 +1,4 @@
-// RUN: %check_clang_tidy %s cppcoreguidelines-special-member-functions %t
-
+// RUN: %check_clang_tidy %s cppcoreguidelines-special-member-functions %t -- -config="{CheckOptions: [{key: cppcoreguidelines-special-member-functions.AllowSoleDefaultDtor, value: false}]}" --
 class DefinesDestructor {
   ~DefinesDestructor();
 };
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-special-member-functions-allow-missing-move-when-copy-is-deleted.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-special-member-functions-allow-missing-move-when-copy-is-deleted.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-special-member-functions-allow-missing-move-when-copy-is-deleted.cpp
@@ -43,7 +43,6 @@
   MissingCopyOperator(const MissingCopyOperator &) = delete;
 };
 
-// CHECK-MESSAGES: [[@LINE+1]]:7:  warning: class 'MissingAll' defines a default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
 class MissingAll {
   ~MissingAll() = default;
 };
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
+++ clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
@@ -25,7 +25,7 @@
 
 .. option:: AllowSoleDefaultDtor
 
-   When set to `true` (default is `false`), this check doesn't flag classes with a sole, explicitly
+   When set to `true` (default is `true`), this check doesn't flag classes with a sole, explicitly
    defaulted destructor. An example for such a class is:
    
    .. code-block:: c++
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
@@ -25,7 +25,7 @@
     StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context), AllowMissingMoveFunctions(Options.get(
                                          "AllowMissingMoveFunctions", false)),
-      AllowSoleDefaultDtor(Options.get("AllowSoleDefaultDtor", false)),
+      AllowSoleDefaultDtor(Options.get("AllowSoleDefaultDtor", true)),
       AllowMissingMoveFunctionsWhenCopyIsDeleted(
           Options.get("AllowMissingMoveFunctionsWhenCopyIsDeleted", false)) {}
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D103511.349211.patch
Type: text/x-patch
Size: 3076 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210602/a9b0e526/attachment-0001.bin>