Add AllowMissingMoveFunctionsWhenCopyIsDeleted flag to cppcoreguidelines-special-member-functions

The flag allows classes to don't define move operations when copy operations
are explicitly deleted. This flag is related to Google C++ Style Guide
https://google.github.io/styleguide/cppguide.html#Copyable_Movable_Types

Author: Paweł Barań

clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp

@@ -25,12 +25,16 @@ SpecialMemberFunctionsCheck::SpecialMemberFunctionsCheck(
     StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       AllowMissingMoveFunctions(Options.get("AllowMissingMoveFunctions", 0)),
-      AllowSoleDefaultDtor(Options.get("AllowSoleDefaultDtor", 0)) {}
+      AllowSoleDefaultDtor(Options.get("AllowSoleDefaultDtor", 0)),
+      AllowMissingMoveFunctionsWhenCopyIsDeleted(
+          Options.get("AllowMissingMoveFunctionsWhenCopyIsDeleted", 0)) {}
 
 void SpecialMemberFunctionsCheck::storeOptions(
     ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "AllowMissingMoveFunctions", AllowMissingMoveFunctions);
   Options.store(Opts, "AllowSoleDefaultDtor", AllowSoleDefaultDtor);
+  Options.store(Opts, "AllowMissingMoveFunctionsWhenCopyIsDeleted",
+                AllowMissingMoveFunctionsWhenCopyIsDeleted);
 }
 
 void SpecialMemberFunctionsCheck::registerMatchers(MatchFinder *Finder) {
@@ -103,17 +107,18 @@ void SpecialMemberFunctionsCheck::check(
 
   ClassDefId ID(MatchedDecl->getLocation(), std::string(MatchedDecl->getName()));
 
-  auto StoreMember = [this, &ID](SpecialMemberFunctionKind Kind) {
-    llvm::SmallVectorImpl<SpecialMemberFunctionKind> &Members =
+  auto StoreMember = [this, &ID](SpecialMemberFunctionData data) {
+    llvm::SmallVectorImpl<SpecialMemberFunctionData> &Members =
         ClassWithSpecialMembers[ID];
-    if (!llvm::is_contained(Members, Kind))
-      Members.push_back(Kind);
+    if (!llvm::is_contained(Members, data))
+      Members.push_back(std::move(data));
   };
 
   if (const auto *Dtor = Result.Nodes.getNodeAs<CXXMethodDecl>("dtor")) {
-    StoreMember(Dtor->isDefaulted()
-                    ? SpecialMemberFunctionKind::DefaultDestructor
-                    : SpecialMemberFunctionKind::NonDefaultDestructor);
+    StoreMember({Dtor->isDefaulted()
+                     ? SpecialMemberFunctionKind::DefaultDestructor
+                     : SpecialMemberFunctionKind::NonDefaultDestructor,
+                 Dtor->isDeleted()});
   }
 
   std::initializer_list<std::pair<std::string, SpecialMemberFunctionKind>>
@@ -123,8 +128,9 @@ void SpecialMemberFunctionsCheck::check(
                   {"move-assign", SpecialMemberFunctionKind::MoveAssignment}};
 
   for (const auto &KV : Matchers)
-    if (Result.Nodes.getNodeAs<CXXMethodDecl>(KV.first)) {
-      StoreMember(KV.second);
+    if (const auto *MethodDecl =
+            Result.Nodes.getNodeAs<CXXMethodDecl>(KV.first)) {
+      StoreMember({KV.second, MethodDecl->isDeleted()});
     }
 }
 
@@ -136,11 +142,19 @@ void SpecialMemberFunctionsCheck::onEndOfTranslationUnit() {
 
 void SpecialMemberFunctionsCheck::checkForMissingMembers(
     const ClassDefId &ID,
-    llvm::ArrayRef<SpecialMemberFunctionKind> DefinedMembers) {
+    llvm::ArrayRef<SpecialMemberFunctionData> DefinedMembers) {
   llvm::SmallVector<SpecialMemberFunctionKind, 5> MissingMembers;
 
   auto HasMember = [&](SpecialMemberFunctionKind Kind) {
-    return llvm::is_contained(DefinedMembers, Kind);
+    return llvm::any_of(DefinedMembers, [Kind](const auto &data) {
+      return data.FunctionKind == Kind;
+    });
+  };
+
+  auto IsDeleted = [&](SpecialMemberFunctionKind Kind) {
+    return llvm::any_of(DefinedMembers, [Kind](const auto &data) {
+      return data.FunctionKind == Kind && data.IsDeleted;
+    });
   };
 
   auto RequireMember = [&](SpecialMemberFunctionKind Kind) {
@@ -171,16 +185,23 @@ void SpecialMemberFunctionsCheck::checkForMissingMembers(
     RequireMember(SpecialMemberFunctionKind::CopyAssignment);
   }
 
-  if (RequireFive) {
+  if (RequireFive &&
+      !(AllowMissingMoveFunctionsWhenCopyIsDeleted &&
+        (IsDeleted(SpecialMemberFunctionKind::CopyConstructor) &&
+         IsDeleted(SpecialMemberFunctionKind::CopyAssignment)))) {
     assert(RequireThree);
     RequireMember(SpecialMemberFunctionKind::MoveConstructor);
     RequireMember(SpecialMemberFunctionKind::MoveAssignment);
   }
 
-  if (!MissingMembers.empty())
+  if (!MissingMembers.empty()) {
+    llvm::SmallVector<SpecialMemberFunctionKind, 5> DefinedMemberKinds;
+    llvm::transform(DefinedMembers, std::back_inserter(DefinedMemberKinds),
+                    [](const auto &data) { return data.FunctionKind; });
     diag(ID.first, "class '%0' defines %1 but does not define %2")
-        << ID.second << cppcoreguidelines::join(DefinedMembers, " and ")
+        << ID.second << cppcoreguidelines::join(DefinedMemberKinds, " and ")
         << cppcoreguidelines::join(MissingMembers, " or ");
+  }
 }
 
 } // namespace cppcoreguidelines

clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h

@@ -43,19 +43,30 @@ class SpecialMemberFunctionsCheck : public ClangTidyCheck {
     MoveAssignment
   };
 
+  struct SpecialMemberFunctionData {
+    SpecialMemberFunctionKind FunctionKind;
+    bool IsDeleted;
+
+    bool operator==(const SpecialMemberFunctionData &Other) {
+      return (Other.FunctionKind == FunctionKind) &&
+             (Other.IsDeleted == IsDeleted);
+    }
+  };
+
   using ClassDefId = std::pair<SourceLocation, std::string>;
 
   using ClassDefiningSpecialMembersMap =
       llvm::DenseMap<ClassDefId,
-                     llvm::SmallVector<SpecialMemberFunctionKind, 5>>;
+                     llvm::SmallVector<SpecialMemberFunctionData, 5>>;
 
 private:
   void checkForMissingMembers(
       const ClassDefId &ID,
-      llvm::ArrayRef<SpecialMemberFunctionKind> DefinedSpecialMembers);
+      llvm::ArrayRef<SpecialMemberFunctionData> DefinedSpecialMembers);
 
   const bool AllowMissingMoveFunctions;
   const bool AllowSoleDefaultDtor;
+  const bool AllowMissingMoveFunctionsWhenCopyIsDeleted;
   ClassDefiningSpecialMembersMap ClassWithSpecialMembers;
 };
 

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst

@@ -46,4 +46,19 @@ Options
        A(const A&);
        A& operator=(const A&);
        ~A();
-     }
+     };
+
+.. option:: AllowMissingMoveFunctionsWhenCopyIsDeleted
+
+   When set to `1` (default is `0`), this check doesn't flag classes which define deleted copy
+   operations but don't define move operations. This flags is related to Google C++ Style Guide
+   https://google.github.io/styleguide/cppguide.html#Copyable_Movable_Types. With this option enabled, the 
+   following class won't be flagged:
+   
+   .. code-block:: c++
+   
+     struct A {
+       A(const A&) = delete;
+       A& operator=(const A&) = delete;
+       ~A();
+     };

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-special-member-functions-allow-missing-move-when-copy-is-deleted.cpp

@@ -0,0 +1,49 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-special-member-functions %t -- -config="{CheckOptions: [{key: cppcoreguidelines-special-member-functions.AllowMissingMoveFunctionsWhenCopyIsDeleted, value: 1}]}" --
+
+class DefinesEverything {
+  DefinesEverything(const DefinesEverything &);
+  DefinesEverything(DefinesEverything &&);
+  DefinesEverything &operator=(const DefinesEverything &);
+  DefinesEverything &operator=(DefinesEverything &&);
+  ~DefinesEverything();
+};
+
+class DefinesNothing {
+};
+
+class DeletedCopyCtorAndOperator {
+  ~DeletedCopyCtorAndOperator() = default;
+  DeletedCopyCtorAndOperator(const DeletedCopyCtorAndOperator &) = delete;
+  DeletedCopyCtorAndOperator &operator=(const DeletedCopyCtorAndOperator &) = delete;
+};
+
+// CHECK-MESSAGES: [[@LINE+1]]:7: warning: class 'DefaultedCopyCtorAndOperator' defines a default destructor, a copy constructor and a copy assignment operator but does not define a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+class DefaultedCopyCtorAndOperator {
+  ~DefaultedCopyCtorAndOperator() = default;
+  DefaultedCopyCtorAndOperator(const DefaultedCopyCtorAndOperator &) = default;
+  DefaultedCopyCtorAndOperator &operator=(const DefaultedCopyCtorAndOperator &) = default;
+};
+
+// CHECK-MESSAGES: [[@LINE+1]]:7: warning: class 'DefinedCopyCtorAndOperator' defines a default destructor, a copy constructor and a copy assignment operator but does not define a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+class DefinedCopyCtorAndOperator {
+  ~DefinedCopyCtorAndOperator() = default;
+  DefinedCopyCtorAndOperator(const DefinedCopyCtorAndOperator &);
+  DefinedCopyCtorAndOperator &operator=(const DefinedCopyCtorAndOperator &);
+};
+
+// CHECK-MESSAGES: [[@LINE+1]]:7: warning: class 'MissingCopyCtor' defines a default destructor and a copy assignment operator but does not define a copy constructor, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+class MissingCopyCtor {
+  ~MissingCopyCtor() = default;
+  MissingCopyCtor &operator=(const MissingCopyCtor &) = delete;
+};
+
+// CHECK-MESSAGES: [[@LINE+1]]:7:  warning: class 'MissingCopyOperator' defines a default destructor and a copy constructor but does not define a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+class MissingCopyOperator {
+  ~MissingCopyOperator() = default;
+  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;
+};