1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
|
//===--- SlicingCheck.cpp - clang-tidy-------------------------------------===//
//
// The LLVM Compiler Infrastructure
//
// This file is distributed under the University of Illinois Open Source
// License. See LICENSE.TXT for details.
//
//===----------------------------------------------------------------------===//
#include "SlicingCheck.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/RecordLayout.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/ASTMatchers/ASTMatchers.h"
using namespace clang::ast_matchers;
namespace clang {
namespace tidy {
namespace cppcoreguidelines {
void SlicingCheck::registerMatchers(MatchFinder *Finder) {
// When we see:
// class B : public A { ... };
// A a;
// B b;
// a = b;
// The assignment is OK if:
// - the assignment operator is defined as taking a B as second parameter,
// or
// - B does not define any additional members (either variables or
// overrides) wrt A.
//
// The same holds for copy ctor calls. This also captures stuff like:
// void f(A a);
// f(b);
// Helpers.
const auto OfBaseClass = ofClass(cxxRecordDecl().bind("BaseDecl"));
const auto IsDerivedFromBaseDecl =
cxxRecordDecl(isDerivedFrom(equalsBoundNode("BaseDecl")))
.bind("DerivedDecl");
const auto HasTypeDerivedFromBaseDecl =
anyOf(hasType(IsDerivedFromBaseDecl),
hasType(references(IsDerivedFromBaseDecl)));
const auto IsWithinDerivedCtor =
hasParent(cxxConstructorDecl(ofClass(equalsBoundNode("DerivedDecl"))));
// Assignement slicing: "a = b;" and "a = std::move(b);" variants.
const auto SlicesObjectInAssignment =
callExpr(callee(cxxMethodDecl(anyOf(isCopyAssignmentOperator(),
isMoveAssignmentOperator()),
OfBaseClass)),
hasArgument(1, HasTypeDerivedFromBaseDecl));
// Construction slicing: "A a{b};" and "f(b);" variants. Note that in case of
// slicing the letter will create a temporary and therefore call a ctor.
const auto SlicesObjectInCtor = cxxConstructExpr(
hasDeclaration(cxxConstructorDecl(
anyOf(isCopyConstructor(), isMoveConstructor()), OfBaseClass)),
hasArgument(0, HasTypeDerivedFromBaseDecl),
// We need to disable matching on the call to the base copy/move
// constructor in DerivedDecl's constructors.
unless(IsWithinDerivedCtor));
Finder->addMatcher(
expr(anyOf(SlicesObjectInAssignment, SlicesObjectInCtor)).bind("Call"),
this);
}
/// Warns on methods overridden in DerivedDecl with respect to BaseDecl.
/// FIXME: this warns on all overrides outside of the sliced path in case of
/// multiple inheritance.
void SlicingCheck::DiagnoseSlicedOverriddenMethods(
const Expr &Call, const CXXRecordDecl &DerivedDecl,
const CXXRecordDecl &BaseDecl) {
if (DerivedDecl.getCanonicalDecl() == BaseDecl.getCanonicalDecl())
return;
for (const auto &Method : DerivedDecl.methods()) {
// Virtual destructors are OK. We're ignoring constructors since they are
// tagged as overrides.
if (isa<CXXConstructorDecl>(Method) || isa<CXXDestructorDecl>(Method))
continue;
if (Method->size_overridden_methods() > 0) {
diag(Call.getExprLoc(),
"slicing object from type %0 to %1 discards override %2")
<< &DerivedDecl << &BaseDecl << Method;
}
}
// Recursively process bases.
for (const auto &Base : DerivedDecl.bases()) {
if (const auto *BaseRecordType = Base.getType()->getAs<RecordType>()) {
if (const auto *BaseRecord = cast_or_null<CXXRecordDecl>(
BaseRecordType->getDecl()->getDefinition()))
DiagnoseSlicedOverriddenMethods(Call, *BaseRecord, BaseDecl);
}
}
}
void SlicingCheck::check(const MatchFinder::MatchResult &Result) {
const auto *BaseDecl = Result.Nodes.getNodeAs<CXXRecordDecl>("BaseDecl");
const auto *DerivedDecl =
Result.Nodes.getNodeAs<CXXRecordDecl>("DerivedDecl");
const auto *Call = Result.Nodes.getNodeAs<Expr>("Call");
assert(BaseDecl != nullptr);
assert(DerivedDecl != nullptr);
assert(Call != nullptr);
// Warn when slicing the vtable.
// We're looking through all the methods in the derived class and see if they
// override some methods in the base class.
// It's not enough to just test whether the class is polymorphic because we
// would be fine slicing B to A if no method in B (or its bases) overrides
// anything in A:
// class A { virtual void f(); };
// class B : public A {};
// because in that case calling A::f is the same as calling B::f.
DiagnoseSlicedOverriddenMethods(*Call, *DerivedDecl, *BaseDecl);
// Warn when slicing member variables.
const auto &BaseLayout =
BaseDecl->getASTContext().getASTRecordLayout(BaseDecl);
const auto &DerivedLayout =
DerivedDecl->getASTContext().getASTRecordLayout(DerivedDecl);
const CharUnits StateSize =
DerivedLayout.getDataSize() - BaseLayout.getDataSize();
if (StateSize.isPositive()) {
diag(Call->getExprLoc(), "slicing object from type %0 to %1 discards "
"%2 bytes of state")
<< DerivedDecl << BaseDecl << static_cast<int>(StateSize.getQuantity());
}
}
} // namespace cppcoreguidelines
} // namespace tidy
} // namespace clang
|
