Skip to content

Commit 5d304b2

Browse files
jbcoejbcoe
committedJul 30, 2016
[clang-tidy] add check cppcoreguidelines-special-member-functions
Summary: Check for classes that violate the rule of five and zero as specified in CppCoreGuidelines: "If a class defines or deletes a default operation then it should define or delete them all." https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c21-if-you-define-or-delete-any-default-operation-define-or-delete-them-all. Reviewers: alexfh, sbenza, aaron.ballman Subscribers: Prazek, Eugene.Zelenko, cfe-commits, ericLemanissier, nemanjai Projects: #clang-tools-extra Differential Revision: https://reviews.llvm.org/D22513 llvm-svn: 277262
1 parent fcfec5f commit 5d304b2

9 files changed

+343
-0
lines changed
 

‎clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ add_clang_library(clangTidyCppCoreGuidelinesModule
1313
ProTypeStaticCastDowncastCheck.cpp
1414
ProTypeUnionAccessCheck.cpp
1515
ProTypeVarargCheck.cpp
16+
SpecialMemberFunctionsCheck.cpp
1617
SlicingCheck.cpp
1718

1819
LINK_LIBS

‎clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "ProTypeStaticCastDowncastCheck.h"
2323
#include "ProTypeUnionAccessCheck.h"
2424
#include "ProTypeVarargCheck.h"
25+
#include "SpecialMemberFunctionsCheck.h"
2526
#include "SlicingCheck.h"
2627

2728
namespace clang {
@@ -54,6 +55,8 @@ class CppCoreGuidelinesModule : public ClangTidyModule {
5455
"cppcoreguidelines-pro-type-union-access");
5556
CheckFactories.registerCheck<ProTypeVarargCheck>(
5657
"cppcoreguidelines-pro-type-vararg");
58+
CheckFactories.registerCheck<SpecialMemberFunctionsCheck>(
59+
"cppcoreguidelines-special-member-functions");
5760
CheckFactories.registerCheck<SlicingCheck>(
5861
"cppcoreguidelines-slicing");
5962
CheckFactories.registerCheck<misc::UnconventionalAssignOperatorCheck>(

‎clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
//===--- SpecialMemberFunctionsCheck.cpp - clang-tidy----------------------===//
2+
//
3+
// The LLVM Compiler Infrastructure
4+
//
5+
// This file is distributed under the University of Illinois Open Source
6+
// License. See LICENSE.TXT for details.
7+
//
8+
//===----------------------------------------------------------------------===//
9+
10+
#include "SpecialMemberFunctionsCheck.h"
11+
12+
#include "clang/AST/ASTContext.h"
13+
#include "clang/ASTMatchers/ASTMatchFinder.h"
14+
#include "llvm/ADT/DenseMapInfo.h"
15+
#include "llvm/ADT/StringExtras.h"
16+
17+
#define DEBUG_TYPE "clang-tidy"
18+
19+
using namespace clang::ast_matchers;
20+
21+
namespace clang {
22+
namespace tidy {
23+
namespace cppcoreguidelines {
24+
25+
void SpecialMemberFunctionsCheck::registerMatchers(MatchFinder *Finder) {
26+
if (!getLangOpts().CPlusPlus)
27+
return;
28+
Finder->addMatcher(
29+
cxxRecordDecl(
30+
eachOf(
31+
has(cxxDestructorDecl(unless(isImplicit())).bind("dtor")),
32+
has(cxxConstructorDecl(isCopyConstructor(), unless(isImplicit()))
33+
.bind("copy-ctor")),
34+
has(cxxMethodDecl(isCopyAssignmentOperator(),
35+
unless(isImplicit()))
36+
.bind("copy-assign")),
37+
has(cxxConstructorDecl(isMoveConstructor(), unless(isImplicit()))
38+
.bind("move-ctor")),
39+
has(cxxMethodDecl(isMoveAssignmentOperator(),
40+
unless(isImplicit()))
41+
.bind("move-assign"))))
42+
.bind("class-def"),
43+
this);
44+
}
45+
46+
llvm::StringRef SpecialMemberFunctionsCheck::toString(
47+
SpecialMemberFunctionsCheck::SpecialMemberFunctionKind K) {
48+
switch (K) {
49+
case SpecialMemberFunctionKind::Destructor:
50+
return "a destructor";
51+
case SpecialMemberFunctionKind::CopyConstructor:
52+
return "a copy constructor";
53+
case SpecialMemberFunctionKind::CopyAssignment:
54+
return "a copy assignment operator";
55+
case SpecialMemberFunctionKind::MoveConstructor:
56+
return "a move constructor";
57+
case SpecialMemberFunctionKind::MoveAssignment:
58+
return "a move assignment operator";
59+
}
60+
llvm_unreachable("Unhandled SpecialMemberFunctionKind");
61+
}
62+
63+
std::string SpecialMemberFunctionsCheck::join(
64+
llvm::ArrayRef<SpecialMemberFunctionKind> SMFS, llvm::StringRef AndOr) {
65+
66+
assert(!SMFS.empty() &&
67+
"List of defined or undefined members should never be empty.");
68+
std::string Buffer;
69+
llvm::raw_string_ostream Stream(Buffer);
70+
71+
Stream << toString(SMFS[0]);
72+
size_t LastIndex = SMFS.size() - 1;
73+
for (size_t i = 1; i < LastIndex; ++i) {
74+
Stream << ", " << toString(SMFS[i]);
75+
}
76+
if (LastIndex != 0) {
77+
Stream << AndOr << toString(SMFS[LastIndex]);
78+
}
79+
return Stream.str();
80+
}
81+
82+
void SpecialMemberFunctionsCheck::check(
83+
const MatchFinder::MatchResult &Result) {
84+
const CXXRecordDecl *MatchedDecl =
85+
Result.Nodes.getNodeAs<CXXRecordDecl>("class-def");
86+
if (!MatchedDecl)
87+
return;
88+
89+
ClassDefId ID(MatchedDecl->getLocation(), MatchedDecl->getName());
90+
91+
std::initializer_list<std::pair<std::string, SpecialMemberFunctionKind>>
92+
Matchers = {{"dtor", SpecialMemberFunctionKind::Destructor},
93+
{"copy-ctor", SpecialMemberFunctionKind::CopyConstructor},
94+
{"copy-assign", SpecialMemberFunctionKind::CopyAssignment},
95+
{"move-ctor", SpecialMemberFunctionKind::MoveConstructor},
96+
{"move-assign", SpecialMemberFunctionKind::MoveAssignment}};
97+
98+
for (const auto &KV : Matchers)
99+
if (Result.Nodes.getNodeAs<CXXMethodDecl>(KV.first))
100+
ClassWithSpecialMembers[ID].push_back(KV.second);
101+
}
102+
103+
void SpecialMemberFunctionsCheck::onEndOfTranslationUnit() {
104+
llvm::SmallVector<SpecialMemberFunctionKind, 5> AllSpecialMembers = {
105+
SpecialMemberFunctionKind::Destructor,
106+
SpecialMemberFunctionKind::CopyConstructor,
107+
SpecialMemberFunctionKind::CopyAssignment};
108+
109+
if (getLangOpts().CPlusPlus11) {
110+
AllSpecialMembers.push_back(SpecialMemberFunctionKind::MoveConstructor);
111+
AllSpecialMembers.push_back(SpecialMemberFunctionKind::MoveAssignment);
112+
}
113+
114+
for (const auto &C : ClassWithSpecialMembers) {
115+
ArrayRef<SpecialMemberFunctionKind> DefinedSpecialMembers = C.second;
116+
117+
if (DefinedSpecialMembers.size() == AllSpecialMembers.size())
118+
continue;
119+
120+
llvm::SmallVector<SpecialMemberFunctionKind, 5> UndefinedSpecialMembers;
121+
std::set_difference(AllSpecialMembers.begin(), AllSpecialMembers.end(),
122+
DefinedSpecialMembers.begin(),
123+
DefinedSpecialMembers.end(),
124+
std::back_inserter(UndefinedSpecialMembers));
125+
126+
diag(C.first.first, "class '%0' defines %1 but does not define %2")
127+
<< C.first.second << join(DefinedSpecialMembers, " and ")
128+
<< join(UndefinedSpecialMembers, " or ");
129+
}
130+
}
131+
} // namespace cppcoreguidelines
132+
} // namespace tidy
133+
} // namespace clang

‎clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
//===--- SpecialMemberFunctionsCheck.h - clang-tidy-------------------*- C++ -*-===//
2+
//
3+
// The LLVM Compiler Infrastructure
4+
//
5+
// This file is distributed under the University of Illinois Open Source
6+
// License. See LICENSE.TXT for details.
7+
//
8+
//===----------------------------------------------------------------------===//
9+
10+
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_SPECIAL_MEMBER_FUNCTIONS_H
11+
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_SPECIAL_MEMBER_FUNCTIONS_H
12+
13+
#include "../ClangTidy.h"
14+
15+
#include "llvm/ADT/DenseMapInfo.h"
16+
17+
namespace clang {
18+
namespace tidy {
19+
namespace cppcoreguidelines {
20+
21+
/// Checks for classes where some, but not all, of the special member functions
22+
/// are defined.
23+
///
24+
/// For the user-facing documentation see:
25+
/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-special-member-functions.html
26+
class SpecialMemberFunctionsCheck : public ClangTidyCheck {
27+
public:
28+
SpecialMemberFunctionsCheck(StringRef Name, ClangTidyContext *Context)
29+
: ClangTidyCheck(Name, Context) {}
30+
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
31+
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
32+
void onEndOfTranslationUnit() override;
33+
34+
enum class SpecialMemberFunctionKind {
35+
Destructor,
36+
CopyConstructor,
37+
CopyAssignment,
38+
MoveConstructor,
39+
MoveAssignment
40+
};
41+
42+
using ClassDefId = std::pair<SourceLocation, std::string>;
43+
44+
using ClassDefiningSpecialMembersMap = llvm::DenseMap<ClassDefId, llvm::SmallVector<SpecialMemberFunctionKind, 5>>;
45+
46+
private:
47+
48+
static llvm::StringRef toString(SpecialMemberFunctionKind K);
49+
50+
static std::string join(llvm::ArrayRef<SpecialMemberFunctionKind> SMFS,
51+
llvm::StringRef AndOr);
52+
53+
ClassDefiningSpecialMembersMap ClassWithSpecialMembers;
54+
};
55+
56+
} // namespace cppcoreguidelines
57+
} // namespace tidy
58+
} // namespace clang
59+
60+
namespace llvm {
61+
/// Specialisation of DenseMapInfo to allow ClassDefId objects in DenseMaps
62+
/// FIXME: Move this to the corresponding cpp file as is done for
63+
/// clang-tidy/readability/IdentifierNamingCheck.cpp.
64+
template <>
65+
struct DenseMapInfo<
66+
clang::tidy::cppcoreguidelines::SpecialMemberFunctionsCheck::ClassDefId> {
67+
using ClassDefId =
68+
clang::tidy::cppcoreguidelines::SpecialMemberFunctionsCheck::ClassDefId;
69+
70+
static inline ClassDefId getEmptyKey() {
71+
return ClassDefId(
72+
clang::SourceLocation::getFromRawEncoding(static_cast<unsigned>(-1)),
73+
"EMPTY");
74+
}
75+
76+
static inline ClassDefId getTombstoneKey() {
77+
return ClassDefId(
78+
clang::SourceLocation::getFromRawEncoding(static_cast<unsigned>(-2)),
79+
"TOMBSTONE");
80+
}
81+
82+
static unsigned getHashValue(ClassDefId Val) {
83+
assert(Val != getEmptyKey() && "Cannot hash the empty key!");
84+
assert(Val != getTombstoneKey() && "Cannot hash the tombstone key!");
85+
86+
std::hash<ClassDefId::second_type> SecondHash;
87+
return Val.first.getRawEncoding() + SecondHash(Val.second);
88+
}
89+
90+
static bool isEqual(ClassDefId LHS, ClassDefId RHS) {
91+
if (RHS == getEmptyKey())
92+
return LHS == getEmptyKey();
93+
if (RHS == getTombstoneKey())
94+
return LHS == getTombstoneKey();
95+
return LHS == RHS;
96+
}
97+
};
98+
99+
} // namespace llvm
100+
101+
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_SPECIAL_MEMBER_FUNCTIONS_H

‎clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,11 @@ Improvements to clang-tidy
6464

6565
Flags slicing of member variables or vtable.
6666

67+
- New `cppcoreguidelines-special-member-functions
68+
<http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-special-member-functions.html>`_ check
69+
70+
Flags classes where some, but not all, special member functions are user-defined.
71+
6772
Improvements to include-fixer
6873
-----------------------------
6974

‎clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
.. title:: clang-tidy - cppcoreguidelines-special-member-functions
2+
3+
cppcoreguidelines-special-member-functions
4+
==========================================
5+
6+
The check finds classes where some but not all of the special member functions
7+
are defined.
8+
9+
By default the compiler defines a copy constructor, copy assignment operator,
10+
move constructor, move assignment operator and destructor. The default can be
11+
supressed by explicit user-definitions. The relationship between which
12+
functions will be supressed by definitions of other functions is complicated
13+
and it is advised that all five are defaulted or explicitly defined.
14+
15+
Note that defining a function with ``= delete`` is considered to be a
16+
definition.
17+
18+
This rule is part of the "Constructors, assignments, and destructors" profile of the C++ Core
19+
Guidelines, corresponding to rule C.21. See
20+
21+
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c21-if-you-define-or-delete-any-default-operation-define-or-delete-them-all.

‎clang-tools-extra/docs/clang-tidy/checks/list.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ Clang-Tidy Checks
3030
cppcoreguidelines-pro-type-union-access
3131
cppcoreguidelines-pro-type-vararg
3232
cppcoreguidelines-slicing
33+
cppcoreguidelines-special-member-functions
3334
google-build-explicit-make-pair
3435
google-build-namespaces
3536
google-build-using-namespace

‎clang-tools-extra/test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// RUN: %check_clang_tidy %s cppcoreguidelines-special-member-functions %t -- -- -std=c++03
2+
3+
class DefinesDestructor {
4+
~DefinesDestructor();
5+
};
6+
// 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]
7+
8+
class DefinesCopyConstructor {
9+
DefinesCopyConstructor(const DefinesCopyConstructor &);
10+
};
11+
// 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]
12+
13+
class DefinesCopyAssignment {
14+
DefinesCopyAssignment &operator=(const DefinesCopyAssignment &);
15+
};
16+
// 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]
17+
18+
class DefinesNothing {
19+
};
20+
21+
class DefinesEverything {
22+
DefinesEverything(const DefinesEverything &);
23+
DefinesEverything &operator=(const DefinesEverything &);
24+
~DefinesEverything();
25+
};
26+

‎clang-tools-extra/test/clang-tidy/cppcoreguidelines-special-member-functions.cpp

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
// RUN: %check_clang_tidy %s cppcoreguidelines-special-member-functions %t
2+
3+
class DefinesDestructor {
4+
~DefinesDestructor();
5+
};
6+
// 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]
7+
8+
class DefinesCopyConstructor {
9+
DefinesCopyConstructor(const DefinesCopyConstructor &);
10+
};
11+
// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyConstructor' defines a copy constructor but does not define a destructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
12+
13+
class DefinesCopyAssignment {
14+
DefinesCopyAssignment &operator=(const DefinesCopyAssignment &);
15+
};
16+
// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyAssignment' defines a copy assignment operator but does not define a destructor, a copy constructor, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
17+
18+
class DefinesMoveConstructor {
19+
DefinesMoveConstructor(DefinesMoveConstructor &&);
20+
};
21+
// 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]
22+
23+
class DefinesMoveAssignment {
24+
DefinesMoveAssignment &operator=(DefinesMoveAssignment &&);
25+
};
26+
// 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]
27+
class DefinesNothing {
28+
};
29+
30+
class DefinesEverything {
31+
DefinesEverything(const DefinesEverything &);
32+
DefinesEverything &operator=(const DefinesEverything &);
33+
DefinesEverything(DefinesEverything &&);
34+
DefinesEverything &operator=(DefinesEverything &&);
35+
~DefinesEverything();
36+
};
37+
38+
class DeletesEverything {
39+
DeletesEverything(const DeletesEverything &) = delete;
40+
DeletesEverything &operator=(const DeletesEverything &) = delete;
41+
DeletesEverything(DeletesEverything &&) = delete;
42+
DeletesEverything &operator=(DeletesEverything &&) = delete;
43+
~DeletesEverything() = delete;
44+
};
45+
46+
class DeletesCopyDefaultsMove {
47+
DeletesCopyDefaultsMove(const DeletesCopyDefaultsMove &) = delete;
48+
DeletesCopyDefaultsMove &operator=(const DeletesCopyDefaultsMove &) = delete;
49+
DeletesCopyDefaultsMove(DeletesCopyDefaultsMove &&) = default;
50+
DeletesCopyDefaultsMove &operator=(DeletesCopyDefaultsMove &&) = default;
51+
~DeletesCopyDefaultsMove() = default;
52+
};

0 commit comments

Comments
 (0)
Please sign in to comment.