jbcoe retitled this revision from "clang-tidy check: 
misc-deprecated-special-member-functions" to "clang-tidy check: 
cppcoreguidelines-rule-of-five-and-zero".
jbcoe updated the summary for this revision.
jbcoe updated this revision to Diff 64490.
jbcoe added a comment.
Herald added a subscriber: nemanjai.

I've rewritten this patch to implement a rule-of-five-and-zero check.

No fixes are offered as we've not found them to be useful.


https://reviews.llvm.org/D16376

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/RuleOfFiveAndZeroCheck.cpp
  clang-tidy/cppcoreguidelines/RuleOfFiveAndZeroCheck.h
  docs/clang-tidy/checks/cppcoreguidelines-rule-of-five-and-zero.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-rule-of-five-and-zero.cpp

Index: test/clang-tidy/cppcoreguidelines-rule-of-five-and-zero.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-rule-of-five-and-zero.cpp
@@ -0,0 +1,44 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-rule-of-five-and-zero %t
+
+class DefinesDestructor {
+  ~DefinesDestructor();
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a destructor but does not define or delete all other special member functions [cppcoreguidelines-rule-of-five-and-zero]
+
+class DefinesCopyConstructor {
+  DefinesCopyConstructor(const DefinesCopyConstructor &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyConstructor' defines a copy constructor but does not define or delete all other special member functions [cppcoreguidelines-rule-of-five-and-zero]
+class DefinesCopyAssignment {
+  DefinesCopyAssignment &operator=(const DefinesCopyAssignment &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyAssignment' defines a copy assignment operator but does not define or delete all other special member functions [cppcoreguidelines-rule-of-five-and-zero]
+
+class DefinesMoveConstructor {
+  DefinesMoveConstructor(DefinesMoveConstructor &&);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesMoveConstructor' defines a move constructor but does not define or delete all other special member functions [cppcoreguidelines-rule-of-five-and-zero]
+
+class DefinesMoveAssignment {
+  DefinesMoveAssignment &operator=(DefinesMoveAssignment &&);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesMoveAssignment' defines a move assignment operator but does not define or delete all other special member functions [cppcoreguidelines-rule-of-five-and-zero]
+
+class DefinesNothing {
+};
+
+class DefinesEverything {
+  DefinesEverything(const DefinesEverything &);
+  DefinesEverything &operator=(const DefinesEverything &);
+  DefinesEverything(DefinesEverything &&);
+  DefinesEverything &operator=(DefinesEverything &&);
+  ~DefinesEverything();
+};
+
+class DeletesEverything {
+  DeletesEverything(const DeletesEverything &);
+  DeletesEverything &operator=(const DeletesEverything &);
+  DeletesEverything(DeletesEverything &&);
+  DeletesEverything &operator=(DeletesEverything &&);
+  ~DeletesEverything();
+};
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -29,6 +29,7 @@
    cppcoreguidelines-pro-type-static-cast-downcast
    cppcoreguidelines-pro-type-union-access
    cppcoreguidelines-pro-type-vararg
+   cppcoreguidelines-rule-of-five-and-zero
    google-build-explicit-make-pair
    google-build-namespaces
    google-build-using-namespace
Index: docs/clang-tidy/checks/cppcoreguidelines-rule-of-five-and-zero.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/cppcoreguidelines-rule-of-five-and-zero.rst
@@ -0,0 +1,21 @@
+.. title:: clang-tidy - cppcoreguidelines-rule-of-five-and-zero
+
+cppcoreguidelines-rule-of-five-and-zero
+=======================================
+
+The check finds classes where some but not all of the special member functions
+are defined.
+
+By default the compiler defines a copy constructor, copy assignment operator,
+move constructor, move assignment operator and destructor. The default can be
+supressed by explciti user-definitions. The relationship between which
+functions will be supressed by definitions of other functions is complicated
+and it is advised that all five are defaulted or explicitly defined.
+
+Note that defining a function with ``=delete`` is considered to be a
+definition. 
+
+This rule is part of the "Constructors, assignments, and destructors" profile of the C++ Core
+Guidelines, corresponding to rule C.21. See
+
+https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c21-if-you-define-or-delete-any-default-operation-define-or-delete-them-all.
Index: clang-tidy/cppcoreguidelines/RuleOfFiveAndZeroCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/cppcoreguidelines/RuleOfFiveAndZeroCheck.h
@@ -0,0 +1,41 @@
+//===--- RuleOfFiveAndZeroCheck.h - clang-tidy-------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_RULE_OF_FIVE_AND_ZERO_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_RULE_OF_FIVE_AND_ZERO_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+/// Checks for classes where some, but not all, of the special member functions
+/// are defined.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-rule-of-five-and-zero.html
+class RuleOfFiveAndZeroCheck : public ClangTidyCheck {
+public:
+  RuleOfFiveAndZeroCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  void
+  checkRuleOfFiveViolation(const ast_matchers::MatchFinder::MatchResult &Result,
+                           llvm::StringRef Match, llvm::StringRef FunctionName);
+};
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_RULE_OF_FIVE_AND_ZERO_H
Index: clang-tidy/cppcoreguidelines/RuleOfFiveAndZeroCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/cppcoreguidelines/RuleOfFiveAndZeroCheck.cpp
@@ -0,0 +1,82 @@
+//===--- RuleOfFiveAndZeroCheck.cpp - clang-tidy---------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "RuleOfFiveAndZeroCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+void RuleOfFiveAndZeroCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus11)
+    return;
+
+  auto DefinesDestructor = has(cxxDestructorDecl(unless(isImplicit())));
+  auto DefinesCopyConstructor =
+      has(cxxConstructorDecl(isCopyConstructor(), unless(isImplicit())));
+  auto DefinesCopyAssignment =
+      has(cxxMethodDecl(isCopyAssignmentOperator(), unless(isImplicit())));
+  auto DefinesMoveConstructor =
+      has(cxxConstructorDecl(isMoveConstructor(), unless(isImplicit())));
+  auto DefinesMoveAssignment =
+      has(cxxMethodDecl(isMoveAssignmentOperator(), unless(isImplicit())));
+  auto DefinesAllSpecialMembers =
+      allOf(DefinesDestructor, DefinesCopyAssignment, DefinesCopyAssignment,
+            DefinesMoveConstructor, DefinesMoveAssignment);
+
+  Finder->addMatcher(
+      cxxRecordDecl(allOf(DefinesDestructor, unless(DefinesAllSpecialMembers)))
+          .bind("dtor"),
+      this);
+  Finder->addMatcher(cxxRecordDecl(allOf(DefinesCopyConstructor,
+                                         unless(DefinesAllSpecialMembers)))
+                         .bind("copy-ctor"),
+                     this);
+  Finder->addMatcher(cxxRecordDecl(allOf(DefinesCopyAssignment,
+                                         unless(DefinesAllSpecialMembers)))
+                         .bind("copy-assign"),
+                     this);
+  Finder->addMatcher(cxxRecordDecl(allOf(DefinesMoveConstructor,
+                                         unless(DefinesAllSpecialMembers)))
+                         .bind("move-ctor"),
+                     this);
+  Finder->addMatcher(cxxRecordDecl(allOf(DefinesMoveAssignment,
+                                         unless(DefinesAllSpecialMembers)))
+                         .bind("move-assign"),
+                     this);
+}
+
+void RuleOfFiveAndZeroCheck::checkRuleOfFiveViolation(
+    const MatchFinder::MatchResult &Result, llvm::StringRef Match,
+    llvm::StringRef FunctionName) {
+  if (const auto *MatchedDecl = Result.Nodes.getNodeAs<CXXRecordDecl>(Match)) {
+
+    diag(MatchedDecl->getLocation(), "class %0 defines a %1 but does not "
+                                     "define or delete all other special "
+                                     "member functions")
+        << MatchedDecl << FunctionName;
+  }
+}
+
+void RuleOfFiveAndZeroCheck::check(const MatchFinder::MatchResult &Result) {
+
+  checkRuleOfFiveViolation(Result, "dtor", "destructor");
+  checkRuleOfFiveViolation(Result, "copy-ctor", "copy constructor");
+  checkRuleOfFiveViolation(Result, "copy-assign", "copy assignment operator");
+  checkRuleOfFiveViolation(Result, "move-ctor", "move constructor");
+  checkRuleOfFiveViolation(Result, "move-assign", "move assignment operator");
+}
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
===================================================================
--- clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -22,6 +22,7 @@
 #include "ProTypeStaticCastDowncastCheck.h"
 #include "ProTypeUnionAccessCheck.h"
 #include "ProTypeVarargCheck.h"
+#include "RuleOfFiveAndZeroCheck.h"
 
 namespace clang {
 namespace tidy {
@@ -53,6 +54,8 @@
         "cppcoreguidelines-pro-type-union-access");
     CheckFactories.registerCheck<ProTypeVarargCheck>(
         "cppcoreguidelines-pro-type-vararg");
+    CheckFactories.registerCheck<RuleOfFiveAndZeroCheck>(
+        "cppcoreguidelines-rule-of-five-and-zero");
     CheckFactories.registerCheck<misc::UnconventionalAssignOperatorCheck>(
         "cppcoreguidelines-c-copy-assignment-signature");
   }
Index: clang-tidy/cppcoreguidelines/CMakeLists.txt
===================================================================
--- clang-tidy/cppcoreguidelines/CMakeLists.txt
+++ clang-tidy/cppcoreguidelines/CMakeLists.txt
@@ -13,6 +13,7 @@
   ProTypeStaticCastDowncastCheck.cpp
   ProTypeUnionAccessCheck.cpp
   ProTypeVarargCheck.cpp
+  RuleOfFiveAndZeroCheck.cpp
 
   LINK_LIBS
   clangAST
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to