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 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( "cppcoreguidelines-pro-type-vararg"); + CheckFactories.registerCheck( + "cppcoreguidelines-rule-of-five-and-zero"); CheckFactories.registerCheck( "cppcoreguidelines-c-copy-assignment-signature"); } 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; + + using ClassDefiningSpecialMembersMap = std::map>; + +private: + + static llvm::StringRef toString(SpecialMemberFunctionKind K); + + static std::string join(llvm::ArrayRef 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 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("class-def"); + if (!MatchedDecl) + return; + + ClassDefId ID(MatchedDecl->getLocation(), MatchedDecl->getName()); + + if (Result.Nodes.getNodeAs("dtor")) { + ClassWithSpecialMembers[ID].push_back( + SpecialMemberFunctionKind::Destructor); + } + if (Result.Nodes.getNodeAs("copy-ctor")) { + ClassWithSpecialMembers[ID].push_back( + SpecialMemberFunctionKind::CopyConstructor); + } + if (Result.Nodes.getNodeAs("copy-assign")) { + ClassWithSpecialMembers[ID].push_back( + SpecialMemberFunctionKind::CopyAssignment); + } + if (Result.Nodes.getNodeAs("move-ctor")) { + ClassWithSpecialMembers[ID].push_back( + SpecialMemberFunctionKind::MoveConstructor); + } + if (Result.Nodes.getNodeAs("move-assign")) { + ClassWithSpecialMembers[ID].push_back( + SpecialMemberFunctionKind::MoveAssignment); + } +} + +void RuleOfFiveAndZeroCheck::onEndOfTranslationUnit() { + llvm::SmallVector 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 DefinedSpecialMembers = C.second; + + if (DefinedSpecialMembers.size() == AllSpecialMembers.size()) + continue; + + llvm::SmallVector 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: 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: 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: 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: 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; +};