compilerplugins/clang/passstuffbyref.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
  10 #include <string>
  11 #include <set>
  12 #include <iostream>
  13
  14 #include "check.hxx"
  15 #include "plugin.hxx"
  16
  17 // Find places where various things are passed by value.
  18 // It's not very efficient, because we generally end up copying it twice - once into the parameter and
  19 // again into the destination.
  20 // They should rather be passed by reference.
  21 //
  22 // Generally recommending lambda capture by-ref rather than by-copy is even more
  23 // problematic than with function parameters, as a lambda instance can easily
  24 // outlive a referenced variable. So once lambdas start to get used in more
  25 // sophisticated ways than passing them into standard algorithms, this plugin's
  26 // advice, at least for explicit captures, will need to be revisited.
  27
  28 namespace {
  29
  30 class PassStuffByRef:
  31     public loplugin::FilteringPlugin<PassStuffByRef>
  32 {
  33 public:
  34     explicit PassStuffByRef(loplugin::InstantiationData const & data): FilteringPlugin(data), mbInsideFunctionDecl(false), mbFoundReturnValueDisqualifier(false) {}
  35
  36     virtual void run() override { TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); }
  37
  38     // When warning about function params of primitive type that could be passed
  39     // by value instead of by reference, make sure not to warn if the parameter
  40     // is ever bound to a reference; on the one hand, this needs scaffolding in
  41     // all Traverse*Decl functions (indirectly) derived from FunctionDecl; and
  42     // on the other hand, use a hack of ignoring just the DeclRefExprs nested in
  43     // LValueToRValue ImplicitCastExprs when determining whether a param is
  44     // bound to a reference:
  45     bool TraverseFunctionDecl(FunctionDecl * decl);
  46     bool TraverseCXXMethodDecl(CXXMethodDecl * decl);
  47     bool TraverseCXXConstructorDecl(CXXConstructorDecl * decl);
  48     bool TraverseCXXDestructorDecl(CXXDestructorDecl * decl);
  49     bool TraverseCXXConversionDecl(CXXConversionDecl * decl);
  50     bool VisitFunctionDecl(const FunctionDecl * decl);
  51     bool TraverseImplicitCastExpr(ImplicitCastExpr * expr);
  52     bool VisitDeclRefExpr(const DeclRefExpr * expr);
  53
  54     bool VisitReturnStmt(const ReturnStmt * );
  55     bool VisitVarDecl(const VarDecl * );
  56
  57 private:
  58     template<typename T> bool traverseAnyFunctionDecl(
  59         T * decl, bool (RecursiveASTVisitor::* fn)(T *));
  60     void checkParams(const FunctionDecl * functionDecl);
  61     void checkReturnValue(const FunctionDecl * functionDecl, const CXXMethodDecl * methodDecl);
  62     bool isPrimitiveConstRef(QualType type);
  63     bool isReturnExprDisqualified(const Expr* expr);
  64
  65     bool mbInsideFunctionDecl;
  66     bool mbFoundReturnValueDisqualifier;
  67
  68     struct FDecl {
  69         std::set<ParmVarDecl const *> parms;
  70         bool check = false;
  71     };
  72     std::vector<FDecl> functionDecls_;
  73 };
  74
  75 bool PassStuffByRef::TraverseFunctionDecl(FunctionDecl * decl) {
  76     return traverseAnyFunctionDecl(
  77         decl, &RecursiveASTVisitor::TraverseFunctionDecl);
  78 }
  79
  80 bool PassStuffByRef::TraverseCXXMethodDecl(CXXMethodDecl * decl) {
  81     return traverseAnyFunctionDecl(
  82         decl, &RecursiveASTVisitor::TraverseCXXMethodDecl);
  83 }
  84
  85 bool PassStuffByRef::TraverseCXXConstructorDecl(CXXConstructorDecl * decl) {
  86     return traverseAnyFunctionDecl(
  87         decl, &RecursiveASTVisitor::TraverseCXXConstructorDecl);
  88 }
  89
  90 bool PassStuffByRef::TraverseCXXDestructorDecl(CXXDestructorDecl * decl) {
  91     return traverseAnyFunctionDecl(
  92         decl, &RecursiveASTVisitor::TraverseCXXDestructorDecl);
  93 }
  94
  95 bool PassStuffByRef::TraverseCXXConversionDecl(CXXConversionDecl * decl) {
  96     return traverseAnyFunctionDecl(
  97         decl, &RecursiveASTVisitor::TraverseCXXConversionDecl);
  98 }
  99
 100 template<typename T> bool PassStuffByRef::traverseAnyFunctionDecl(
 101     T * decl, bool (RecursiveASTVisitor::* fn)(T *))
 102 {
 103     if (ignoreLocation(decl)) {
 104         return true;
 105     }
 106     if (decl->doesThisDeclarationHaveABody()) {
 107         functionDecls_.emplace_back();
 108     }
 109     bool ret = (this->*fn)(decl);
 110     if (decl->doesThisDeclarationHaveABody()) {
 111         assert(!functionDecls_.empty());
 112         if (functionDecls_.back().check) {
 113             for (auto d: functionDecls_.back().parms) {
 114                 report(
 115                     DiagnosticsEngine::Warning,
 116                     ("passing primitive type param %0 by const &, rather pass"
 117                      " by value"),
 118                     d->getLocation())
 119                     << d->getType() << d->getSourceRange();
 120                 auto can = decl->getCanonicalDecl();
 121                 if (can->getLocation() != decl->getLocation()) {
 122                     report(
 123                         DiagnosticsEngine::Note, "function is declared here:",
 124                         can->getLocation())
 125                         << can->getSourceRange();
 126                 }
 127             }
 128         }
 129         functionDecls_.pop_back();
 130     }
 131     return ret;
 132 }
 133
 134 bool PassStuffByRef::VisitFunctionDecl(const FunctionDecl * functionDecl) {
 135     if (ignoreLocation(functionDecl)) {
 136         return true;
 137     }
 138     if (functionDecl->isDeleted()
 139         || functionDecl->isFunctionTemplateSpecialization())
 140     {
 141         return true;
 142     }
 143     // only consider base declarations, not overridden ones, or we warn on methods that
 144     // are overriding stuff from external libraries
 145     const CXXMethodDecl * methodDecl = dyn_cast<CXXMethodDecl>(functionDecl);
 146     if (methodDecl && methodDecl->size_overridden_methods() > 0) {
 147             return true;
 148     }
 149
 150     checkParams(functionDecl);
 151     checkReturnValue(functionDecl, methodDecl);
 152     return true;
 153 }
 154
 155 bool PassStuffByRef::TraverseImplicitCastExpr(ImplicitCastExpr * expr) {
 156     if (ignoreLocation(expr)) {
 157         return true;
 158     }
 159     return
 160         (expr->getCastKind() == CK_LValueToRValue
 161          && (dyn_cast<DeclRefExpr>(expr->getSubExpr()->IgnoreParenImpCasts())
 162              != nullptr))
 163         || RecursiveASTVisitor::TraverseImplicitCastExpr(expr);
 164 }
 165
 166 bool PassStuffByRef::VisitDeclRefExpr(const DeclRefExpr * expr) {
 167     if (ignoreLocation(expr)) {
 168         return true;
 169     }
 170     auto d = dyn_cast<ParmVarDecl>(expr->getDecl());
 171     if (d == nullptr) {
 172         return true;
 173     }
 174     for (auto & fd: functionDecls_) {
 175         if (fd.parms.erase(d) == 1) {
 176             break;
 177         }
 178     }
 179     return true;
 180 }
 181
 182 void PassStuffByRef::checkParams(const FunctionDecl * functionDecl) {
 183     // Only warn on the definition of the function:
 184     if (!functionDecl->doesThisDeclarationHaveABody()) {
 185         return;
 186     }
 187     // ignore stuff that forms part of the stable URE interface
 188     if (isInUnoIncludeFile(functionDecl)) {
 189         return;
 190     }
 191     // these functions are passed as parameters to another function
 192     if (loplugin::DeclCheck(functionDecl).MemberFunction()
 193         .Class("ShapeAttributeLayer").Namespace("internal")
 194         .Namespace("slideshow").GlobalNamespace())
 195     {
 196         return;
 197     }
 198     unsigned n = functionDecl->getNumParams();
 199     assert(!functionDecls_.empty());
 200     functionDecls_.back().check = true;
 201     for (unsigned i = 0; i != n; ++i) {
 202         const ParmVarDecl * pvDecl = functionDecl->getParamDecl(i);
 203         auto const t = pvDecl->getType();
 204         if (isPrimitiveConstRef(t)) {
 205             functionDecls_.back().parms.insert(pvDecl);
 206         }
 207     }
 208 }
 209
 210 static bool startswith(const std::string& rStr, const char* pSubStr) {
 211     return rStr.compare(0, strlen(pSubStr), pSubStr) == 0;
 212 }
 213
 214 void PassStuffByRef::checkReturnValue(const FunctionDecl * functionDecl, const CXXMethodDecl * methodDecl) {
 215     if (methodDecl && (methodDecl->isVirtual() || methodDecl->hasAttr<OverrideAttr>())) {
 216         return;
 217     }
 218     if( !functionDecl->doesThisDeclarationHaveABody()
 219         || functionDecl->isLateTemplateParsed())
 220     {
 221         return;
 222     }
 223
 224     const QualType type = functionDecl->getReturnType().getDesugaredType(compiler.getASTContext());
 225     if (type->isReferenceType() || type->isIntegralOrEnumerationType() || type->isPointerType()
 226         || type->isTemplateTypeParmType() || type->isDependentType() || type->isBuiltinType()
 227         || type->isScalarType())
 228     {
 229         return;
 230     }
 231
 232     // not sure if it's possible to modify these
 233     if (isa<CXXConversionDecl>(functionDecl))
 234         return;
 235
 236     // ignore stuff that forms part of the stable URE interface
 237     if (isInUnoIncludeFile(functionDecl)) {
 238         return;
 239     }
 240
 241     loplugin::DeclCheck dc(functionDecl);
 242     // function is passed as parameter to another function
 243     if (dc.Function("ImplColMonoFnc").Class("GDIMetaFile").GlobalNamespace()
 244         || (dc.Function("darkColor").Class("SvxBorderLine").Namespace("editeng")
 245             .GlobalNamespace())
 246         || (dc.MemberFunction().Class("Binding").Namespace("xforms")
 247             .GlobalNamespace())
 248         || (dc.MemberFunction().Class("Model").Namespace("xforms")
 249             .GlobalNamespace())
 250         || (dc.MemberFunction().Class("Submission").Namespace("xforms")
 251             .GlobalNamespace())
 252         || (dc.Function("TopLeft").Class("SwRect").GlobalNamespace())
 253         || (dc.Function("ConvDicList_CreateInstance").GlobalNamespace())
 254         || (dc.Function("Create").Class("OUnoAutoPilot").Namespace("dbp").GlobalNamespace())
 255         || (dc.Function("Size_").Class("SwRect").GlobalNamespace()))
 256     {
 257         return;
 258     }
 259     // not sure how to exclude this yet, returns copy of one of it's params
 260     if (dc.Function("sameDistColor").GlobalNamespace()
 261         || dc.Function("sameColor").GlobalNamespace()
 262         || (dc.Operator(OO_Call).Struct("StringIdentity").AnonymousNamespace()
 263             .Namespace("pcr").GlobalNamespace())
 264         || (dc.Function("accumulate").Namespace("internal")
 265             .Namespace("slideshow").GlobalNamespace())
 266         || (dc.Function("lerp").Namespace("internal").Namespace("slideshow")
 267             .GlobalNamespace()))
 268         return;
 269     // depends on a define
 270     if (dc.Function("GetSharedFileURL").Class("SfxObjectShell")
 271         .GlobalNamespace()) {
 272         return;
 273     }
 274     // hides a constructor
 275     if (dc.Function("createNonOwningCopy").Class("SortedAutoCompleteStrings").Namespace("editeng")
 276         .GlobalNamespace()) {
 277         return;
 278     }
 279     // template function
 280     if (dc.Function("convertItems").Class("ValueParser").Namespace("configmgr").GlobalNamespace()
 281         || dc.Function("parseListValue").AnonymousNamespace().Namespace("configmgr").GlobalNamespace()
 282         || dc.Function("parseSingleValue").AnonymousNamespace().Namespace("configmgr").GlobalNamespace()
 283         || dc.Function("Create").Class("HandlerComponentBase").Namespace("pcr").GlobalNamespace()) {
 284         return;
 285     }
 286     if (startswith(type.getAsString(), "struct o3tl::strong_int")) {
 287         return;
 288     }
 289     auto tc = loplugin::TypeCheck(functionDecl->getReturnType());
 290     // these functions are passed by function-pointer
 291     if (functionDecl->getIdentifier() && functionDecl->getName() == "GetRanges"
 292         && tc.Struct("WhichRangesContainer").GlobalNamespace())
 293         return;
 294     // extremely simple class, might as well pass by value
 295     if (tc.Class("Color")) {
 296         return;
 297     }
 298     // extremely simple class, might as well pass by value
 299     if (tc.Struct("TranslateId")) {
 300         return;
 301     }
 302
 303     // functionDecl->dump();
 304
 305     mbInsideFunctionDecl = true;
 306     mbFoundReturnValueDisqualifier = false;
 307     TraverseStmt(functionDecl->getBody());
 308     mbInsideFunctionDecl = false;
 309
 310     if (mbFoundReturnValueDisqualifier)
 311         return;
 312
 313     report( DiagnosticsEngine::Warning,
 314             "rather return %0 by const& than by value, to avoid unnecessary copying",
 315             functionDecl->getSourceRange().getBegin())
 316         << type.getAsString() << functionDecl->getSourceRange();
 317
 318     // display the location of the class member declaration so I don't have to search for it by hand
 319     auto canonicalDecl = functionDecl->getCanonicalDecl();
 320     if (functionDecl != canonicalDecl)
 321     {
 322         report( DiagnosticsEngine::Note,
 323                 "decl here",
 324                 canonicalDecl->getSourceRange().getBegin())
 325             << canonicalDecl->getSourceRange();
 326     }
 327
 328     //functionDecl->dump();
 329 }
 330
 331 bool PassStuffByRef::VisitReturnStmt(const ReturnStmt * returnStmt)
 332 {
 333     if (!mbInsideFunctionDecl)
 334         return true;
 335     const Expr* expr = dyn_cast<Expr>(*returnStmt->child_begin())->IgnoreParenCasts();
 336
 337     if (isReturnExprDisqualified(expr))
 338         mbFoundReturnValueDisqualifier = true;
 339
 340     return true;
 341 }
 342
 343 /**
 344  * Does a return expression disqualify this method from doing return by const & ?
 345  */
 346 bool PassStuffByRef::isReturnExprDisqualified(const Expr* expr)
 347 {
 348     while (true)
 349     {
 350         expr = expr->IgnoreParens();
 351         if (auto implicitCast = dyn_cast<ImplicitCastExpr>(expr)) {
 352             expr = implicitCast->getSubExpr();
 353             continue;
 354         }
 355         if (auto exprWithCleanups = dyn_cast<ExprWithCleanups>(expr)) {
 356             expr = exprWithCleanups->getSubExpr();
 357             continue;
 358         }
 359         if (auto constructExpr = dyn_cast<CXXConstructExpr>(expr))
 360         {
 361             if (constructExpr->getNumArgs()==1
 362                 && constructExpr->getConstructor()->isCopyOrMoveConstructor())
 363             {
 364                 expr = constructExpr->getArg(0);
 365                 continue;
 366             }
 367             else
 368                 return true;
 369         }
 370         if (isa<CXXFunctionalCastExpr>(expr)) {
 371             return true;
 372         }
 373         if (isa<MaterializeTemporaryExpr>(expr)) {
 374             return true;
 375         }
 376         if (isa<CXXBindTemporaryExpr>(expr)) {
 377             return true;
 378         }
 379         if (isa<InitListExpr>(expr)) {
 380             return true;
 381         }
 382         expr = expr->IgnoreParenCasts();
 383         if (auto childExpr = dyn_cast<ArraySubscriptExpr>(expr)) {
 384             expr = childExpr->getLHS();
 385             continue;
 386         }
 387         if (auto memberExpr = dyn_cast<MemberExpr>(expr)) {
 388             expr = memberExpr->getBase();
 389             continue;
 390         }
 391         if (auto declRef = dyn_cast<DeclRefExpr>(expr)) {
 392             // a param might be a temporary
 393             if (isa<ParmVarDecl>(declRef->getDecl()))
 394                 return true;
 395             const VarDecl* varDecl = dyn_cast<VarDecl>(declRef->getDecl());
 396             if (varDecl) {
 397                 if (varDecl->getStorageDuration() == SD_Thread
 398                     || varDecl->getStorageDuration() == SD_Static ) {
 399                     return false;
 400                 }
 401                 return true;
 402             }
 403         }
 404         if (auto condOper = dyn_cast<ConditionalOperator>(expr)) {
 405             return isReturnExprDisqualified(condOper->getTrueExpr())
 406                 || isReturnExprDisqualified(condOper->getFalseExpr());
 407         }
 408         if (auto unaryOp = dyn_cast<UnaryOperator>(expr)) {
 409             expr = unaryOp->getSubExpr();
 410             continue;
 411         }
 412         if (auto operatorCallExpr = dyn_cast<CXXOperatorCallExpr>(expr)) {
 413             // TODO could improve this, but sometimes it means we're returning a copy of a temporary.
 414             // Same logic as CXXOperatorCallExpr::isAssignmentOp(), which our supported clang
 415             // doesn't have yet.
 416             auto Opc = operatorCallExpr->getOperator();
 417             if (Opc == OO_Equal || Opc == OO_StarEqual ||
 418                 Opc == OO_SlashEqual || Opc == OO_PercentEqual ||
 419                 Opc == OO_PlusEqual || Opc == OO_MinusEqual ||
 420                 Opc == OO_LessLessEqual || Opc == OO_GreaterGreaterEqual ||
 421                 Opc == OO_AmpEqual || Opc == OO_CaretEqual ||
 422                 Opc == OO_PipeEqual)
 423                 return true;
 424             if (Opc == OO_Subscript)
 425             {
 426                 if (isReturnExprDisqualified(operatorCallExpr->getArg(0)))
 427                     return true;
 428                 // otherwise fall through to the checking below
 429             }
 430         }
 431         if (auto memberCallExpr = dyn_cast<CXXMemberCallExpr>(expr)) {
 432             if (isReturnExprDisqualified(memberCallExpr->getImplicitObjectArgument()))
 433                 return true;
 434             // otherwise fall through to the checking in CallExpr
 435         }
 436         if (auto callExpr = dyn_cast<CallExpr>(expr)) {
 437             FunctionDecl const * calleeFunctionDecl = callExpr->getDirectCallee();
 438             if (!calleeFunctionDecl)
 439                 return true;
 440             // TODO anything takes a non-integral param is suspect because it might return the param by ref.
 441             // we could tighten this to only reject functions that have a param of the same type
 442             // as the return type. Or we could check for such functions and disallow them.
 443             // Or we could force such functions to be annotated somehow.
 444             for (unsigned i = 0; i != calleeFunctionDecl->getNumParams(); ++i) {
 445                 if (!calleeFunctionDecl->getParamDecl(i)->getType()->isIntegralOrEnumerationType())
 446                     return true;
 447             }
 448             auto tc = loplugin::TypeCheck(calleeFunctionDecl->getReturnType());
 449             if (!tc.LvalueReference() && !tc.Pointer())
 450                 return true;
 451         }
 452         return false;
 453     }
 454 }
 455
 456 bool PassStuffByRef::VisitVarDecl(const VarDecl * varDecl)
 457 {
 458     if (!mbInsideFunctionDecl)
 459         return true;
 460     // things guarded by locking are probably best left alone
 461     loplugin::TypeCheck dc(varDecl->getType());
 462     if (dc.Class("Guard").Namespace("osl").GlobalNamespace())
 463         mbFoundReturnValueDisqualifier = true;
 464     if (dc.Class("ClearableGuard").Namespace("osl").GlobalNamespace())
 465         mbFoundReturnValueDisqualifier = true;
 466     if (dc.Class("ResettableGuard").Namespace("osl").GlobalNamespace())
 467         mbFoundReturnValueDisqualifier = true;
 468     else if (dc.Class("SolarMutexGuard").GlobalNamespace())
 469         mbFoundReturnValueDisqualifier = true;
 470     else if (dc.Class("SfxModelGuard").GlobalNamespace())
 471         mbFoundReturnValueDisqualifier = true;
 472     else if (dc.Class("ReadWriteGuard").Namespace("utl").GlobalNamespace())
 473         mbFoundReturnValueDisqualifier = true;
 474     return true;
 475 }
 476
 477 bool PassStuffByRef::isPrimitiveConstRef(QualType type) {
 478     if (type->isIncompleteType()) {
 479         return false;
 480     }
 481     const clang::ReferenceType* referenceType = type->getAs<ReferenceType>();
 482     if (referenceType == nullptr) {
 483         return false;
 484     }
 485     QualType pointeeQT = referenceType->getPointeeType();
 486     if (!pointeeQT.isConstQualified()) {
 487         return false;
 488     }
 489     if (!pointeeQT->isFundamentalType()) {
 490         return false;
 491     }
 492     // ignore double for now, some of our code seems to believe it is cheaper to pass by ref
 493     const BuiltinType* builtinType = pointeeQT->getAs<BuiltinType>();
 494     return builtinType->getKind() != BuiltinType::Kind::Double;
 495 }
 496
 497
 498 loplugin::Plugin::Registration< PassStuffByRef > X("passstuffbyref", false);
 499
 500 }
 501
 502 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */