summaryrefslogtreecommitdiff
path: root/compilerplugins/clang/moveparam.cxx
diff options
context:
space:
mode:
authorNoel Grandin <noel.grandin@collabora.co.uk>2021-10-12 09:56:59 +0200
committerNoel Grandin <noel.grandin@collabora.co.uk>2021-10-12 13:12:11 +0200
commitd968425f009598bca3d10964c64f093b8d785c86 (patch)
treed2243b3dce0159044e5c112dac5fc54b702efe51 /compilerplugins/clang/moveparam.cxx
parentd19299f579e820c34b38ee56b26125dc30f96ada (diff)
loplugin:moveparam check for collection params
Empirically, when we are passing a collection type to a constructor, 80% of the time, we are passing a local temporary that can be moved instead of being copied. Change-Id: I5acc9d761c920691934a4be806a3d3ab6cdbab96 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/123056 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Diffstat (limited to 'compilerplugins/clang/moveparam.cxx')
-rw-r--r--compilerplugins/clang/moveparam.cxx128
1 files changed, 96 insertions, 32 deletions
diff --git a/compilerplugins/clang/moveparam.cxx b/compilerplugins/clang/moveparam.cxx
index 46816184071f..930e8a61a927 100644
--- a/compilerplugins/clang/moveparam.cxx
+++ b/compilerplugins/clang/moveparam.cxx
@@ -19,8 +19,13 @@
#include "check.hxx"
/*
-Look for places where we can pass by Primitive2DContainer param and so avoid
+Look for places where we can pass by move && param and so avoid
unnecessary copies.
+Empirically, when we are passing a container type to a function, 80% of the time,
+we are passing a local temporary that can be moved instead of being copied.
+
+TODO this could be a lot smarter, with ignoring false+ e.g. when copying a param
+in a loop
*/
namespace
@@ -33,7 +38,24 @@ public:
{
}
- virtual bool preRun() override { return true; }
+ virtual bool preRun() override
+ {
+ std::string fn(handler.getMainFileName());
+ loplugin::normalizeDotDotInFilePath(fn);
+ if (loplugin::hasPathnamePrefix(fn, SRCDIR "/filter/source/msfilter/escherex.cxx"))
+ return false;
+ if (loplugin::hasPathnamePrefix(fn, SRCDIR "/sc/source/ui/docshell/docfunc.cxx"))
+ return false;
+ if (loplugin::hasPathnamePrefix(fn, SRCDIR "/sc/source/ui/view/viewfunc.cxx"))
+ return false;
+ if (loplugin::hasPathnamePrefix(fn, SRCDIR "/basegfx/source/polygon/b2dpolygontools.cxx"))
+ return false;
+ if (loplugin::hasPathnamePrefix(fn, SRCDIR "/basegfx/source/polygon/b3dpolygontools.cxx"))
+ return false;
+ if (loplugin::hasPathnamePrefix(fn, SRCDIR "/connectivity/source/commontools/dbtools.cxx"))
+ return false;
+ return true;
+ }
virtual void run() override
{
@@ -41,10 +63,10 @@ public:
TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
}
- bool PreTraverseConstructorInitializer(CXXCtorInitializer*);
- bool PostTraverseConstructorInitializer(CXXCtorInitializer*, bool);
- bool TraverseConstructorInitializer(CXXCtorInitializer*);
bool VisitCXXOperatorCallExpr(const CXXOperatorCallExpr*);
+ bool VisitCXXConstructExpr(const CXXConstructExpr*);
+
+ bool isContainerType(QualType qt);
};
bool MoveParam::VisitCXXOperatorCallExpr(const CXXOperatorCallExpr* callExpr)
@@ -53,9 +75,8 @@ bool MoveParam::VisitCXXOperatorCallExpr(const CXXOperatorCallExpr* callExpr)
return true;
if (!callExpr->isAssignmentOp())
return true;
- if (!loplugin::TypeCheck(callExpr->getType())
- .Class("Primitive2DContainer")
- .Namespace("primitive2d"))
+ auto qt = callExpr->getType();
+ if (!isContainerType(qt))
return true;
auto declRef = dyn_cast<DeclRefExpr>(callExpr->getArg(1)->IgnoreParenImpCasts());
if (!declRef)
@@ -68,29 +89,29 @@ bool MoveParam::VisitCXXOperatorCallExpr(const CXXOperatorCallExpr* callExpr)
if (!loplugin::TypeCheck(parmVarDecl->getType()).LvalueReference().Const())
return true;
- report(DiagnosticsEngine::Warning, "rather use move && param", compat::getBeginLoc(callExpr));
+ StringRef aFileName = getFilenameOfLocation(
+ compiler.getSourceManager().getSpellingLoc(compat::getBeginLoc(parmVarDecl)));
+ if (loplugin::hasPathnamePrefix(aFileName,
+ SRCDIR "/svx/source/sidebar/line/LineWidthValueSet.cxx"))
+ return true;
+
+ report(DiagnosticsEngine::Warning, "rather use move && param1", compat::getBeginLoc(callExpr));
return true;
}
-bool MoveParam::PreTraverseConstructorInitializer(CXXCtorInitializer* init)
+bool MoveParam::VisitCXXConstructExpr(const CXXConstructExpr* constructExpr)
{
- if (ignoreLocation(init->getSourceLocation()))
+ if (ignoreLocation(compat::getBeginLoc(constructExpr)))
return true;
- const FieldDecl* fieldDecl = init->getAnyMember();
- if (!fieldDecl)
+ if (isInUnoIncludeFile(compat::getBeginLoc(constructExpr)))
return true;
- auto dc = loplugin::TypeCheck(fieldDecl->getType())
- .Class("Primitive2DContainer")
- .Namespace("primitive2d")
- .Namespace("drawinglayer")
- .GlobalNamespace();
- if (!dc)
+ auto qt = constructExpr->getType();
+ if (!isContainerType(qt))
return true;
- auto constructExpr = dyn_cast_or_null<CXXConstructExpr>(init->getInit());
- if (!constructExpr || constructExpr->getNumArgs() != 1)
+ if (constructExpr->getNumArgs() != 1)
return true;
auto declRef = dyn_cast<DeclRefExpr>(constructExpr->getArg(0)->IgnoreParenImpCasts());
@@ -104,23 +125,66 @@ bool MoveParam::PreTraverseConstructorInitializer(CXXCtorInitializer* init)
if (!loplugin::TypeCheck(parmVarDecl->getType()).LvalueReference().Const())
return true;
- report(DiagnosticsEngine::Warning, "rather use move && param", init->getSourceLocation());
+ StringRef aFileName = getFilenameOfLocation(
+ compiler.getSourceManager().getSpellingLoc(compat::getBeginLoc(parmVarDecl)));
+ if (loplugin::hasPathnamePrefix(aFileName, SRCDIR
+ "/include/drawinglayer/primitive2d/Primitive2DContainer.hxx"))
+ return true;
+ if (loplugin::hasPathnamePrefix(aFileName,
+ SRCDIR "/include/drawinglayer/primitive3d/baseprimitive3d.hxx"))
+ return true;
+ if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/svx/source/svdraw/svdmrkv.cxx"))
+ return true;
+ if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/include/editeng/swafopt.hxx"))
+ return true;
+ if (loplugin::hasPathnamePrefix(
+ aFileName, SRCDIR "/drawinglayer/source/primitive2d/textdecoratedprimitive2d.cxx"))
+ return true;
+ if (loplugin::hasPathnamePrefix(aFileName,
+ SRCDIR "/chart2/source/tools/InternalDataProvider.cxx"))
+ return true;
+ if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sc/source/core/data/attrib.cxx"))
+ return true;
+ if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sw/source/core/doc/docfmt.cxx"))
+ return true;
+ if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/configmgr/source/modifications.cxx"))
+ return true;
+ if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/svx/source/dialog/srchdlg.cxx"))
+ return true;
+ if (loplugin::hasPathnamePrefix(aFileName,
+ SRCDIR "/stoc/source/servicemanager/servicemanager.cxx"))
+ return true;
+
+ report(DiagnosticsEngine::Warning, "rather use move && param3",
+ compat::getBeginLoc(constructExpr));
return true;
}
-bool MoveParam::PostTraverseConstructorInitializer(CXXCtorInitializer*, bool) { return true; }
-bool MoveParam::TraverseConstructorInitializer(CXXCtorInitializer* init)
+
+bool MoveParam::isContainerType(QualType qt)
{
- bool ret = true;
- if (PreTraverseConstructorInitializer(init))
- {
- ret = FilteringPlugin<MoveParam>::TraverseConstructorInitializer(init);
- PostTraverseConstructorInitializer(init, ret);
- }
- return ret;
+ auto tc = loplugin::TypeCheck(qt);
+ return tc.Class("Primitive2DContainer")
+ .Namespace("primitive2d")
+ .Namespace("drawinglayer")
+ .GlobalNamespace()
+ || tc.ClassOrStruct("sorted_vector").Namespace("o3tl").GlobalNamespace()
+ || tc.ClassOrStruct("array").StdNamespace() || tc.ClassOrStruct("vector").StdNamespace()
+ || tc.ClassOrStruct("deque").StdNamespace()
+ || tc.ClassOrStruct("forward_list").StdNamespace()
+ || tc.ClassOrStruct("list").StdNamespace() || tc.ClassOrStruct("set").StdNamespace()
+ || tc.ClassOrStruct("map").StdNamespace() || tc.ClassOrStruct("multiset").StdNamespace()
+ || tc.ClassOrStruct("multimap").StdNamespace()
+ || tc.ClassOrStruct("unordered_set").StdNamespace()
+ || tc.ClassOrStruct("unordered_map").StdNamespace()
+ || tc.ClassOrStruct("unordered_multiset").StdNamespace()
+ || tc.ClassOrStruct("unordered_multimap").StdNamespace()
+ || tc.ClassOrStruct("stack").StdNamespace() || tc.ClassOrStruct("queue").StdNamespace()
+ || tc.ClassOrStruct("priority_queue").StdNamespace();
}
-loplugin::Plugin::Registration<MoveParam> moveparam("moveparam", true);
+/** off by default because it needs some hand-holding */
+loplugin::Plugin::Registration<MoveParam> moveparam("moveparam", false);
} // namespace