fgross updated this revision to Diff 91183.
fgross added a comment.
Added examples to options doc.
https://reviews.llvm.org/D30610
Files:
clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp
test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
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 &);
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: 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: 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<SpecialMemberFunctionKind, 5>>;
private:
+ void checkForMissingMembers(
+ const ClassDefId &ID,
+ llvm::ArrayRef<SpecialMemberFunctionKind> 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<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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
