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
  13 #include "check.hxx"
  14 #include "compat.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 referrenced 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 RecursiveASTVisitor<PassStuffByRef>, public loplugin::Plugin
  32 {
  33 public:
  34     explicit PassStuffByRef(InstantiationData const & data): Plugin(data), mbInsideFunctionDecl(false), mbFoundDisqualifier(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 isFat(QualType type);
  63     bool isPrimitiveConstRef(QualType type);
  64     bool isReturnExprDisqualified(const Expr* expr);
  65
  66     bool mbInsideFunctionDecl;
  67     bool mbFoundDisqualifier;
  68
  69     struct FDecl {
  70         std::set<ParmVarDecl const *> parms;
  71         bool check = false;
  72     };
  73     std::vector<FDecl> functionDecls_;
  74 };
  75
  76 bool PassStuffByRef::TraverseFunctionDecl(FunctionDecl * decl) {
  77     return traverseAnyFunctionDecl(
  78         decl, &RecursiveASTVisitor::TraverseFunctionDecl);
  79 }
  80
  81 bool PassStuffByRef::TraverseCXXMethodDecl(CXXMethodDecl * decl) {
  82     return traverseAnyFunctionDecl(
  83         decl, &RecursiveASTVisitor::TraverseCXXMethodDecl);
  84 }
  85
  86 bool PassStuffByRef::TraverseCXXConstructorDecl(CXXConstructorDecl * decl) {
  87     return traverseAnyFunctionDecl(
  88         decl, &RecursiveASTVisitor::TraverseCXXConstructorDecl);
  89 }
  90
  91 bool PassStuffByRef::TraverseCXXDestructorDecl(CXXDestructorDecl * decl) {
  92     return traverseAnyFunctionDecl(
  93         decl, &RecursiveASTVisitor::TraverseCXXDestructorDecl);
  94 }
  95
  96 bool PassStuffByRef::TraverseCXXConversionDecl(CXXConversionDecl * decl) {
  97     return traverseAnyFunctionDecl(
  98         decl, &RecursiveASTVisitor::TraverseCXXConversionDecl);
  99 }
 100
 101 template<typename T> bool PassStuffByRef::traverseAnyFunctionDecl(
 102     T * decl, bool (RecursiveASTVisitor::* fn)(T *))
 103 {
 104     if (ignoreLocation(decl)) {
 105         return true;
 106     }
 107     if (decl->doesThisDeclarationHaveABody()) {
 108         functionDecls_.emplace_back();
 109     }
 110     bool ret = (this->*fn)(decl);
 111     if (decl->doesThisDeclarationHaveABody()) {
 112         assert(!functionDecls_.empty());
 113         if (functionDecls_.back().check) {
 114             for (auto d: functionDecls_.back().parms) {
 115                 report(
 116                     DiagnosticsEngine::Warning,
 117                     ("passing primitive type param %0 by const &, rather pass"
 118                      " by value"),
 119                     d->getLocation())
 120                     << d->getType() << d->getSourceRange();
 121                 auto can = decl->getCanonicalDecl();
 122                 if (can->getLocation() != decl->getLocation()) {
 123                     report(
 124                         DiagnosticsEngine::Note, "function is declared here:",
 125                         can->getLocation())
 126                         << can->getSourceRange();
 127                 }
 128             }
 129         }
 130         functionDecls_.pop_back();
 131     }
 132     return ret;
 133 }
 134
 135 bool PassStuffByRef::VisitFunctionDecl(const FunctionDecl * functionDecl) {
 136     if (ignoreLocation(functionDecl)) {
 137         return true;
 138     }
 139     if (functionDecl->isDeleted()
 140         || functionDecl->isFunctionTemplateSpecialization())
 141     {
 142         return true;
 143     }
 144     // only consider base declarations, not overriden ones, or we warn on methods that
 145     // are overriding stuff from external libraries
 146     const CXXMethodDecl * methodDecl = dyn_cast<CXXMethodDecl>(functionDecl);
 147     if (methodDecl && methodDecl->size_overridden_methods() > 0) {
 148             return true;
 149     }
 150
 151     checkParams(functionDecl);
 152     checkReturnValue(functionDecl, methodDecl);
 153     return true;
 154 }
 155
 156 bool PassStuffByRef::TraverseImplicitCastExpr(ImplicitCastExpr * expr) {
 157     if (ignoreLocation(expr)) {
 158         return true;
 159     }
 160     return
 161         (expr->getCastKind() == CK_LValueToRValue
 162          && (dyn_cast<DeclRefExpr>(expr->getSubExpr()->IgnoreParenImpCasts())
 163              != nullptr))
 164         || RecursiveASTVisitor::TraverseImplicitCastExpr(expr);
 165 }
 166
 167 bool PassStuffByRef::VisitDeclRefExpr(const DeclRefExpr * expr) {
 168     if (ignoreLocation(expr)) {
 169         return true;
 170     }
 171     auto d = dyn_cast<ParmVarDecl>(expr->getDecl());
 172     if (d == nullptr) {
 173         return true;
 174     }
 175     for (auto & fd: functionDecls_) {
 176         if (fd.parms.erase(d) == 1) {
 177             break;
 178         }
 179     }
 180     return true;
 181 }
 182
 183 void PassStuffByRef::checkParams(const FunctionDecl * functionDecl) {
 184     // Only warn on the definition of the function:
 185     if (!functionDecl->doesThisDeclarationHaveABody()) {
 186         return;
 187     }
 188     unsigned n = functionDecl->getNumParams();
 189     for (unsigned i = 0; i != n; ++i) {
 190         const ParmVarDecl * pvDecl = functionDecl->getParamDecl(i);
 191         auto const t = pvDecl->getType();
 192         if (isFat(t)) {
 193             report(
 194                 DiagnosticsEngine::Warning,
 195                 ("passing %0 by value, rather pass by const lvalue reference"),
 196                 pvDecl->getLocation())
 197                 << t << pvDecl->getSourceRange();
 198             auto can = functionDecl->getCanonicalDecl();
 199             if (can->getLocation() != functionDecl->getLocation()) {
 200                 report(
 201                     DiagnosticsEngine::Note, "function is declared here:",
 202                     can->getLocation())
 203                     << can->getSourceRange();
 204             }
 205         }
 206     }
 207     // ignore stuff that forms part of the stable URE interface
 208     if (isInUnoIncludeFile(functionDecl)) {
 209         return;
 210     }
 211     // these functions are passed as parameters to another function
 212     if (loplugin::DeclCheck(functionDecl).MemberFunction()
 213         .Class("ShapeAttributeLayer").Namespace("internal")
 214         .Namespace("slideshow").GlobalNamespace())
 215     {
 216         return;
 217     }
 218     assert(!functionDecls_.empty());
 219     functionDecls_.back().check = true;
 220     for (unsigned i = 0; i != n; ++i) {
 221         const ParmVarDecl * pvDecl = functionDecl->getParamDecl(i);
 222         auto const t = pvDecl->getType();
 223         if (isPrimitiveConstRef(t)) {
 224             functionDecls_.back().parms.insert(pvDecl);
 225         }
 226     }
 227 }
 228
 229 void PassStuffByRef::checkReturnValue(const FunctionDecl * functionDecl, const CXXMethodDecl * methodDecl) {
 230
 231     if (methodDecl && (methodDecl->isVirtual() || methodDecl->hasAttr<OverrideAttr>())) {
 232         return;
 233     }
 234     if( !functionDecl->doesThisDeclarationHaveABody()) {
 235         return;
 236     }
 237
 238     const QualType type = compat::getReturnType(*functionDecl).getDesugaredType(compiler.getASTContext());
 239     if (type->isReferenceType() || type->isIntegralOrEnumerationType() || type->isPointerType()
 240         || type->isTemplateTypeParmType() || type->isDependentType() || type->isBuiltinType()
 241         || type->isScalarType())
 242     {
 243         return;
 244     }
 245
 246     // ignore stuff that forms part of the stable URE interface
 247     if (isInUnoIncludeFile(functionDecl)) {
 248         return;
 249     }
 250     loplugin::DeclCheck dc(functionDecl);
 251     // function is passed as parameter to another function
 252     if (dc.Function("ImplColMonoFnc").Class("GDIMetaFile").GlobalNamespace()
 253         || (dc.Function("darkColor").Class("SvxBorderLine").Namespace("editeng")
 254             .GlobalNamespace())
 255         || (dc.MemberFunction().Class("Binding").Namespace("xforms")
 256             .GlobalNamespace())
 257         || (dc.MemberFunction().Class("Model").Namespace("xforms")
 258             .GlobalNamespace())
 259         || (dc.MemberFunction().Class("Submission").Namespace("xforms")
 260             .GlobalNamespace())
 261         || (dc.Function("TopLeft").Class("SwRect").GlobalNamespace()))
 262     {
 263         return;
 264     }
 265     // not sure how to exclude this yet, returns copy of one of it's params
 266     if (dc.Function("sameDistColor").GlobalNamespace()
 267         || dc.Function("sameColor").GlobalNamespace()
 268         || (dc.Operator(OO_Call).Struct("StringIdentity").AnonymousNamespace()
 269             .Namespace("pcr").GlobalNamespace())
 270         || (dc.Function("accumulate").Namespace("internal")
 271             .Namespace("slideshow").GlobalNamespace())
 272         || (dc.Function("lerp").Namespace("internal").Namespace("slideshow")
 273             .GlobalNamespace()))
 274         return;
 275     // depends on a define
 276     if (dc.Function("GetSharedFileURL").Class("SfxObjectShell")
 277         .GlobalNamespace()) {
 278         return;
 279     }
 280     mbInsideFunctionDecl = true;
 281     mbFoundDisqualifier = false;
 282     TraverseStmt(functionDecl->getBody());
 283     mbInsideFunctionDecl = false;
 284
 285     if (mbFoundDisqualifier)
 286         return;
 287
 288     report(
 289             DiagnosticsEngine::Warning,
 290             "rather return %0 from function %1 %2 by const& than by value, to avoid unnecessary copying",
 291             functionDecl->getSourceRange().getBegin())
 292         << type.getAsString() << functionDecl->getQualifiedNameAsString() << type->getTypeClassName() << functionDecl->getSourceRange();
 293
 294     // display the location of the class member declaration so I don't have to search for it by hand
 295     if (functionDecl->getSourceRange().getBegin() != functionDecl->getCanonicalDecl()->getSourceRange().getBegin())
 296     {
 297         report(
 298                 DiagnosticsEngine::Note,
 299                 "decl here",
 300                 functionDecl->getCanonicalDecl()->getSourceRange().getBegin())
 301             << functionDecl->getCanonicalDecl()->getSourceRange();
 302     }
 303 }
 304
 305 bool PassStuffByRef::VisitReturnStmt(const ReturnStmt * returnStmt)
 306 {
 307     if (!mbInsideFunctionDecl)
 308         return true;
 309     const Expr* expr = dyn_cast<Expr>(*returnStmt->child_begin())->IgnoreParenCasts();
 310
 311     if (isReturnExprDisqualified(expr)) {
 312         mbFoundDisqualifier = true;
 313         return true;
 314     }
 315     return true;
 316 }
 317
 318 bool PassStuffByRef::isReturnExprDisqualified(const Expr* expr)
 319 {
 320     if (isa<ExprWithCleanups>(expr)) {
 321         return true;
 322     }
 323     if (const CXXConstructExpr* constructExpr = dyn_cast<CXXConstructExpr>(expr)) {
 324         if (constructExpr->getNumArgs()==1) {
 325             expr = constructExpr->getArg(0)->IgnoreParenCasts();
 326         }
 327     }
 328     if (isa<CXXConstructExpr>(expr)) {
 329         return true;
 330     }
 331     if (const ArraySubscriptExpr* childExpr = dyn_cast<ArraySubscriptExpr>(expr)) {
 332         expr = childExpr->getLHS();
 333     }
 334     if (const MemberExpr* memberExpr = dyn_cast<MemberExpr>(expr)) {
 335         expr = memberExpr->getBase();
 336     }
 337     if (const DeclRefExpr* declRef = dyn_cast<DeclRefExpr>(expr)) {
 338         const VarDecl* varDecl = dyn_cast<VarDecl>(declRef->getDecl());
 339         if (varDecl) {
 340             if (varDecl->getStorageDuration() == SD_Automatic
 341                 || varDecl->getStorageDuration() == SD_FullExpression ) {
 342                 return true;
 343             }
 344             return false;
 345         }
 346     }
 347     if (const ConditionalOperator* condOper = dyn_cast<ConditionalOperator>(expr)) {
 348         return isReturnExprDisqualified(condOper->getTrueExpr())
 349             || isReturnExprDisqualified(condOper->getFalseExpr());
 350     }
 351     if (const CallExpr* callExpr = dyn_cast<CallExpr>(expr)) {
 352         return !loplugin::TypeCheck(callExpr->getType()).Const().LvalueReference();
 353     }
 354     return true;
 355 }
 356
 357 bool PassStuffByRef::VisitVarDecl(const VarDecl * varDecl)
 358 {
 359     if (!mbInsideFunctionDecl)
 360         return true;
 361     // things guarded by locking are probably best left alone
 362     loplugin::TypeCheck dc(varDecl->getType());
 363     if (dc.Class("Guard").Namespace("osl").GlobalNamespace())
 364         mbFoundDisqualifier = true;
 365     if (dc.Class("ClearableGuard").Namespace("osl").GlobalNamespace())
 366         mbFoundDisqualifier = true;
 367     if (dc.Class("ResettableGuard").Namespace("osl").GlobalNamespace())
 368         mbFoundDisqualifier = true;
 369     else if (dc.Class("SolarMutexGuard").GlobalNamespace())
 370         mbFoundDisqualifier = true;
 371     else if (dc.Class("SfxModelGuard").GlobalNamespace())
 372         mbFoundDisqualifier = true;
 373     else if (dc.Class("ReadWriteGuard").Namespace("utl").GlobalNamespace())
 374         mbFoundDisqualifier = true;
 375     return true;
 376 }
 377
 378 // Would produce a wrong recommendation for
 379 //
 380 //   PresenterFrameworkObserver::RunOnUpdateEnd(
 381 //       xCC,
 382 //       [pSelf](bool){ return pSelf->ShutdownPresenterScreen(); });
 383 //
 384 // in PresenterScreen::RequestShutdownPresenterScreen
 385 // (sdext/source/presenter/PresenterScreen.cxx), with no obvious way to work
 386 // around it:
 387 //
 388 // bool PassStuffByRef::VisitLambdaExpr(const LambdaExpr * expr) {
 389 //     if (ignoreLocation(expr)) {
 390 //         return true;
 391 //     }
 392 //     for (auto i(expr->capture_begin()); i != expr->capture_end(); ++i) {
 393 //         if (i->getCaptureKind() == LambdaCaptureKind::LCK_ByCopy) {
 394 //             auto const t = i->getCapturedVar()->getType();
 395 //             if (isFat(t)) {
 396 //                 report(
 397 //                     DiagnosticsEngine::Warning,
 398 //                     ("%0 capture of %1 variable by copy, rather use capture"
 399 //                      " by reference---UNLESS THE LAMBDA OUTLIVES THE VARIABLE"),
 400 //                     i->getLocation())
 401 //                     << (i->isImplicit() ? "implicit" : "explicit") << t
 402 //                     << expr->getSourceRange();
 403 //             }
 404 //         }
 405 //     }
 406 //     return true;
 407 // }
 408
 409 bool PassStuffByRef::isFat(QualType type) {
 410     if (!type->isRecordType()) {
 411         return false;
 412     }
 413     loplugin::TypeCheck tc(type);
 414     if ((tc.Class("Reference").Namespace("uno").Namespace("star")
 415             .Namespace("sun").Namespace("com").GlobalNamespace())
 416         || (tc.Class("Sequence").Namespace("uno").Namespace("star")
 417             .Namespace("sun").Namespace("com").GlobalNamespace())
 418         || tc.Class("OString").Namespace("rtl").GlobalNamespace()
 419         || tc.Class("OUString").Namespace("rtl").GlobalNamespace()
 420         || tc.Class("Reference").Namespace("rtl").GlobalNamespace())
 421     {
 422         return true;
 423     }
 424     if (type->isIncompleteType()) {
 425         return false;
 426     }
 427     Type const * t2 = type.getTypePtrOrNull();
 428     return t2 != nullptr
 429         && compiler.getASTContext().getTypeSizeInChars(t2).getQuantity() > 64;
 430 }
 431
 432 bool PassStuffByRef::isPrimitiveConstRef(QualType type) {
 433     if (type->isIncompleteType()) {
 434         return false;
 435     }
 436     const clang::ReferenceType* referenceType = type->getAs<ReferenceType>();
 437     if (referenceType == nullptr) {
 438         return false;
 439     }
 440     QualType pointeeQT = referenceType->getPointeeType();
 441     if (!pointeeQT.isConstQualified()) {
 442         return false;
 443     }
 444     if (!pointeeQT->isFundamentalType()) {
 445         return false;
 446     }
 447     // ignore double for now, some of our code seems to believe it is cheaper to pass by ref
 448     const BuiltinType* builtinType = pointeeQT->getAs<BuiltinType>();
 449     return builtinType->getKind() != BuiltinType::Kind::Double;
 450 }
 451
 452
 453 loplugin::Plugin::Registration< PassStuffByRef > X("passstuffbyref");
 454
 455 }
 456
 457 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */