pbaran updated this revision to Diff 248729.
pbaran added a comment.
Herald added a subscriber: wuzish.
Removed unintendedly added comment
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75745/new/
https://reviews.llvm.org/D75745
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-special-member-functions-allow-missing-move-when-copy-is-deleted.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-special-member-functions-allow-missing-move-when-copy-is-deleted.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-special-member-functions-allow-missing-move-when-copy-is-deleted.cpp
@@ -0,0 +1,49 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-special-member-functions %t -- -config="{CheckOptions: [{key: cppcoreguidelines-special-member-functions.AllowMissingMoveFunctionsWhenCopyIsDeleted, value: 1}]}" --
+
+class DefinesEverything {
+ DefinesEverything(const DefinesEverything &);
+ DefinesEverything(DefinesEverything &&);
+ DefinesEverything &operator=(const DefinesEverything &);
+ DefinesEverything &operator=(DefinesEverything &&);
+ ~DefinesEverything();
+};
+
+class DefinesNothing {
+};
+
+class DeletedCopyCtorAndOperator {
+ ~DeletedCopyCtorAndOperator() = default;
+ DeletedCopyCtorAndOperator(const DeletedCopyCtorAndOperator &) = delete;
+ DeletedCopyCtorAndOperator &operator=(const DeletedCopyCtorAndOperator &) = delete;
+};
+
+// CHECK-MESSAGES: [[@LINE+1]]:7: warning: class 'DefaultedCopyCtorAndOperator' defines a default destructor, a copy constructor and a copy assignment operator but does not define a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+class DefaultedCopyCtorAndOperator {
+ ~DefaultedCopyCtorAndOperator() = default;
+ DefaultedCopyCtorAndOperator(const DefaultedCopyCtorAndOperator &) = default;
+ DefaultedCopyCtorAndOperator &operator=(const DefaultedCopyCtorAndOperator &) = default;
+};
+
+// CHECK-MESSAGES: [[@LINE+1]]:7: warning: class 'DefinedCopyCtorAndOperator' defines a default destructor, a copy constructor and a copy assignment operator but does not define a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+class DefinedCopyCtorAndOperator {
+ ~DefinedCopyCtorAndOperator() = default;
+ DefinedCopyCtorAndOperator(const DefinedCopyCtorAndOperator &);
+ DefinedCopyCtorAndOperator &operator=(const DefinedCopyCtorAndOperator &);
+};
+
+// CHECK-MESSAGES: [[@LINE+1]]:7: warning: class 'MissingCopyCtor' defines a default destructor and a copy assignment operator but does not define a copy constructor, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+class MissingCopyCtor {
+ ~MissingCopyCtor() = default;
+ MissingCopyCtor &operator=(const MissingCopyCtor &) = delete;
+};
+
+// CHECK-MESSAGES: [[@LINE+1]]:7: warning: class 'MissingCopyOperator' defines a default destructor and a copy constructor but does not define a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+class MissingCopyOperator {
+ ~MissingCopyOperator() = default;
+ MissingCopyOperator(const MissingCopyOperator &) = delete;
+};
+
+// CHECK-MESSAGES: [[@LINE+1]]:7: warning: class 'MissingAll' 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 MissingAll {
+ ~MissingAll() = default;
+};
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
+++ clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
@@ -46,4 +46,19 @@
A(const A&);
A& operator=(const A&);
~A();
- }
+ };
+
+.. option:: AllowMissingMoveFunctionsWhenCopyIsDeleted
+
+ When set to `1` (default is `0`), this check doesn't flag classes which define deleted copy
+ operations but don't define move operations. This flags is related to Google C++ Style Guide
+ https://google.github.io/styleguide/cppguide.html#Copyable_Movable_Types. With this option enabled, the
+ following class won't be flagged:
+
+ .. code-block:: c++
+
+ struct A {
+ A(const A&) = delete;
+ A& operator=(const A&) = delete;
+ ~A();
+ };
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
@@ -43,19 +43,30 @@
MoveAssignment
};
+ struct SpecialMemberFunctionData {
+ SpecialMemberFunctionKind FunctionKind;
+ bool IsDeleted;
+
+ bool operator==(const SpecialMemberFunctionData &Other) {
+ return (Other.FunctionKind == FunctionKind) &&
+ (Other.IsDeleted == IsDeleted);
+ }
+ };
+
using ClassDefId = std::pair<SourceLocation, std::string>;
using ClassDefiningSpecialMembersMap =
llvm::DenseMap<ClassDefId,
- llvm::SmallVector<SpecialMemberFunctionKind, 5>>;
+ llvm::SmallVector<SpecialMemberFunctionData, 5>>;
private:
void checkForMissingMembers(
const ClassDefId &ID,
- llvm::ArrayRef<SpecialMemberFunctionKind> DefinedSpecialMembers);
+ llvm::ArrayRef<SpecialMemberFunctionData> DefinedSpecialMembers);
const bool AllowMissingMoveFunctions;
const bool AllowSoleDefaultDtor;
+ const bool AllowMissingMoveFunctionsWhenCopyIsDeleted;
ClassDefiningSpecialMembersMap ClassWithSpecialMembers;
};
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
@@ -25,12 +25,16 @@
StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
AllowMissingMoveFunctions(Options.get("AllowMissingMoveFunctions", 0)),
- AllowSoleDefaultDtor(Options.get("AllowSoleDefaultDtor", 0)) {}
+ AllowSoleDefaultDtor(Options.get("AllowSoleDefaultDtor", 0)),
+ AllowMissingMoveFunctionsWhenCopyIsDeleted(
+ Options.get("AllowMissingMoveFunctionsWhenCopyIsDeleted", 0)) {}
void SpecialMemberFunctionsCheck::storeOptions(
ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "AllowMissingMoveFunctions", AllowMissingMoveFunctions);
Options.store(Opts, "AllowSoleDefaultDtor", AllowSoleDefaultDtor);
+ Options.store(Opts, "AllowMissingMoveFunctionsWhenCopyIsDeleted",
+ AllowMissingMoveFunctionsWhenCopyIsDeleted);
}
void SpecialMemberFunctionsCheck::registerMatchers(MatchFinder *Finder) {
@@ -103,17 +107,18 @@
ClassDefId ID(MatchedDecl->getLocation(), std::string(MatchedDecl->getName()));
- auto StoreMember = [this, &ID](SpecialMemberFunctionKind Kind) {
- llvm::SmallVectorImpl<SpecialMemberFunctionKind> &Members =
+ auto StoreMember = [this, &ID](SpecialMemberFunctionData data) {
+ llvm::SmallVectorImpl<SpecialMemberFunctionData> &Members =
ClassWithSpecialMembers[ID];
- if (!llvm::is_contained(Members, Kind))
- Members.push_back(Kind);
+ if (!llvm::is_contained(Members, data))
+ Members.push_back(std::move(data));
};
if (const auto *Dtor = Result.Nodes.getNodeAs<CXXMethodDecl>("dtor")) {
- StoreMember(Dtor->isDefaulted()
- ? SpecialMemberFunctionKind::DefaultDestructor
- : SpecialMemberFunctionKind::NonDefaultDestructor);
+ StoreMember({Dtor->isDefaulted()
+ ? SpecialMemberFunctionKind::DefaultDestructor
+ : SpecialMemberFunctionKind::NonDefaultDestructor,
+ Dtor->isDeleted()});
}
std::initializer_list<std::pair<std::string, SpecialMemberFunctionKind>>
@@ -123,8 +128,9 @@
{"move-assign", SpecialMemberFunctionKind::MoveAssignment}};
for (const auto &KV : Matchers)
- if (Result.Nodes.getNodeAs<CXXMethodDecl>(KV.first)) {
- StoreMember(KV.second);
+ if (const auto *MethodDecl =
+ Result.Nodes.getNodeAs<CXXMethodDecl>(KV.first)) {
+ StoreMember({KV.second, MethodDecl->isDeleted()});
}
}
@@ -136,11 +142,19 @@
void SpecialMemberFunctionsCheck::checkForMissingMembers(
const ClassDefId &ID,
- llvm::ArrayRef<SpecialMemberFunctionKind> DefinedMembers) {
+ llvm::ArrayRef<SpecialMemberFunctionData> DefinedMembers) {
llvm::SmallVector<SpecialMemberFunctionKind, 5> MissingMembers;
auto HasMember = [&](SpecialMemberFunctionKind Kind) {
- return llvm::is_contained(DefinedMembers, Kind);
+ return llvm::any_of(DefinedMembers, [Kind](const auto &data) {
+ return data.FunctionKind == Kind;
+ });
+ };
+
+ auto IsDeleted = [&](SpecialMemberFunctionKind Kind) {
+ return llvm::any_of(DefinedMembers, [Kind](const auto &data) {
+ return data.FunctionKind == Kind && data.IsDeleted;
+ });
};
auto RequireMember = [&](SpecialMemberFunctionKind Kind) {
@@ -171,16 +185,23 @@
RequireMember(SpecialMemberFunctionKind::CopyAssignment);
}
- if (RequireFive) {
+ if (RequireFive &&
+ !(AllowMissingMoveFunctionsWhenCopyIsDeleted &&
+ (IsDeleted(SpecialMemberFunctionKind::CopyConstructor) &&
+ IsDeleted(SpecialMemberFunctionKind::CopyAssignment)))) {
assert(RequireThree);
RequireMember(SpecialMemberFunctionKind::MoveConstructor);
RequireMember(SpecialMemberFunctionKind::MoveAssignment);
}
- if (!MissingMembers.empty())
+ if (!MissingMembers.empty()) {
+ llvm::SmallVector<SpecialMemberFunctionKind, 5> DefinedMemberKinds;
+ llvm::transform(DefinedMembers, std::back_inserter(DefinedMemberKinds),
+ [](const auto &data) { return data.FunctionKind; });
diag(ID.first, "class '%0' defines %1 but does not define %2")
- << ID.second << cppcoreguidelines::join(DefinedMembers, " and ")
+ << ID.second << cppcoreguidelines::join(DefinedMemberKinds, " and ")
<< cppcoreguidelines::join(MissingMembers, " or ");
+ }
}
} // namespace cppcoreguidelines
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
