[clang-tools-extra] 2f20417 - Add AllowMissingMoveFunctionsWhenCopyIsDeleted flag to cppcoreguidelines-special-member-functions
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 16 05:14:58 PDT 2020
Author: Paweł Barań
Date: 2020-03-16T08:14:48-04:00
New Revision: 2f20417ef04781cd5019b9a99b85df4790bdf525
URL: https://github.com/llvm/llvm-project/commit/2f20417ef04781cd5019b9a99b85df4790bdf525
DIFF: https://github.com/llvm/llvm-project/commit/2f20417ef04781cd5019b9a99b85df4790bdf525.diff
LOG: Add AllowMissingMoveFunctionsWhenCopyIsDeleted flag to cppcoreguidelines-special-member-functions
The flag allows classes to don't define move operations when copy operations
are explicitly deleted. This flag is related to Google C++ Style Guide
https://google.github.io/styleguide/cppguide.html#Copyable_Movable_Types
Added:
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-special-member-functions-allow-missing-move-when-copy-is-deleted.cpp
Modified:
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
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
index 6ff60495a72b..b7f5eb6e1e33 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
@@ -25,12 +25,16 @@ SpecialMemberFunctionsCheck::SpecialMemberFunctionsCheck(
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 @@ void SpecialMemberFunctionsCheck::check(
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 @@ void SpecialMemberFunctionsCheck::check(
{"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::onEndOfTranslationUnit() {
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 @@ void SpecialMemberFunctionsCheck::checkForMissingMembers(
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
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
index 29982ba4796a..78468cc5c31e 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
@@ -43,19 +43,30 @@ class SpecialMemberFunctionsCheck : public ClangTidyCheck {
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;
};
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
index 28a5e8e5a0df..4ced6c1bc563 100644
--- 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
@@ -46,4 +46,19 @@ Options
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();
+ };
diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-special-member-functions-allow-missing-move-when-copy-is-deleted.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-special-member-functions-allow-missing-move-when-copy-is-deleted.cpp
new file mode 100644
index 000000000000..216a49a44eff
--- /dev/null
+++ b/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;
+};
More information about the cfe-commits
mailing list