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
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
|
/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
/*
* This file is part of the LibreOffice project.
*
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/
#include <string>
#include <unordered_set>
#include "check.hxx"
#include "compat.hxx"
#include "plugin.hxx"
// Find places where parameters are passed by value
// It's not very efficient, because we generally end up copying it twice - once into the parameter and
// again into the destination.
// They should rather be passed by reference.
//
// Generally recommending lambda capture by-ref rather than by-copy is even more
// problematic than with function parameters, as a lambda instance can easily
// outlive a referenced variable. So once lambdas start to get used in more
// sophisticated ways than passing them into standard algorithms, this plugin's
// advice, at least for explicit captures, will need to be revisited.
namespace {
class PassParamsByRef:
public RecursiveASTVisitor<PassParamsByRef>, public loplugin::Plugin
{
public:
explicit PassParamsByRef(InstantiationData const & data): Plugin(data), mbInsideFunctionDecl(false) {}
virtual void run() override { TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); }
// When warning about function params of primitive type that could be passed
// by value instead of by reference, make sure not to warn if the parameter
// is ever bound to a reference; on the one hand, this needs scaffolding in
// all Traverse*Decl functions (indirectly) derived from FunctionDecl; and
// on the other hand, use a hack of ignoring just the DeclRefExprs nested in
// LValueToRValue ImplicitCastExprs when determining whether a param is
// bound to a reference:
bool TraverseFunctionDecl(FunctionDecl *);
bool TraverseImplicitCastExpr(ImplicitCastExpr *);
bool VisitBinAssign(BinaryOperator const *);
bool VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *);
private:
bool isFat(QualType type);
bool mbInsideFunctionDecl;
std::unordered_set<ParmVarDecl const *> mParamExclusions;
};
bool PassParamsByRef::TraverseFunctionDecl(FunctionDecl* functionDecl)
{
if (ignoreLocation(functionDecl))
return true;
if (functionDecl->isDeleted()
|| functionDecl->isFunctionTemplateSpecialization())
{
return true;
}
// only consider base declarations, not overridden ones, or we warn on methods that
// are overriding stuff from external libraries
const CXXMethodDecl * methodDecl = dyn_cast<CXXMethodDecl>(functionDecl);
if (methodDecl && methodDecl->size_overridden_methods() > 0)
return true;
// Only warn on the definition of the function:
if (!functionDecl->doesThisDeclarationHaveABody())
return true;
mbInsideFunctionDecl = true;
mParamExclusions.clear();
bool ret = RecursiveASTVisitor::TraverseFunctionDecl(functionDecl);
mbInsideFunctionDecl = false;
auto cxxConstructorDecl = dyn_cast<CXXConstructorDecl>(functionDecl);
unsigned n = functionDecl->getNumParams();
for (unsigned i = 0; i != n; ++i) {
const ParmVarDecl * pvDecl = functionDecl->getParamDecl(i);
auto const t = pvDecl->getType();
if (!isFat(t))
continue;
if (mParamExclusions.find(pvDecl) != mParamExclusions.end())
continue;
// Ignore cases where the parameter is std::move'd.
// This is a fairly simple check, might need some more complexity if the parameter is std::move'd
// somewhere else in the constructor.
bool bFoundMove = false;
if (cxxConstructorDecl) {
for (CXXCtorInitializer const * cxxCtorInitializer : cxxConstructorDecl->inits()) {
if (cxxCtorInitializer->isMemberInitializer())
{
auto cxxConstructExpr = dyn_cast<CXXConstructExpr>(cxxCtorInitializer->getInit()->IgnoreParenImpCasts());
if (cxxConstructExpr && cxxConstructExpr->getNumArgs() == 1)
{
if (auto callExpr = dyn_cast<CallExpr>(cxxConstructExpr->getArg(0)->IgnoreParenImpCasts())) {
if (loplugin::DeclCheck(callExpr->getCalleeDecl()).Function("move").StdNamespace()) {
bFoundMove = true;
break;
}
}
}
}
}
}
if (bFoundMove)
continue;
report(
DiagnosticsEngine::Warning,
("passing %0 by value, rather pass by const lvalue reference"),
pvDecl->getLocation())
<< t << pvDecl->getSourceRange();
auto can = functionDecl->getCanonicalDecl();
if (can->getLocation() != functionDecl->getLocation()) {
report(
DiagnosticsEngine::Note, "function is declared here:",
can->getLocation())
<< can->getSourceRange();
}
}
return ret;
}
bool PassParamsByRef::TraverseImplicitCastExpr(ImplicitCastExpr * expr) {
if (ignoreLocation(expr))
return true;
return
(expr->getCastKind() == CK_LValueToRValue
&& (dyn_cast<DeclRefExpr>(expr->getSubExpr()->IgnoreParenImpCasts())
!= nullptr))
|| RecursiveASTVisitor::TraverseImplicitCastExpr(expr);
}
bool PassParamsByRef::VisitBinAssign(const BinaryOperator * binaryOperator)
{
if (!mbInsideFunctionDecl)
return true;
// if we are assigning to a parameter, it can be inconvenient to make the param pass-by-ref
if (auto declRefExpr = dyn_cast<DeclRefExpr>(binaryOperator->getLHS()))
{
if (auto parmVarDecl = dyn_cast<ParmVarDecl>(declRefExpr->getDecl()))
mParamExclusions.emplace(parmVarDecl);
}
return true;
}
bool PassParamsByRef::VisitCXXOperatorCallExpr(const CXXOperatorCallExpr * cxxOperatorCallExpr )
{
if (!mbInsideFunctionDecl)
return true;
// if we are assigning to a parameter, it can be inconvenient to make the param pass-by-ref
auto op = cxxOperatorCallExpr->getOperator();
if ( op != clang::OverloadedOperatorKind::OO_Equal
&& op != clang::OverloadedOperatorKind::OO_SlashEqual
&& op != clang::OverloadedOperatorKind::OO_StarEqual
&& op != clang::OverloadedOperatorKind::OO_MinusEqual
&& op != clang::OverloadedOperatorKind::OO_PlusEqual)
return true;
auto declRefExpr = dyn_cast<DeclRefExpr>(cxxOperatorCallExpr->getArg(0));
if (!declRefExpr)
return true;
if (auto parmVarDecl = dyn_cast<ParmVarDecl>(declRefExpr->getDecl()))
mParamExclusions.emplace(parmVarDecl);
return true;
}
bool PassParamsByRef::isFat(QualType type) {
if (!type->isRecordType()) {
return false;
}
loplugin::TypeCheck tc(type);
if ((tc.Class("Reference").Namespace("uno").Namespace("star")
.Namespace("sun").Namespace("com").GlobalNamespace())
|| (tc.Class("Sequence").Namespace("uno").Namespace("star")
.Namespace("sun").Namespace("com").GlobalNamespace())
|| tc.Class("OString").Namespace("rtl").GlobalNamespace()
|| tc.Class("OUString").Namespace("rtl").GlobalNamespace()
|| tc.Class("Reference").Namespace("rtl").GlobalNamespace())
{
return true;
}
if (type->isIncompleteType()) {
return false;
}
clang::Type const * t2 = type.getTypePtrOrNull();
return t2 != nullptr
&& compiler.getASTContext().getTypeSizeInChars(t2).getQuantity() > 64;
}
loplugin::Plugin::Registration< PassParamsByRef > X("passparamsbyref");
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
|
