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         || dc.Function("toAny").Struct("Convert").Namespace("detail").Namespace("comphelper").GlobalNamespace()
 285         || dc.Function("makeAny").Namespace("utl").GlobalNamespace()) {
 286         return;
 287     }
 288     if (startswith(type.getAsString(), "struct o3tl::strong_int")) {
 289         return;
 290     }
 291     auto tc = loplugin::TypeCheck(functionDecl->getReturnType());
 292     // these functions are passed by function-pointer
 293     if (functionDecl->getIdentifier() && functionDecl->getName() == "GetRanges"
 294         && tc.Struct("WhichRangesContainer").GlobalNamespace())
 295         return;
 296     // extremely simple class, might as well pass by value
 297     if (tc.Class("Color")) {
 298         return;
 299     }
 300     // extremely simple class, might as well pass by value
 301     if (tc.Struct("TranslateId")) {
 302         return;
 303     }
 304     if (tc.Class("span").Namespace("o3tl")) {
 305         return;
 306     }
 307
 308     // functionDecl->dump();
 309
 310     mbInsideFunctionDecl = true;
 311     mbFoundReturnValueDisqualifier = false;
 312     TraverseStmt(functionDecl->getBody());
 313     mbInsideFunctionDecl = false;
 314
 315     if (mbFoundReturnValueDisqualifier)
 316         return;
 317
 318     report( DiagnosticsEngine::Warning,
 319             "rather return %0 by const& than by value, to avoid unnecessary copying",
 320             functionDecl->getSourceRange().getBegin())
 321         << type.getAsString() << functionDecl->getSourceRange();
 322
 323     // display the location of the class member declaration so I don't have to search for it by hand
 324     auto canonicalDecl = functionDecl->getCanonicalDecl();
 325     if (functionDecl != canonicalDecl)
 326     {
 327         report( DiagnosticsEngine::Note,
 328                 "decl here",
 329                 canonicalDecl->getSourceRange().getBegin())
 330             << canonicalDecl->getSourceRange();
 331     }
 332
 333     //functionDecl->dump();
 334 }
 335
 336 bool PassStuffByRef::VisitReturnStmt(const ReturnStmt * returnStmt)
 337 {
 338     if (!mbInsideFunctionDecl)
 339         return true;
 340     const Expr* expr = dyn_cast<Expr>(*returnStmt->child_begin())->IgnoreParenImpCasts();
 341
 342     if (isReturnExprDisqualified(expr))
 343         mbFoundReturnValueDisqualifier = true;
 344
 345     return true;
 346 }
 347
 348 /**
 349  * Does a return expression disqualify this method from doing return by const & ?
 350  */
 351 bool PassStuffByRef::isReturnExprDisqualified(const Expr* expr)
 352 {
 353     while (true)
 354     {
 355         expr = expr->IgnoreParens();
 356 //        expr->dump();
 357         if (auto implicitCast = dyn_cast<ImplicitCastExpr>(expr)) {
 358             expr = implicitCast->getSubExpr();
 359             continue;
 360         }
 361         if (auto exprWithCleanups = dyn_cast<ExprWithCleanups>(expr)) {
 362             expr = exprWithCleanups->getSubExpr();
 363             continue;
 364         }
 365         if (auto constructExpr = dyn_cast<CXXConstructExpr>(expr))
 366         {
 367             if (constructExpr->getNumArgs()==1
 368                 && constructExpr->getConstructor()->isCopyOrMoveConstructor())
 369             {
 370                 expr = constructExpr->getArg(0);
 371                 continue;
 372             }
 373             else
 374                 return true;
 375         }
 376         if (isa<CXXFunctionalCastExpr>(expr)) {
 377             return true;
 378         }
 379         if (isa<MaterializeTemporaryExpr>(expr)) {
 380             return true;
 381         }
 382         if (isa<CXXBindTemporaryExpr>(expr)) {
 383             return true;
 384         }
 385         if (isa<InitListExpr>(expr)) {
 386             return true;
 387         }
 388         expr = expr->IgnoreParenCasts();
 389         if (auto childExpr = dyn_cast<ArraySubscriptExpr>(expr)) {
 390             expr = childExpr->getLHS();
 391             continue;
 392         }
 393         if (auto memberExpr = dyn_cast<MemberExpr>(expr)) {
 394             expr = memberExpr->getBase();
 395             continue;
 396         }
 397         if (auto declRef = dyn_cast<DeclRefExpr>(expr)) {
 398             // a param might be a temporary
 399             if (isa<ParmVarDecl>(declRef->getDecl()))
 400                 return true;
 401             const VarDecl* varDecl = dyn_cast<VarDecl>(declRef->getDecl());
 402             if (varDecl) {
 403                 if (varDecl->getStorageDuration() == SD_Thread
 404                     || varDecl->getStorageDuration() == SD_Static ) {
 405                     return false;
 406                 }
 407                 return true;
 408             }
 409         }
 410         if (auto condOper = dyn_cast<ConditionalOperator>(expr)) {
 411             return isReturnExprDisqualified(condOper->getTrueExpr())
 412                 || isReturnExprDisqualified(condOper->getFalseExpr());
 413         }
 414         if (auto unaryOp = dyn_cast<UnaryOperator>(expr)) {
 415             expr = unaryOp->getSubExpr();
 416             continue;
 417         }
 418         if (auto operatorCallExpr = dyn_cast<CXXOperatorCallExpr>(expr)) {
 419             // TODO could improve this, but sometimes it means we're returning a copy of a temporary.
 420             // Same logic as CXXOperatorCallExpr::isAssignmentOp(), which our supported clang
 421             // doesn't have yet.
 422             auto Opc = operatorCallExpr->getOperator();
 423             if (Opc == OO_Equal || Opc == OO_StarEqual ||
 424                 Opc == OO_SlashEqual || Opc == OO_PercentEqual ||
 425                 Opc == OO_PlusEqual || Opc == OO_MinusEqual ||
 426                 Opc == OO_LessLessEqual || Opc == OO_GreaterGreaterEqual ||
 427                 Opc == OO_AmpEqual || Opc == OO_CaretEqual ||
 428                 Opc == OO_PipeEqual)
 429                 return true;
 430             if (Opc == OO_Subscript)
 431             {
 432                 if (isReturnExprDisqualified(operatorCallExpr->getArg(0)))
 433                     return true;
 434                 // otherwise fall through to the checking below
 435             }
 436             if (Opc == OO_Arrow)
 437                 if (isReturnExprDisqualified(operatorCallExpr->getArg(0)))
 438                     return true;
 439         }
 440         if (auto memberCallExpr = dyn_cast<CXXMemberCallExpr>(expr)) {
 441             if (isReturnExprDisqualified(memberCallExpr->getImplicitObjectArgument()))
 442                 return true;
 443             // otherwise fall through to the checking in CallExpr
 444         }
 445         if (auto callExpr = dyn_cast<CallExpr>(expr)) {
 446             FunctionDecl const * calleeFunctionDecl = callExpr->getDirectCallee();
 447             if (!calleeFunctionDecl)
 448                 return true;
 449             // TODO anything takes a non-integral param is suspect because it might return the param by ref.
 450             // we could tighten this to only reject functions that have a param of the same type
 451             // as the return type. Or we could check for such functions and disallow them.
 452             // Or we could force such functions to be annotated somehow.
 453             for (unsigned i = 0; i != calleeFunctionDecl->getNumParams(); ++i) {
 454                 if (!calleeFunctionDecl->getParamDecl(i)->getType()->isIntegralOrEnumerationType())
 455                     return true;
 456             }
 457             auto tc = loplugin::TypeCheck(calleeFunctionDecl->getReturnType());
 458             if (!tc.LvalueReference() && !tc.Pointer())
 459                 return true;
 460         }
 461         return false;
 462     }
 463 }
 464
 465 bool PassStuffByRef::VisitVarDecl(const VarDecl * varDecl)
 466 {
 467     if (!mbInsideFunctionDecl || mbFoundReturnValueDisqualifier)
 468         return true;
 469     // things guarded by locking are probably best left alone
 470     loplugin::TypeCheck dc(varDecl->getType());
 471     if (dc.Class("Guard").Namespace("osl").GlobalNamespace() ||
 472         dc.Class("ClearableGuard").Namespace("osl").GlobalNamespace() ||
 473         dc.Class("ResettableGuard").Namespace("osl").GlobalNamespace() ||
 474         dc.Class("SolarMutexGuard").GlobalNamespace() ||
 475         dc.Class("SfxModelGuard").GlobalNamespace() ||
 476         dc.Class("ReadWriteGuard").Namespace("utl").GlobalNamespace() ||
 477         dc.Class("unique_lock").StdNamespace() ||
 478         dc.Class("lock_guard").StdNamespace() ||
 479         dc.Class("scoped_lock").StdNamespace())
 480         mbFoundReturnValueDisqualifier = true;
 481     return true;
 482 }
 483
 484 bool PassStuffByRef::isPrimitiveConstRef(QualType type) {
 485     if (type->isIncompleteType()) {
 486         return false;
 487     }
 488     const clang::ReferenceType* referenceType = type->getAs<ReferenceType>();
 489     if (referenceType == nullptr) {
 490         return false;
 491     }
 492     QualType pointeeQT = referenceType->getPointeeType();
 493     if (!pointeeQT.isConstQualified()) {
 494         return false;
 495     }
 496     if (!pointeeQT->isFundamentalType()) {
 497         return false;
 498     }
 499     // ignore double for now, some of our code seems to believe it is cheaper to pass by ref
 500     const BuiltinType* builtinType = pointeeQT->getAs<BuiltinType>();
 501     return builtinType->getKind() != BuiltinType::Kind::Double;
 502 }
 503
 504
 505 loplugin::Plugin::Registration< PassStuffByRef > X("passstuffbyref", false);
 506
 507 }
 508
 509 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */