[clang-tools-extra] r297671 - Add the 'AllowSoleDefaultDtor' and 'AllowMissingMoveFunctions' options to the cppcoreguidelines-special-member-functions check.
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 13 14:39:00 PDT 2017
Author: aaronballman
Date: Mon Mar 13 16:39:00 2017
New Revision: 297671
URL: http://llvm.org/viewvc/llvm-project?rev=297671&view=rev
Log:
Add the 'AllowSoleDefaultDtor' and 'AllowMissingMoveFunctions' options to the cppcoreguidelines-special-member-functions check.
Patch by Florian Gross.
Added:
clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-special-member-functions-relaxed.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp
clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
Modified: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp?rev=297671&r1=297670&r2=297671&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp Mon Mar 13 16:39:00 2017
@@ -22,6 +22,18 @@ namespace clang {
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 @@ toString(SpecialMemberFunctionsCheck::Sp
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 @@ void SpecialMemberFunctionsCheck::check(
ClassDefId ID(MatchedDecl->getLocation(), MatchedDecl->getName());
+ auto StoreMember = [this, &ID](SpecialMemberFunctionKind Kind) {
+ llvm::SmallVectorImpl<SpecialMemberFunctionKind> &Members =
+ ClassWithSpecialMembers[ID];
+ if (!llvm::is_contained(Members, Kind))
+ Members.push_back(Kind);
+ };
+
+ if (const auto *Dtor = Result.Nodes.getNodeAs<CXXMethodDecl>("dtor")) {
+ StoreMember(Dtor->isDefaulted()
+ ? SpecialMemberFunctionKind::DefaultDestructor
+ : SpecialMemberFunctionKind::NonDefaultDestructor);
+ }
+
std::initializer_list<std::pair<std::string, SpecialMemberFunctionKind>>
- 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<CXXMethodDecl>(KV.first)) {
- SpecialMemberFunctionKind Kind = KV.second;
- llvm::SmallVectorImpl<SpecialMemberFunctionKind> &Members =
- ClassWithSpecialMembers[ID];
- if (find(Members, Kind) == Members.end())
- Members.push_back(Kind);
+ StoreMember(KV.second);
}
}
void SpecialMemberFunctionsCheck::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) {
+ checkForMissingMembers(C.first, C.second);
}
+}
- for (const auto &C : ClassWithSpecialMembers) {
- const auto &DefinedSpecialMembers = C.second;
+void SpecialMemberFunctionsCheck::checkForMissingMembers(
+ const ClassDefId &ID,
+ llvm::ArrayRef<SpecialMemberFunctionKind> DefinedMembers) {
+ llvm::SmallVector<SpecialMemberFunctionKind, 5> 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<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 ");
+ 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
Modified: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h?rev=297671&r1=297670&r2=297671&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h (original)
+++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h Mon Mar 13 16:39:00 2017
@@ -25,14 +25,16 @@ namespace cppcoreguidelines {
/// 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 @@ public:
llvm::SmallVector<SpecialMemberFunctionKind, 5>>;
private:
+ void checkForMissingMembers(
+ const ClassDefId &ID,
+ llvm::ArrayRef<SpecialMemberFunctionKind> DefinedSpecialMembers);
+
+ const bool AllowMissingMoveFunctions;
+ const bool AllowSoleDefaultDtor;
ClassDefiningSpecialMembersMap ClassWithSpecialMembers;
};
Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst?rev=297671&r1=297670&r2=297671&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst Mon Mar 13 16:39:00 2017
@@ -19,3 +19,31 @@ This rule is part of the "Constructors,
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();
+ }
Modified: clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp?rev=297671&r1=297670&r2=297671&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp Mon Mar 13 16:39:00 2017
@@ -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 &);
Added: clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-special-member-functions-relaxed.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-special-member-functions-relaxed.cpp?rev=297671&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-special-member-functions-relaxed.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-special-member-functions-relaxed.cpp Mon Mar 13 16:39:00 2017
@@ -0,0 +1,71 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-special-member-functions %t -- -config="{CheckOptions: [{key: cppcoreguidelines-special-member-functions.AllowMissingMoveFunctions, value: 1}, {key: cppcoreguidelines-special-member-functions.AllowSoleDefaultDtor, value: 1}]}" --
+
+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]
+
+class DefinesDefaultedDestructor {
+ ~DefinesDefaultedDestructor() = default;
+};
+
+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-special-member-functions]
+
+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-special-member-functions]
+
+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-special-member-functions]
+
+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-special-member-functions]
+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;
+};
+
+template <typename T>
+struct TemplateClass {
+ TemplateClass() = default;
+ TemplateClass(const TemplateClass &);
+ TemplateClass &operator=(const TemplateClass &);
+ TemplateClass(TemplateClass &&);
+ TemplateClass &operator=(TemplateClass &&);
+ ~TemplateClass();
+};
+
+// Multiple instantiations of a class template will trigger multiple matches for defined special members.
+// This should not cause problems.
+TemplateClass<int> InstantiationWithInt;
+TemplateClass<double> InstantiationWithDouble;
Modified: clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-special-member-functions.cpp?rev=297671&r1=297670&r2=297671&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-special-member-functions.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-special-member-functions.cpp Mon Mar 13 16:39:00 2017
@@ -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 &);
More information about the cfe-commits
mailing list