diff options
Diffstat (limited to 'compilerplugins/clang/moveit.cxx')
-rw-r--r-- | compilerplugins/clang/moveit.cxx | 250 |
1 files changed, 250 insertions, 0 deletions
diff --git a/compilerplugins/clang/moveit.cxx b/compilerplugins/clang/moveit.cxx new file mode 100644 index 000000000..353359e28 --- /dev/null +++ b/compilerplugins/clang/moveit.cxx @@ -0,0 +1,250 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ +/* + * This file is part of the LibreOffice project. + * + * Based on LLVM/Clang. + * + * This file is distributed under the University of Illinois Open Source + * License. See LICENSE.TXT for details. + * + */ + +#include <cassert> +#include <string> +#include <iostream> +#include <fstream> +#include "config_clang.h" +#include "plugin.hxx" +#include "check.hxx" + +/* +Look for local variables that can be std::move'd into parameters. + +TODO +(*) Ideally we would use a proper data-flow analysis, to detect that the var is dead after this point, + like the one in clang at include/clang/Analysis/CFG.h +(*) we could expand the set of approved/interesting types +*/ + +namespace +{ +class MoveIt : public loplugin::FilteringPlugin<MoveIt> +{ +public: + explicit MoveIt(loplugin::InstantiationData const& data) + : FilteringPlugin(data) + { + } + + virtual bool preRun() override + { + std::string fn(handler.getMainFileName()); + loplugin::normalizeDotDotInFilePath(fn); + // false + + if (loplugin::hasPathnamePrefix(fn, SRCDIR "/basic/source/classes/sbunoobj.cxx")) + return false; + if (loplugin::hasPathnamePrefix(fn, SRCDIR "/svx/source/svdraw/textchainflow.cxx")) + return false; + if (loplugin::hasPathnamePrefix(fn, SRCDIR "/svx/source/unodraw/unoshtxt.cxx")) + return false; + if (loplugin::hasPathnamePrefix(fn, + SRCDIR "/connectivity/source/drivers/dbase/dindexnode.cxx")) + return false; + if (loplugin::hasPathnamePrefix(fn, SRCDIR "/basctl/source/basicide/moduldlg.cxx")) + return false; + if (loplugin::hasPathnamePrefix(fn, SRCDIR "/sw/source/core/SwNumberTree/SwNumberTree.cxx")) + return false; + if (loplugin::hasPathnamePrefix(fn, SRCDIR "/sc/source/core/tool/scmatrix.cxx")) + return false; + if (loplugin::hasPathnamePrefix(fn, SRCDIR "/sw/source/core/bastyp/calc.cxx")) + return false; + if (loplugin::hasPathnamePrefix(fn, SRCDIR "/sw/source/core/edit/edattr.cxx")) + return false; + if (loplugin::hasPathnamePrefix(fn, SRCDIR "/sw/source/core/fields/expfld.cxx")) + return false; + if (loplugin::hasPathnamePrefix(fn, SRCDIR "/sw/source/uibase/docvw/edtwin.cxx")) + return false; + if (loplugin::hasPathnamePrefix(fn, SRCDIR "/sw/source/uibase/uiview/viewling.cxx")) + return false; + if (loplugin::hasPathnamePrefix( + fn, SRCDIR "/writerfilter/source/dmapper/DomainMapperTableHandler.cxx")) + return false; + return true; + } + + virtual void run() override + { + if (preRun()) + TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); + } + + bool VisitCXXMemberCallExpr(const CXXMemberCallExpr*); + bool VisitCXXConstructExpr(const CXXConstructExpr*); + +private: + bool isInterestingType(QualType); +}; + +bool MoveIt::VisitCXXMemberCallExpr(const CXXMemberCallExpr* topExpr) +{ + if (ignoreLocation(topExpr)) + return true; + const CXXMethodDecl* methodDecl = topExpr->getMethodDecl(); + if (!methodDecl) + return true; + + unsigned len = std::min(topExpr->getNumArgs(), methodDecl->getNumParams()); + for (unsigned i = 0; i < len; ++i) + { + // check if the parameter is a moveable type + const ParmVarDecl* parmVarDecl = methodDecl->getParamDecl(i); + if (!parmVarDecl->getType()->isRecordType()) + continue; + const CXXRecordDecl* recordDecl + = dyn_cast<CXXRecordDecl>(parmVarDecl->getType()->getAsRecordDecl()); + if (!recordDecl || !recordDecl->hasMoveConstructor() || recordDecl->isTriviallyCopyable()) + continue; + if (!isInterestingType(parmVarDecl->getType())) + continue; + + // check if (a) we're making a copy to pass to the param and (b) we're making a copy of a local var + const Expr* argExpr = topExpr->getArg(i); + if (!argExpr) + continue; + const CXXConstructExpr* argSubExpr = dyn_cast<CXXConstructExpr>(argExpr->IgnoreImplicit()); + if (!argSubExpr || argSubExpr->getNumArgs() == 0) + continue; + const DeclRefExpr* dre = dyn_cast<DeclRefExpr>(argSubExpr->getArg(0)->IgnoreImplicit()); + if (!dre) + continue; + const VarDecl* localVarDecl = dyn_cast<VarDecl>(dre->getDecl()); + if (!localVarDecl || localVarDecl->getType()->isReferenceType() + || localVarDecl->getType()->isPointerType() || !localVarDecl->hasLocalStorage()) + continue; + // because sometimes the parameter type is some obscured STL thing + if (!isInterestingType(localVarDecl->getType())) + continue; + + report(DiagnosticsEngine::Warning, "can std::move this var into this param", + argExpr->getBeginLoc()); + report(DiagnosticsEngine::Note, "passing to this param", parmVarDecl->getBeginLoc()); + report(DiagnosticsEngine::Note, "local var declared here", localVarDecl->getBeginLoc()); + report(DiagnosticsEngine::Note, "type declared here", recordDecl->getBeginLoc()); + // parmVarDecl->getType()->dump(); + } + + // StringRef aFileName = getFilenameOfLocation( + // compiler.getSourceManager().getSpellingLoc(parmVarDecl->getBeginLoc())); + // if (loplugin::hasPathnamePrefix(aFileName, + // SRCDIR "/svx/source/sidebar/line/LineWidthValueSet.cxx")) + // return true; + + return true; +} + +bool MoveIt::VisitCXXConstructExpr(const CXXConstructExpr* topExpr) +{ + if (ignoreLocation(topExpr)) + return true; + if (isa<CXXTemporaryObjectExpr>(topExpr)) + return true; + const CXXConstructorDecl* methodDecl = topExpr->getConstructor(); + if (!methodDecl) + return true; + + unsigned len = std::min(topExpr->getNumArgs(), methodDecl->getNumParams()); + for (unsigned i = 0; i < len; ++i) + { + // check if the parameter is a moveable type + const ParmVarDecl* parmVarDecl = methodDecl->getParamDecl(i); + if (!parmVarDecl->getType()->isRecordType()) + continue; + const CXXRecordDecl* recordDecl + = dyn_cast<CXXRecordDecl>(parmVarDecl->getType()->getAsRecordDecl()); + if (!recordDecl || !recordDecl->hasMoveConstructor() || recordDecl->isTriviallyCopyable()) + continue; + if (!isInterestingType(parmVarDecl->getType())) + continue; + + // check if (a) we're making a copy to pass to the param and (b) we're making a copy of a local var + const Expr* argExpr = topExpr->getArg(i); + if (!argExpr) + continue; + const CXXConstructExpr* argSubExpr = dyn_cast<CXXConstructExpr>(argExpr->IgnoreImplicit()); + if (!argSubExpr || argSubExpr->getNumArgs() == 0) + continue; + const DeclRefExpr* dre = dyn_cast<DeclRefExpr>(argSubExpr->getArg(0)->IgnoreImplicit()); + if (!dre) + continue; + const VarDecl* localVarDecl = dyn_cast<VarDecl>(dre->getDecl()); + if (!localVarDecl || localVarDecl->getType()->isReferenceType() + || localVarDecl->getType()->isPointerType() || !localVarDecl->hasLocalStorage()) + continue; + // because sometimes the parameter type is some obscured STL thing + if (!isInterestingType(localVarDecl->getType())) + continue; + + report(DiagnosticsEngine::Warning, "can std::move this var into this param", + argExpr->getBeginLoc()); + report(DiagnosticsEngine::Note, "passing to this param", parmVarDecl->getBeginLoc()); + report(DiagnosticsEngine::Note, "local var declared here", localVarDecl->getBeginLoc()); + report(DiagnosticsEngine::Note, "type declared here", recordDecl->getBeginLoc()); + // parmVarDecl->getType()->dump(); + } + + // StringRef aFileName = getFilenameOfLocation( + // compiler.getSourceManager().getSpellingLoc(parmVarDecl->getBeginLoc())); + // if (loplugin::hasPathnamePrefix(aFileName, + // SRCDIR "/svx/source/sidebar/line/LineWidthValueSet.cxx")) + // return true; + + return true; +} + +/// Exclude boring types, so that we don't generate too many low-value conversions. +/// e.g. for now I ignore ref-counted types like Sequence and OUString and css::uno::Reference, +/// because that generates too many changes +bool MoveIt::isInterestingType(QualType qt) +{ + if (qt->isEnumeralType()) + return false; + if (!qt->isRecordType()) + return false; + + auto tc = loplugin::TypeCheck(qt); + + // clang-format off + return !tc.ClassOrStruct("iterator") + && !tc.ClassOrStruct("const_iterator") + && !tc.Typedef("iterator") + && !tc.Typedef("const_iterator") + && !tc.Class("_Safe_iterator") + && !tc.Typedef("string") + && !tc.ClassOrStruct("shared_ptr").StdNamespace() + && !tc.ClassOrStruct("shared_ptr").Namespace("boost") + && !tc.Class("B2DHomMatrix").Namespace("basegfx").GlobalNamespace() + && !tc.Class("Pipe").Namespace("osl") + && !tc.Class("Any").Namespace("uno") + && !tc.Class("TypeDescription").Namespace("uno") + && !tc.Class("UnoInterfaceReference").Namespace("uno") + && !tc.Class("ByteSequence").Namespace("rtl").GlobalNamespace() + && !tc.Class("OUString").Namespace("rtl").GlobalNamespace() + && !tc.Class("OString").Namespace("rtl").GlobalNamespace() + && !tc.Class("BinaryAny") + && !tc.Class("Reference") + && !tc.Class("SvRef").Namespace("tools").GlobalNamespace() + && !tc.ClassOrStruct("sk_sp") // skia shared pointer + && !tc.ClassOrStruct("VclPtr") + && !tc.Typedef("IterString") // SalInstanceTreeView::IterString + && !tc.Typedef("svtree_render_args") + && !tc.Typedef("render_args") // weld::ComboBox::render_args + ; + // clang-format on +} + +/// off by default because each warning needs to be hand checked to ensure it is not a false+ +loplugin::Plugin::Registration<MoveIt> moveit("moveit", false); + +} // namespace + +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ |