jbcoe updated this revision to Diff 65096.
jbcoe marked 6 inline comments as done.
jbcoe added a comment.
Address comments from review, thanks for those.
https://reviews.llvm.org/D22513
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-cxx-03.cpp
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,52 @@
+// 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 a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [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 a destructor, a copy assignment operator, a move constructor or a move assignment operator [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 a destructor, a copy constructor, a move constructor or a move assignment operator [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 a destructor, a copy constructor, a copy assignment operator or a move assignment operator [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 a destructor, a copy constructor, a copy assignment operator or a move constructor [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 &) = delete;
+ DeletesEverything &operator=(const DeletesEverything &) = delete;
+ DeletesEverything(DeletesEverything &&) = delete;
+ DeletesEverything &operator=(DeletesEverything &&) = delete;
+ ~DeletesEverything() = delete;
+};
+
+class DeletesCopyDefaultsMove {
+ DeletesCopyDefaultsMove(const DeletesCopyDefaultsMove &) = delete;
+ DeletesCopyDefaultsMove &operator=(const DeletesCopyDefaultsMove &) = delete;
+ DeletesCopyDefaultsMove(DeletesCopyDefaultsMove &&) = default;
+ DeletesCopyDefaultsMove &operator=(DeletesCopyDefaultsMove &&) = default;
+ ~DeletesCopyDefaultsMove() = default;
+};
Index: test/clang-tidy/cppcoreguidelines-rule-of-five-and-zero-cxx-03.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-rule-of-five-and-zero-cxx-03.cpp
@@ -0,0 +1,26 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-rule-of-five-and-zero %t -- -- -std=c++03
+
+class DefinesDestructor {
+ ~DefinesDestructor();
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a destructor but does not define a copy constructor or a copy assignment operator [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 a destructor or a copy assignment operator [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 a destructor or a copy constructor [cppcoreguidelines-rule-of-five-and-zero]
+
+class DefinesNothing {
+};
+
+class DefinesEverything {
+ DefinesEverything(const DefinesEverything &);
+ DefinesEverything &operator=(const DefinesEverything &);
+ ~DefinesEverything();
+};
+
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 explicit 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,58 @@
+//===--- 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;
+ void onEndOfTranslationUnit() override;
+
+ enum class SpecialMemberFunctionKind {
+ Destructor,
+ CopyConstructor,
+ CopyAssignment,
+ MoveConstructor,
+ MoveAssignment
+ };
+
+ using ClassDefId = std::pair<SourceLocation, std::string>;
+
+ using ClassDefiningSpecialMembersMap = std::map<ClassDefId, llvm::SmallVector<SpecialMemberFunctionKind, 5>>;
+
+private:
+
+ static llvm::StringRef toString(SpecialMemberFunctionKind K);
+
+ static std::string join(llvm::ArrayRef<SpecialMemberFunctionKind> SMFS,
+ llvm::StringRef AndOr);
+
+ ClassDefiningSpecialMembersMap ClassWithSpecialMembers;
+};
+
+} // 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,137 @@
+//===--- 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"
+
+#define DEBUG_TYPE "clang-tidy"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+void RuleOfFiveAndZeroCheck::registerMatchers(MatchFinder *Finder) {
+ if (!getLangOpts().CPlusPlus)
+ return;
+ Finder->addMatcher(
+ cxxRecordDecl(
+ eachOf(
+ has(cxxDestructorDecl(unless(isImplicit())).bind("dtor")),
+ has(cxxConstructorDecl(isCopyConstructor(), unless(isImplicit()))
+ .bind("copy-ctor")),
+ has(cxxMethodDecl(isCopyAssignmentOperator(),
+ unless(isImplicit()))
+ .bind("copy-assign")),
+ has(cxxConstructorDecl(isMoveConstructor(), unless(isImplicit()))
+ .bind("move-ctor")),
+ has(cxxMethodDecl(isMoveAssignmentOperator(),
+ unless(isImplicit()))
+ .bind("move-assign"))))
+ .bind("class-def"),
+ this);
+}
+
+llvm::StringRef RuleOfFiveAndZeroCheck::toString(
+ RuleOfFiveAndZeroCheck::SpecialMemberFunctionKind K) {
+ switch (K) {
+ case SpecialMemberFunctionKind::Destructor:
+ return "a destructor";
+ case SpecialMemberFunctionKind::CopyConstructor:
+ return "a copy constructor";
+ case SpecialMemberFunctionKind::CopyAssignment:
+ return "a copy assignment operator";
+ case SpecialMemberFunctionKind::MoveConstructor:
+ return "a move constructor";
+ case SpecialMemberFunctionKind::MoveAssignment:
+ return "a move assignment operator";
+ }
+}
+
+std::string
+RuleOfFiveAndZeroCheck::join(llvm::ArrayRef<SpecialMemberFunctionKind> SMFS,
+ llvm::StringRef AndOr) {
+ assert(!SMFS.empty());
+ std::string Buffer;
+ llvm::raw_string_ostream Stream(Buffer);
+
+ Stream << toString(SMFS[0]);
+ size_t LastIndex = SMFS.size() - 1;
+ for (size_t i = 1; i < LastIndex; ++i) {
+ Stream << ", " << toString(SMFS[i]);
+ }
+ if (LastIndex != 0) {
+ Stream << AndOr << toString(SMFS[LastIndex]);
+ }
+ return Stream.str();
+}
+
+void RuleOfFiveAndZeroCheck::check(const MatchFinder::MatchResult &Result) {
+ const CXXRecordDecl *MatchedDecl =
+ Result.Nodes.getNodeAs<CXXRecordDecl>("class-def");
+ if (!MatchedDecl)
+ return;
+
+ ClassDefId ID(MatchedDecl->getLocation(), MatchedDecl->getName());
+
+ if (Result.Nodes.getNodeAs<CXXDestructorDecl>("dtor")) {
+ ClassWithSpecialMembers[ID].push_back(
+ SpecialMemberFunctionKind::Destructor);
+ }
+ if (Result.Nodes.getNodeAs<CXXConstructorDecl>("copy-ctor")) {
+ ClassWithSpecialMembers[ID].push_back(
+ SpecialMemberFunctionKind::CopyConstructor);
+ }
+ if (Result.Nodes.getNodeAs<CXXMethodDecl>("copy-assign")) {
+ ClassWithSpecialMembers[ID].push_back(
+ SpecialMemberFunctionKind::CopyAssignment);
+ }
+ if (Result.Nodes.getNodeAs<CXXConstructorDecl>("move-ctor")) {
+ ClassWithSpecialMembers[ID].push_back(
+ SpecialMemberFunctionKind::MoveConstructor);
+ }
+ if (Result.Nodes.getNodeAs<CXXMethodDecl>("move-assign")) {
+ ClassWithSpecialMembers[ID].push_back(
+ SpecialMemberFunctionKind::MoveAssignment);
+ }
+}
+
+void RuleOfFiveAndZeroCheck::onEndOfTranslationUnit() {
+ llvm::SmallVector<SpecialMemberFunctionKind, 5> AllSpecialMembers = {
+ SpecialMemberFunctionKind::Destructor,
+ SpecialMemberFunctionKind::CopyConstructor,
+ SpecialMemberFunctionKind::CopyAssignment};
+
+ if (getLangOpts().CPlusPlus11) {
+ AllSpecialMembers.push_back(SpecialMemberFunctionKind::MoveConstructor);
+ AllSpecialMembers.push_back(SpecialMemberFunctionKind::MoveAssignment);
+ }
+
+ for (const auto &C : ClassWithSpecialMembers) {
+ ArrayRef<SpecialMemberFunctionKind> DefinedSpecialMembers = C.second;
+
+ if (DefinedSpecialMembers.size() == AllSpecialMembers.size())
+ continue;
+
+ llvm::SmallVector<SpecialMemberFunctionKind, 5> UndefinedSpecialMembers;
+ std::set_difference(AllSpecialMembers.begin(), AllSpecialMembers.end(),
+ DefinedSpecialMembers.begin(),
+ DefinedSpecialMembers.end(),
+ std::back_inserter(UndefinedSpecialMembers));
+
+ diag(C.first.first, "class '%0' defines %1 but does not define %2")
+ << C.first.second << join(DefinedSpecialMembers, " and ")
+ << join(UndefinedSpecialMembers, " or ");
+ }
+}
+} // 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
