compilerplugins/clang/passparamsbyref.cxx

   1 /* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
   2 /*
   3  * This file is part of the LibreOffice project.
   4  *
   5  * This Source Code Form is subject to the terms of the Mozilla Public
   6  * License, v. 2.0. If a copy of the MPL was not distributed with this
   7  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
   8  */
   9 #ifndef LO_CLANG_SHARED_PLUGINS
  10
  11 #include <string>
  12 #include <unordered_set>
  13
  14 #include "check.hxx"
  15 #include "compat.hxx"
  16 #include "plugin.hxx"
  17
  18 // Find places where parameters are passed by value
  19 // It's not very efficient, because we generally end up copying it twice - once into the parameter and
  20 // again into the destination.
  21 // They should rather be passed by reference.
  22 //
  23 // Generally recommending lambda capture by-ref rather than by-copy is even more
  24 // problematic than with function parameters, as a lambda instance can easily
  25 // outlive a referenced variable. So once lambdas start to get used in more
  26 // sophisticated ways than passing them into standard algorithms, this plugin's
  27 // advice, at least for explicit captures, will need to be revisited.
  28
  29 namespace {
  30
  31 class PassParamsByRef:
  32     public loplugin::FilteringPlugin<PassParamsByRef>
  33 {
  34 public:
  35     explicit PassParamsByRef(loplugin::InstantiationData const & data): FilteringPlugin(data), mbInsideFunctionDecl(false) {}
  36
  37     virtual void run() override { TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); }
  38
  39     // When warning about function params of primitive type that could be passed
  40     // by value instead of by reference, make sure not to warn if the parameter
  41     // is ever bound to a reference; on the one hand, this needs scaffolding in
  42     // all Traverse*Decl functions (indirectly) derived from FunctionDecl; and
  43     // on the other hand, use a hack of ignoring just the DeclRefExprs nested in
  44     // LValueToRValue ImplicitCastExprs when determining whether a param is
  45     // bound to a reference:
  46     bool PreTraverseFunctionDecl(FunctionDecl *);
  47     bool PostTraverseFunctionDecl(FunctionDecl *, bool);
  48     bool TraverseFunctionDecl(FunctionDecl *);
  49     bool PreTraverseImplicitCastExpr(ImplicitCastExpr *);
  50     bool TraverseImplicitCastExpr(ImplicitCastExpr *);
  51
  52     bool VisitBinaryOperator(BinaryOperator const *);
  53     bool VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *);
  54
  55 private:
  56     bool isFat(QualType type);
  57
  58     bool mbInsideFunctionDecl;
  59     std::unordered_set<ParmVarDecl const *> mParamExclusions;
  60 };
  61
  62 bool PassParamsByRef::PreTraverseFunctionDecl(FunctionDecl* functionDecl)
  63 {
  64     if (ignoreLocation(functionDecl))
  65         return false;
  66     if (functionDecl->isDeleted()
  67         || functionDecl->isFunctionTemplateSpecialization())
  68     {
  69         return false;
  70     }
  71     // only consider base declarations, not overridden ones, or we warn on methods that
  72     // are overriding stuff from external libraries
  73     const CXXMethodDecl * methodDecl = dyn_cast<CXXMethodDecl>(functionDecl);
  74     if (methodDecl && methodDecl->size_overridden_methods() > 0)
  75         return false;
  76
  77     // Only warn on the definition of the function:
  78     if (!functionDecl->doesThisDeclarationHaveABody())
  79         return false;
  80
  81     mbInsideFunctionDecl = true;
  82     mParamExclusions.clear();
  83     return true;
  84 }
  85
  86 bool PassParamsByRef::PostTraverseFunctionDecl(FunctionDecl* functionDecl, bool)
  87 {
  88     mbInsideFunctionDecl = false;
  89
  90     auto cxxConstructorDecl = dyn_cast<CXXConstructorDecl>(functionDecl);
  91     unsigned n = functionDecl->getNumParams();
  92     for (unsigned i = 0; i != n; ++i) {
  93         const ParmVarDecl * pvDecl = functionDecl->getParamDecl(i);
  94         auto const t = pvDecl->getType();
  95         if (!isFat(t))
  96             continue;
  97         if (mParamExclusions.find(pvDecl) != mParamExclusions.end())
  98             continue;
  99         // Ignore cases where the parameter is std::move'd.
 100         // This is a fairly simple check, might need some more complexity if the parameter is std::move'd
 101         // somewhere else in the constructor.
 102         bool bFoundMove = false;
 103         if (cxxConstructorDecl) {
 104             for (CXXCtorInitializer const * cxxCtorInitializer : cxxConstructorDecl->inits()) {
 105                 if (cxxCtorInitializer->isMemberInitializer())
 106                 {
 107                     auto cxxConstructExpr = dyn_cast<CXXConstructExpr>(cxxCtorInitializer->getInit()->IgnoreParenImpCasts());
 108                     if (cxxConstructExpr && cxxConstructExpr->getNumArgs() == 1)
 109                     {
 110                         if (auto callExpr = dyn_cast<CallExpr>(cxxConstructExpr->getArg(0)->IgnoreParenImpCasts())) {
 111                             if (loplugin::DeclCheck(callExpr->getCalleeDecl()).Function("move").StdNamespace()) {
 112                                 bFoundMove = true;
 113                                 break;
 114                             }
 115                         }
 116                     }
 117                 }
 118             }
 119         }
 120         if (bFoundMove)
 121             continue;
 122         report(
 123             DiagnosticsEngine::Warning,
 124             ("passing %0 by value, rather pass by const lvalue reference"),
 125             pvDecl->getLocation())
 126             << t << pvDecl->getSourceRange();
 127         auto can = functionDecl->getCanonicalDecl();
 128         if (can->getLocation() != functionDecl->getLocation()) {
 129             report(
 130                 DiagnosticsEngine::Note, "function is declared here:",
 131                 can->getLocation())
 132                 << can->getSourceRange();
 133         }
 134     }
 135     return true;
 136 }
 137
 138 bool PassParamsByRef::TraverseFunctionDecl(FunctionDecl* functionDecl)
 139 {
 140     bool ret = true;
 141     if (PreTraverseFunctionDecl(functionDecl))
 142     {
 143         ret = RecursiveASTVisitor::TraverseFunctionDecl(functionDecl);
 144         PostTraverseFunctionDecl(functionDecl, ret);
 145     }
 146     return ret;
 147 }
 148
 149 bool PassParamsByRef::PreTraverseImplicitCastExpr(ImplicitCastExpr * expr)
 150 {
 151     if (ignoreLocation(expr))
 152         return false;
 153     if (expr->getCastKind() == CK_LValueToRValue
 154          && isa<DeclRefExpr>(expr->getSubExpr()->IgnoreParenImpCasts()))
 155         return false;
 156     return true;
 157 }
 158
 159 bool PassParamsByRef::TraverseImplicitCastExpr(ImplicitCastExpr * expr)
 160 {
 161     bool ret = true;
 162     if (PreTraverseImplicitCastExpr(expr))
 163     {
 164         ret = RecursiveASTVisitor::TraverseImplicitCastExpr(expr);
 165     }
 166     return ret;
 167 }
 168
 169 bool PassParamsByRef::VisitBinaryOperator(const BinaryOperator * binaryOperator)
 170 {
 171     if (binaryOperator->getOpcode() != BO_Assign) {
 172         return true;
 173     }
 174     if (!mbInsideFunctionDecl)
 175         return true;
 176     // if we are assigning to a parameter, it can be inconvenient to make the param pass-by-ref
 177     if (auto declRefExpr = dyn_cast<DeclRefExpr>(binaryOperator->getLHS()))
 178     {
 179         if (auto parmVarDecl = dyn_cast<ParmVarDecl>(declRefExpr->getDecl()))
 180             mParamExclusions.emplace(parmVarDecl);
 181     }
 182     return true;
 183 }
 184
 185 bool PassParamsByRef::VisitCXXOperatorCallExpr(const CXXOperatorCallExpr * cxxOperatorCallExpr )
 186 {
 187     if (!mbInsideFunctionDecl)
 188         return true;
 189     // if we are assigning to a parameter, it can be inconvenient to make the param pass-by-ref
 190     auto op = cxxOperatorCallExpr->getOperator();
 191     if ( op != clang::OverloadedOperatorKind::OO_Equal
 192          && op != clang::OverloadedOperatorKind::OO_SlashEqual
 193          && op != clang::OverloadedOperatorKind::OO_StarEqual
 194          && op != clang::OverloadedOperatorKind::OO_MinusEqual
 195          && op != clang::OverloadedOperatorKind::OO_PlusEqual)
 196         return true;
 197     auto declRefExpr = dyn_cast<DeclRefExpr>(cxxOperatorCallExpr->getArg(0));
 198     if (!declRefExpr)
 199         return true;
 200     if (auto parmVarDecl = dyn_cast<ParmVarDecl>(declRefExpr->getDecl()))
 201         mParamExclusions.emplace(parmVarDecl);
 202     return true;
 203 }
 204
 205 bool PassParamsByRef::isFat(QualType type) {
 206     if (!type->isRecordType()) {
 207         return false;
 208     }
 209     loplugin::TypeCheck tc(type);
 210     if ((tc.Class("Reference").Namespace("uno").Namespace("star")
 211             .Namespace("sun").Namespace("com").GlobalNamespace())
 212         || (tc.Class("Sequence").Namespace("uno").Namespace("star")
 213             .Namespace("sun").Namespace("com").GlobalNamespace())
 214         || tc.Class("OString").Namespace("rtl").GlobalNamespace()
 215         || tc.Class("OUString").Namespace("rtl").GlobalNamespace()
 216         || tc.Class("Reference").Namespace("rtl").GlobalNamespace())
 217     {
 218         return true;
 219     }
 220     if (type->isIncompleteType()) {
 221         return false;
 222     }
 223     clang::Type const * t2 = type.getTypePtrOrNull();
 224     return t2 != nullptr
 225         && compiler.getASTContext().getTypeSizeInChars(t2).getQuantity() > 64;
 226 }
 227
 228 loplugin::Plugin::Registration< PassParamsByRef > passparamsbyref("passparamsbyref");
 229
 230 } // namespace
 231
 232 #endif // LO_CLANG_SHARED_PLUGINS
 233
 234 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */