compilerplugins/clang/moveparam.cxx | 127 +++++++++++ compilerplugins/clang/test/moveparam.cxx | 43 +++ drawinglayer/source/drawinglayeruno/xprimitive2drenderer.cxx | 4 drawinglayer/source/primitive2d/patternfillprimitive2d.cxx | 6 drawinglayer/source/tools/converters.cxx | 6 include/drawinglayer/converters.hxx | 2 solenv/CompilerTest_compilerplugins_clang.mk | 1 7 files changed, 180 insertions(+), 9 deletions(-)
New commits: commit 7934085eb95cc0ff39e948525f0cc2b80edc0169 Author: Noel Grandin <noel.gran...@collabora.co.uk> AuthorDate: Fri Oct 1 20:55:05 2021 +0200 Commit: Noel Grandin <noel.gran...@collabora.co.uk> CommitDate: Sat Oct 2 08:00:28 2021 +0200 new loplugin:moveparam Look for places where we can pass Primitive2DContainer by move reference and so avoid unnecessary copies. Change-Id: I1db167feba6d1a616ca6fc39778118ae20106bd1 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122964 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk> diff --git a/compilerplugins/clang/moveparam.cxx b/compilerplugins/clang/moveparam.cxx new file mode 100644 index 000000000000..46816184071f --- /dev/null +++ b/compilerplugins/clang/moveparam.cxx @@ -0,0 +1,127 @@ +/* -*- 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 <set> +#include <unordered_set> +#include "plugin.hxx" +#include "check.hxx" + +/* +Look for places where we can pass by Primitive2DContainer param and so avoid +unnecessary copies. +*/ + +namespace +{ +class MoveParam : public loplugin::FilteringPlugin<MoveParam> +{ +public: + explicit MoveParam(loplugin::InstantiationData const& data) + : FilteringPlugin(data) + { + } + + virtual bool preRun() override { return true; } + + virtual void run() override + { + if (preRun()) + TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); + } + + bool PreTraverseConstructorInitializer(CXXCtorInitializer*); + bool PostTraverseConstructorInitializer(CXXCtorInitializer*, bool); + bool TraverseConstructorInitializer(CXXCtorInitializer*); + bool VisitCXXOperatorCallExpr(const CXXOperatorCallExpr*); +}; + +bool MoveParam::VisitCXXOperatorCallExpr(const CXXOperatorCallExpr* callExpr) +{ + if (ignoreLocation(callExpr)) + return true; + if (!callExpr->isAssignmentOp()) + return true; + if (!loplugin::TypeCheck(callExpr->getType()) + .Class("Primitive2DContainer") + .Namespace("primitive2d")) + return true; + auto declRef = dyn_cast<DeclRefExpr>(callExpr->getArg(1)->IgnoreParenImpCasts()); + if (!declRef) + return true; + + auto parmVarDecl = dyn_cast_or_null<ParmVarDecl>(declRef->getDecl()); + if (!parmVarDecl) + return true; + + if (!loplugin::TypeCheck(parmVarDecl->getType()).LvalueReference().Const()) + return true; + + report(DiagnosticsEngine::Warning, "rather use move && param", compat::getBeginLoc(callExpr)); + + return true; +} + +bool MoveParam::PreTraverseConstructorInitializer(CXXCtorInitializer* init) +{ + if (ignoreLocation(init->getSourceLocation())) + return true; + const FieldDecl* fieldDecl = init->getAnyMember(); + if (!fieldDecl) + return true; + + auto dc = loplugin::TypeCheck(fieldDecl->getType()) + .Class("Primitive2DContainer") + .Namespace("primitive2d") + .Namespace("drawinglayer") + .GlobalNamespace(); + if (!dc) + return true; + + auto constructExpr = dyn_cast_or_null<CXXConstructExpr>(init->getInit()); + if (!constructExpr || constructExpr->getNumArgs() != 1) + return true; + + auto declRef = dyn_cast<DeclRefExpr>(constructExpr->getArg(0)->IgnoreParenImpCasts()); + if (!declRef) + return true; + + auto parmVarDecl = dyn_cast_or_null<ParmVarDecl>(declRef->getDecl()); + if (!parmVarDecl) + return true; + + if (!loplugin::TypeCheck(parmVarDecl->getType()).LvalueReference().Const()) + return true; + + report(DiagnosticsEngine::Warning, "rather use move && param", init->getSourceLocation()); + + return true; +} +bool MoveParam::PostTraverseConstructorInitializer(CXXCtorInitializer*, bool) { return true; } +bool MoveParam::TraverseConstructorInitializer(CXXCtorInitializer* init) +{ + bool ret = true; + if (PreTraverseConstructorInitializer(init)) + { + ret = FilteringPlugin<MoveParam>::TraverseConstructorInitializer(init); + PostTraverseConstructorInitializer(init, ret); + } + return ret; +} + +loplugin::Plugin::Registration<MoveParam> moveparam("moveparam", true); + +} // namespace + +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/compilerplugins/clang/test/moveparam.cxx b/compilerplugins/clang/test/moveparam.cxx new file mode 100644 index 000000000000..c90e8ae4bbd4 --- /dev/null +++ b/compilerplugins/clang/test/moveparam.cxx @@ -0,0 +1,43 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */ +/* + * This file is part of the LibreOffice project. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +#include "config_clang.h" +#include "o3tl/cow_wrapper.hxx" + +namespace drawinglayer::primitive2d +{ +class Primitive2DContainer +{ +}; +} + +struct Foo +{ + drawinglayer::primitive2d::Primitive2DContainer maMine; + + // expected-error@+2 {{rather use move && param [loplugin:moveparam]}} + Foo(drawinglayer::primitive2d::Primitive2DContainer const& rContainer) + : maMine(rContainer) + { + } + + // no warning expected + Foo(drawinglayer::primitive2d::Primitive2DContainer&& rContainer) + : maMine(rContainer) + { + } + + void foo1(const drawinglayer::primitive2d::Primitive2DContainer& rContainer) + { + // expected-error@+1 {{rather use move && param [loplugin:moveparam]}} + maMine = rContainer; + } +}; + +/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */ diff --git a/drawinglayer/source/drawinglayeruno/xprimitive2drenderer.cxx b/drawinglayer/source/drawinglayeruno/xprimitive2drenderer.cxx index 98ca81433f12..abca8a310925 100644 --- a/drawinglayer/source/drawinglayeruno/xprimitive2drenderer.cxx +++ b/drawinglayer/source/drawinglayeruno/xprimitive2drenderer.cxx @@ -134,11 +134,11 @@ namespace drawinglayer::unorenderer new primitive2d::TransformPrimitive2D( aEmbedding, comphelper::sequenceToContainer<primitive2d::Primitive2DContainer>(aPrimitive2DSequence))); - const primitive2d::Primitive2DContainer xEmbedSeq { xEmbedRef }; + primitive2d::Primitive2DContainer xEmbedSeq { xEmbedRef }; BitmapEx aBitmapEx( convertToBitmapEx( - xEmbedSeq, + std::move(xEmbedSeq), aViewInformation2D, nDiscreteWidth, nDiscreteHeight, diff --git a/drawinglayer/source/primitive2d/patternfillprimitive2d.cxx b/drawinglayer/source/primitive2d/patternfillprimitive2d.cxx index fc0538564896..cd4c58d11127 100644 --- a/drawinglayer/source/primitive2d/patternfillprimitive2d.cxx +++ b/drawinglayer/source/primitive2d/patternfillprimitive2d.cxx @@ -133,7 +133,7 @@ namespace drawinglayer::primitive2d const BitmapEx aBitmapEx( convertToBitmapEx( - xEmbedSeq, + std::move(xEmbedSeq), aViewInformation2D, mnDiscreteWidth, mnDiscreteHeight, @@ -191,10 +191,10 @@ namespace drawinglayer::primitive2d new primitive2d::TransformPrimitive2D( basegfx::utils::createScaleB2DHomMatrix(nWidth, nHeight), std::move(aContent))); - const primitive2d::Primitive2DContainer xEmbedSeq { xEmbedRef }; + primitive2d::Primitive2DContainer xEmbedSeq { xEmbedRef }; return convertToBitmapEx( - xEmbedSeq, + std::move(xEmbedSeq), aViewInformation2D, nWidth, nHeight, diff --git a/drawinglayer/source/tools/converters.cxx b/drawinglayer/source/tools/converters.cxx index 1f51384480cd..382b81197526 100644 --- a/drawinglayer/source/tools/converters.cxx +++ b/drawinglayer/source/tools/converters.cxx @@ -36,7 +36,7 @@ namespace drawinglayer { BitmapEx convertToBitmapEx( - const drawinglayer::primitive2d::Primitive2DContainer& rSeq, + drawinglayer::primitive2d::Primitive2DContainer&& rSeq, const geometry::ViewInformation2D& rViewInformation2D, sal_uInt32 nDiscreteWidth, sal_uInt32 nDiscreteHeight, @@ -61,12 +61,12 @@ namespace drawinglayer const drawinglayer::primitive2d::Primitive2DReference aEmbed( new drawinglayer::primitive2d::TransformPrimitive2D( basegfx::utils::createScaleB2DHomMatrix(fReduceFactor, fReduceFactor), - primitive2d::Primitive2DContainer(rSeq))); + std::move(rSeq))); aSequence = drawinglayer::primitive2d::Primitive2DContainer { aEmbed }; } else - aSequence = rSeq; + aSequence = std::move(rSeq); const Point aEmptyPoint; const Size aSizePixel(nDiscreteWidth, nDiscreteHeight); diff --git a/include/drawinglayer/converters.hxx b/include/drawinglayer/converters.hxx index 64d0e2ce7d4e..a0b8c4c4be78 100644 --- a/include/drawinglayer/converters.hxx +++ b/include/drawinglayer/converters.hxx @@ -25,7 +25,7 @@ namespace drawinglayer { BitmapEx DRAWINGLAYER_DLLPUBLIC -convertToBitmapEx(const drawinglayer::primitive2d::Primitive2DContainer& rSeq, +convertToBitmapEx(drawinglayer::primitive2d::Primitive2DContainer&& rSeq, const geometry::ViewInformation2D& rViewInformation2D, sal_uInt32 nDiscreteWidth, sal_uInt32 nDiscreteHeight, sal_uInt32 nMaxSquarePixels); diff --git a/solenv/CompilerTest_compilerplugins_clang.mk b/solenv/CompilerTest_compilerplugins_clang.mk index 0bbfcefe9db1..0521606ebe63 100644 --- a/solenv/CompilerTest_compilerplugins_clang.mk +++ b/solenv/CompilerTest_compilerplugins_clang.mk @@ -49,6 +49,7 @@ $(eval $(call gb_CompilerTest_add_exception_objects,compilerplugins_clang, \ compilerplugins/clang/test/loopvartoosmall \ compilerplugins/clang/test/mapindex \ compilerplugins/clang/test/makeshared \ + compilerplugins/clang/test/moveparam \ compilerplugins/clang/test/namespaceindentation \ compilerplugins/clang/test/noexcept \ compilerplugins/clang/test/noexceptmove \