Index: clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h =================================================================== --- clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h +++ clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h @@ -25,14 +25,16 @@ /// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-special-member-functions.html class SpecialMemberFunctionsCheck : public ClangTidyCheck { public: - SpecialMemberFunctionsCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + SpecialMemberFunctionsCheck(StringRef Name, ClangTidyContext *Context); + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; void onEndOfTranslationUnit() override; enum class SpecialMemberFunctionKind : uint8_t { Destructor, + DefaultDestructor, + NonDefaultDestructor, CopyConstructor, CopyAssignment, MoveConstructor, @@ -46,6 +48,12 @@ llvm::SmallVector>; private: + void checkForMissingMembers( + const ClassDefId &ID, + llvm::ArrayRef DefinedSpecialMembers); + + const bool AllowMissingMoveFunctions; + const bool AllowSoleDefaultDtor; ClassDefiningSpecialMembersMap ClassWithSpecialMembers; }; Index: clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp =================================================================== --- clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp +++ clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp @@ -22,6 +22,18 @@ namespace tidy { namespace cppcoreguidelines { +SpecialMemberFunctionsCheck::SpecialMemberFunctionsCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + AllowMissingMoveFunctions(Options.get("AllowMissingMoveFunctions", 0)), + AllowSoleDefaultDtor(Options.get("AllowSoleDefaultDtor", 0)) {} + +void SpecialMemberFunctionsCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "AllowMissingMoveFunctions", AllowMissingMoveFunctions); + Options.store(Opts, "AllowSoleDefaultDtor", AllowSoleDefaultDtor); +} + void SpecialMemberFunctionsCheck::registerMatchers(MatchFinder *Finder) { if (!getLangOpts().CPlusPlus) return; @@ -48,6 +60,12 @@ switch (K) { case SpecialMemberFunctionsCheck::SpecialMemberFunctionKind::Destructor: return "a destructor"; + case SpecialMemberFunctionsCheck::SpecialMemberFunctionKind:: + DefaultDestructor: + return "a default destructor"; + case SpecialMemberFunctionsCheck::SpecialMemberFunctionKind:: + NonDefaultDestructor: + return "a non-default destructor"; case SpecialMemberFunctionsCheck::SpecialMemberFunctionKind::CopyConstructor: return "a copy constructor"; case SpecialMemberFunctionsCheck::SpecialMemberFunctionKind::CopyAssignment: @@ -88,51 +106,86 @@ ClassDefId ID(MatchedDecl->getLocation(), MatchedDecl->getName()); + auto StoreMember = [this, &ID](SpecialMemberFunctionKind Kind) { + llvm::SmallVectorImpl &Members = + ClassWithSpecialMembers[ID]; + if (!llvm::is_contained(Members, Kind)) + Members.push_back(Kind); + }; + + if (const auto *Dtor = Result.Nodes.getNodeAs("dtor")) { + StoreMember(Dtor->isDefaulted() + ? SpecialMemberFunctionKind::DefaultDestructor + : SpecialMemberFunctionKind::NonDefaultDestructor); + } + std::initializer_list> - Matchers = {{"dtor", SpecialMemberFunctionKind::Destructor}, - {"copy-ctor", SpecialMemberFunctionKind::CopyConstructor}, + Matchers = {{"copy-ctor", SpecialMemberFunctionKind::CopyConstructor}, {"copy-assign", SpecialMemberFunctionKind::CopyAssignment}, {"move-ctor", SpecialMemberFunctionKind::MoveConstructor}, {"move-assign", SpecialMemberFunctionKind::MoveAssignment}}; for (const auto &KV : Matchers) if (Result.Nodes.getNodeAs(KV.first)) { - SpecialMemberFunctionKind Kind = KV.second; - llvm::SmallVectorImpl &Members = - ClassWithSpecialMembers[ID]; - if (find(Members, Kind) == Members.end()) - Members.push_back(Kind); + StoreMember(KV.second); } } void SpecialMemberFunctionsCheck::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) { + checkForMissingMembers(C.first, C.second); } +} - for (const auto &C : ClassWithSpecialMembers) { - const auto &DefinedSpecialMembers = C.second; +void SpecialMemberFunctionsCheck::checkForMissingMembers( + const ClassDefId &ID, + llvm::ArrayRef DefinedMembers) { + llvm::SmallVector MissingMembers; + + auto HasMember = [&](SpecialMemberFunctionKind Kind) { + return llvm::is_contained(DefinedMembers, Kind); + }; + + auto RequireMember = [&](SpecialMemberFunctionKind Kind) { + if (!HasMember(Kind)) + MissingMembers.push_back(Kind); + }; + + bool RequireThree = + HasMember(SpecialMemberFunctionKind::NonDefaultDestructor) || + (!AllowSoleDefaultDtor && + HasMember(SpecialMemberFunctionKind::DefaultDestructor)) || + HasMember(SpecialMemberFunctionKind::CopyConstructor) || + HasMember(SpecialMemberFunctionKind::CopyAssignment) || + HasMember(SpecialMemberFunctionKind::MoveConstructor) || + HasMember(SpecialMemberFunctionKind::MoveAssignment); + + bool RequireFive = (!AllowMissingMoveFunctions && RequireThree && + getLangOpts().CPlusPlus11) || + HasMember(SpecialMemberFunctionKind::MoveConstructor) || + HasMember(SpecialMemberFunctionKind::MoveAssignment); + + if (RequireThree) { + if (!HasMember(SpecialMemberFunctionKind::DefaultDestructor) && + !HasMember(SpecialMemberFunctionKind::NonDefaultDestructor)) + MissingMembers.push_back(SpecialMemberFunctionKind::Destructor); - if (DefinedSpecialMembers.size() == AllSpecialMembers.size()) - continue; + RequireMember(SpecialMemberFunctionKind::CopyConstructor); + RequireMember(SpecialMemberFunctionKind::CopyAssignment); + } - 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 "); + if (RequireFive) { + assert(RequireThree); + RequireMember(SpecialMemberFunctionKind::MoveConstructor); + RequireMember(SpecialMemberFunctionKind::MoveAssignment); } + + if (!MissingMembers.empty()) + diag(ID.first, "class '%0' defines %1 but does not define %2") + << ID.second << join(DefinedMembers, " and ") + << join(MissingMembers, " or "); } + } // namespace cppcoreguidelines } // namespace tidy } // namespace clang Index: docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst =================================================================== --- docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst +++ docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst @@ -19,3 +19,31 @@ 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. + +Options +------- + +.. option:: AllowSoleDefaultDtor + + When set to `1` (default is `0`), this check doesn't flag classes with a sole, explicitly + defaulted destructor. An example for such a class is: + + .. code-block:: c++ + + struct A { + virtual ~A() = default; + }; + +.. option:: AllowMissingMoveFunctions + + When set to `1` (default is `0`), this check doesn't flag classes which define no move + operations at all. It still flags classes which define only one of either + move constructor or move assignment operator. With this option enabled, the following class won't be flagged: + + .. code-block:: c++ + + struct A { + A(const A&); + A& operator=(const A&); + ~A(); + } Index: test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp =================================================================== --- test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp +++ test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp @@ -3,7 +3,7 @@ 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-special-member-functions] +// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a non-default destructor but does not define a copy constructor or a copy assignment operator [cppcoreguidelines-special-member-functions] class DefinesCopyConstructor { DefinesCopyConstructor(const DefinesCopyConstructor &); Index: test/clang-tidy/cppcoreguidelines-special-member-functions.cpp =================================================================== --- test/clang-tidy/cppcoreguidelines-special-member-functions.cpp +++ test/clang-tidy/cppcoreguidelines-special-member-functions.cpp @@ -3,7 +3,12 @@ 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-special-member-functions] +// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a non-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 DefinesDefaultedDestructor { + ~DefinesDefaultedDestructor() = default; +}; +// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDefaultedDestructor' 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 DefinesCopyConstructor { DefinesCopyConstructor(const DefinesCopyConstructor &);