diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp @@ -108,10 +108,14 @@ }; if (const auto *Dtor = Result.Nodes.getNodeAs("dtor")) { - StoreMember({Dtor->isDefaulted() - ? SpecialMemberFunctionKind::DefaultDestructor - : SpecialMemberFunctionKind::NonDefaultDestructor, - Dtor->isDeleted()}); + SpecialMemberFunctionKind DestructorType = + SpecialMemberFunctionKind::Destructor; + if (Dtor->isDefined()) { + DestructorType = Dtor->getDefinition()->isDefaulted() + ? SpecialMemberFunctionKind::DefaultDestructor + : SpecialMemberFunctionKind::NonDefaultDestructor; + } + StoreMember({DestructorType, Dtor->isDeleted()}); } std::initializer_list> @@ -158,7 +162,8 @@ bool RequireThree = HasMember(SpecialMemberFunctionKind::NonDefaultDestructor) || (!AllowSoleDefaultDtor && - HasMember(SpecialMemberFunctionKind::DefaultDestructor)) || + (HasMember(SpecialMemberFunctionKind::Destructor) || + HasMember(SpecialMemberFunctionKind::DefaultDestructor))) || HasMember(SpecialMemberFunctionKind::CopyConstructor) || HasMember(SpecialMemberFunctionKind::CopyAssignment) || HasMember(SpecialMemberFunctionKind::MoveConstructor) || @@ -170,7 +175,8 @@ HasMember(SpecialMemberFunctionKind::MoveAssignment); if (RequireThree) { - if (!HasMember(SpecialMemberFunctionKind::DefaultDestructor) && + if (!HasMember(SpecialMemberFunctionKind::Destructor) && + !HasMember(SpecialMemberFunctionKind::DefaultDestructor) && !HasMember(SpecialMemberFunctionKind::NonDefaultDestructor)) MissingMembers.push_back(SpecialMemberFunctionKind::Destructor); diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/special-member-functions.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/special-member-functions.rst --- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/special-member-functions.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/special-member-functions.rst @@ -25,15 +25,25 @@ .. option:: AllowSoleDefaultDtor - When set to `true` (default is `false`), this check doesn't flag classes with a sole, explicitly - defaulted destructor. An example for such a class is: + When set to `true` (default is `false`), this check will only trigger on + destructors if they are defined and not defaulted. .. code-block:: c++ - struct A { + struct A { // This is fine. virtual ~A() = default; }; + struct B { // This is not fine. + ~B() {} + }; + + struct C { + // This is not checked, because the destructor might be defaulted in + // another translation unit. + ~C(); + }; + .. option:: AllowMissingMoveFunctions When set to `true` (default is `false`), this check doesn't flag classes which define no move diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/special-member-functions-cxx-03.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/special-member-functions-cxx-03.cpp --- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/special-member-functions-cxx-03.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/special-member-functions-cxx-03.cpp @@ -3,7 +3,7 @@ class DefinesDestructor { ~DefinesDestructor(); }; -// 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] +// 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] class DefinesCopyConstructor { DefinesCopyConstructor(const DefinesCopyConstructor &); diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/special-member-functions-relaxed.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/special-member-functions-relaxed.cpp --- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/special-member-functions-relaxed.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/special-member-functions-relaxed.cpp @@ -1,14 +1,26 @@ // RUN: %check_clang_tidy %s cppcoreguidelines-special-member-functions %t -- -config="{CheckOptions: [{key: cppcoreguidelines-special-member-functions.AllowMissingMoveFunctions, value: true}, {key: cppcoreguidelines-special-member-functions.AllowSoleDefaultDtor, value: true}]}" -- +// Don't warn on destructors without definitions, they might be defaulted in another TU. +class DeclaresDestructor { + ~DeclaresDestructor(); +}; + class DefinesDestructor { ~DefinesDestructor(); }; -// 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] +DefinesDestructor::~DefinesDestructor() {} +// CHECK-MESSAGES: [[@LINE-4]]: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 DefinesDefaultedDestructor { ~DefinesDefaultedDestructor() = default; }; +class DefinesDefaultedDestructorOutside { + ~DefinesDefaultedDestructorOutside(); +}; + +DefinesDefaultedDestructorOutside::~DefinesDefaultedDestructorOutside() = default; + class DefinesCopyConstructor { DefinesCopyConstructor(const DefinesCopyConstructor &); }; diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/special-member-functions.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/special-member-functions.cpp --- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/special-member-functions.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/special-member-functions.cpp @@ -3,7 +3,7 @@ class DefinesDestructor { ~DefinesDestructor(); }; -// 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] +// 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] class DefinesDefaultedDestructor { ~DefinesDefaultedDestructor() = default;