loplugin:moveopt

An attempt that did not find anything convincing enough to finish it up
and make it permanently active.
So just leave it in /store for now.

Change-Id: I1750e177655a4a510da100f880ba81bf762be277
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/114742
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

11 files changed, 224 insertions, 18 deletions

diff --git a/canvas/source/cairo/cairo_canvashelper.cxx b/canvas/source/cairo/cairo_canvashelper.cxx
index 8e96fe5f9994..d308961a0865 100644
--- a/canvas/source/cairo/cairo_canvashelper.cxx
+++ b/canvas/source/cairo/cairo_canvashelper.cxx
@@ -364,14 +364,13 @@ namespace cairocanvas
                 tools::Long nHeight;
                 vcl::bitmap::CanvasCairoExtractBitmapData(aBmpEx, aBitmap, data, bHasAlpha, nWidth, nHeight);
 
-                SurfaceSharedPtr pImageSurface = rSurfaceProvider->getOutputDevice()->CreateSurface(
+                pSurface = rSurfaceProvider->getOutputDevice()->CreateSurface(
                     CairoSurfaceSharedPtr(
                         cairo_image_surface_create_for_data(
                             data,
                             bHasAlpha ? CAIRO_FORMAT_ARGB32 : CAIRO_FORMAT_RGB24,
                             nWidth, nHeight, nWidth*4 ),
                         &cairo_surface_destroy) );
-                pSurface = pImageSurface;
 
                 SAL_INFO( "canvas.cairo","image: " << nWidth << " x " << nHeight << " alpha: " << bHasAlpha);
             }
diff --git a/compilerplugins/clang/store/optmove.cxx b/compilerplugins/clang/store/optmove.cxx
new file mode 100644
index 000000000000..a4831179ed14
--- /dev/null
+++ b/compilerplugins/clang/store/optmove.cxx
@@ -0,0 +1,161 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * 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 "plugin.hxx"
+#include "check.hxx"
+
+#include <string>
+#include <set>
+
+/**
+ * This plugin is unfinished, abandoned because it did not find anything interesting.
+ *
+ * Look for variables that are
+ * (a) copied from
+ * (b) never used after the copy
+ * (c) have move operators
+ *
+ * The intention being to find places where we can move data (e.g. in containers) instead of copying.
+*/
+
+namespace
+{
+class OptMove : public loplugin::FilteringPlugin<OptMove>
+{
+public:
+    explicit OptMove(loplugin::InstantiationData const& data)
+        : FilteringPlugin(data)
+    {
+    }
+
+    virtual void run() override
+    {
+        TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
+
+        for (auto const& pair : m_Candidates)
+        {
+            //auto varDecl = pair.first;
+            auto candidate = pair.second;
+            if (!candidate.canUseExpr)
+                continue;
+            report(DiagnosticsEngine::Warning, "can std::move value instead of copy",
+                   candidate.canUseExpr->getSourceRange().getBegin())
+                << candidate.canUseExpr->getSourceRange();
+            //varDecl->dump();
+        }
+    }
+
+    bool VisitVarDecl(const VarDecl*);
+    bool VisitCXXOperatorCallExpr(const CXXOperatorCallExpr*);
+    bool VisitDeclRefExpr(const DeclRefExpr*);
+    bool VisitFunctionDecl(const FunctionDecl* f)
+    {
+        if (f->getIdentifier() && f->getName() == "foo")
+            f->dump();
+        return true;
+    }
+
+private:
+    struct Candidate
+    {
+        const DeclRefExpr* operatorArg1 = nullptr;
+        const Expr* canUseExpr = nullptr;
+    };
+    std::map<const VarDecl*, Candidate> m_Candidates;
+};
+
+bool OptMove::VisitVarDecl(const VarDecl* varDecl)
+{
+    if (ignoreLocation(varDecl))
+        return true;
+    if (varDecl->hasGlobalStorage())
+        return true;
+    if (varDecl->getLinkageAndVisibility().getLinkage() == ExternalLinkage)
+        return true;
+    if (!varDecl->getType()->isRecordType())
+        return true;
+
+    auto cxxRecord = dyn_cast<CXXRecordDecl>(varDecl->getType()->getAsRecordDecl());
+    if (!cxxRecord || !cxxRecord->hasDefinition() || !cxxRecord->hasMoveAssignment())
+        return true;
+    // ignore our simpler types for now, I'm after bigger game
+    auto typeName = cxxRecord->getName();
+    if (typeName.contains("Reference") || typeName.contains("Color") || typeName.contains("VclPtr")
+        || typeName.contains("OString") || typeName.contains("OUString")
+        || typeName.contains("Rectangle") || typeName.contains("Size")
+        || typeName.contains("Selection") || typeName.contains("Point")
+        || typeName.contains("strong_int"))
+        return true;
+    m_Candidates.emplace(varDecl, Candidate());
+
+    if (!varDecl->hasInit())
+        return true;
+    auto cons = dyn_cast<CXXConstructExpr>(varDecl->getInit());
+    if (!cons || !cons->getConstructor()->isCopyConstructor())
+        return true;
+    auto arg1 = dyn_cast<DeclRefExpr>(compat::IgnoreImplicit(cons->getArg(0)));
+    if (!arg1)
+        return true;
+    auto varDecl1 = dyn_cast<VarDecl>(arg1->getDecl());
+    if (!varDecl1)
+        return true;
+    auto it = m_Candidates.find(varDecl1);
+    if (it == m_Candidates.end())
+        return true;
+    it->second.operatorArg1 = arg1;
+    it->second.canUseExpr = cons;
+    return true;
+}
+
+bool OptMove::VisitCXXOperatorCallExpr(CXXOperatorCallExpr const* cxxOperatorCallExpr)
+{
+    if (ignoreLocation(cxxOperatorCallExpr))
+        return true;
+    auto op = cxxOperatorCallExpr->getOperator();
+    if (op != OO_Equal)
+        return true;
+    auto arg0 = dyn_cast<DeclRefExpr>(compat::IgnoreImplicit(cxxOperatorCallExpr->getArg(0)));
+    auto arg1 = dyn_cast<DeclRefExpr>(compat::IgnoreImplicit(cxxOperatorCallExpr->getArg(1)));
+    if (!arg0 || !arg1)
+        return true;
+    auto varDecl0 = dyn_cast<VarDecl>(arg0->getDecl());
+    auto varDecl1 = dyn_cast<VarDecl>(arg1->getDecl());
+    if (!varDecl0 || !varDecl1)
+        return true;
+    auto cxxMethodDecl = dyn_cast_or_null<CXXMethodDecl>(cxxOperatorCallExpr->getDirectCallee());
+    if (!cxxMethodDecl || !cxxMethodDecl->isCopyAssignmentOperator())
+        return true;
+    auto it = m_Candidates.find(varDecl1);
+    if (it == m_Candidates.end())
+        return true;
+    it->second.operatorArg1 = arg1;
+    it->second.canUseExpr = cxxOperatorCallExpr;
+    return true;
+}
+
+bool OptMove::VisitDeclRefExpr(const DeclRefExpr* declRefExpr)
+{
+    if (ignoreLocation(declRefExpr))
+        return true;
+    auto varDecl = dyn_cast<VarDecl>(declRefExpr->getDecl());
+    if (!varDecl)
+        return true;
+    auto it = m_Candidates.find(varDecl);
+    if (it == m_Candidates.end())
+        return true;
+    if (it->second.operatorArg1 == declRefExpr)
+        return true;
+    m_Candidates.erase(it);
+    return true;
+}
+
+loplugin::Plugin::Registration<OptMove> noexceptmove("optmove");
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/compilerplugins/clang/test/optmove.cxx b/compilerplugins/clang/test/optmove.cxx
new file mode 100644
index 000000000000..976806772f7e
--- /dev/null
+++ b/compilerplugins/clang/test/optmove.cxx
@@ -0,0 +1,53 @@
+/* -*- 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 <vector>
+
+namespace test1
+{
+void foo1(std::vector<int> x)
+{
+    std::vector<int> y;
+    // expected-error@+1 {{can std::move value instead of copy [loplugin:optmove]}}
+    y = x;
+}
+}
+
+namespace test2
+{
+void foo(std::vector<int> x)
+{
+    // expected-error@+1 {{can std::move value instead of copy [loplugin:optmove]}}
+    std::vector<int> y = x;
+}
+}
+
+namespace test3
+{
+void foo1(std::vector<int> x)
+{
+    std::vector<int> y, z;
+    y = x;
+    // expected-error@+1 {{can std::move value instead of copy [loplugin:optmove]}}
+    z = x;
+}
+}
+
+namespace test4
+{
+void foo1(std::vector<int> x)
+{
+    std::vector<int> y;
+    // no warning expected, don't even try to follow loop
+    for (int i = 0; i < 10; i++)
+        y = x;
+}
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/drawinglayer/source/tools/emfpregion.cxx b/drawinglayer/source/tools/emfpregion.cxx
index 7813e23e24ae..4856d3b08fef 100644
--- a/drawinglayer/source/tools/emfpregion.cxx
+++ b/drawinglayer/source/tools/emfpregion.cxx
@@ -70,14 +70,13 @@ namespace emfplushelper
 
             const ::basegfx::B2DPoint mappedStartPoint(rR.Map(dx, dy));
             const ::basegfx::B2DPoint mappedEndPoint(rR.Map(dx + dw, dy + dh));
-            const ::basegfx::B2DPolyPolygon polyPolygon(
+            polygon = ::basegfx::B2DPolyPolygon(
                     ::basegfx::utils::createPolygonFromRect(
                         ::basegfx::B2DRectangle(
                             mappedStartPoint.getX(),
                             mappedStartPoint.getY(),
                             mappedEndPoint.getX(),
                             mappedEndPoint.getY())));
-            polygon = polyPolygon;
             break;
         }
         case RegionNodeDataTypePath:
diff --git a/fpicker/source/office/OfficeFilePicker.cxx b/fpicker/source/office/OfficeFilePicker.cxx
index dd8e0d58203d..3386979e4714 100644
--- a/fpicker/source/office/OfficeFilePicker.cxx
+++ b/fpicker/source/office/OfficeFilePicker.cxx
@@ -144,8 +144,7 @@ void SvtFilePicker::prepareExecute()
                 aPath = givenPath;
             else
             {
-                INetURLObject aStdDirObj( SvtPathOptions().GetWorkPath() );
-                aPath = aStdDirObj;
+                aPath = INetURLObject( SvtPathOptions().GetWorkPath() );
             }
             if ( !m_aDefaultName.isEmpty() )
             {
diff --git a/i18npool/source/localedata/localedata.cxx b/i18npool/source/localedata/localedata.cxx
index 273415d9461e..8bc822936b7b 100644
--- a/i18npool/source/localedata/localedata.cxx
+++ b/i18npool/source/localedata/localedata.cxx
@@ -719,10 +719,9 @@ Sequence< CalendarItem2 > LocaleDataImpl::getCalendarItems(
             case REF_PMONTHS:
                 for (CalendarItem2& rItem : aItems)
                 {
-                    CalendarItem2 item{ OUString(allCalendars[rnOffset]),
+                    rItem = CalendarItem2{ OUString(allCalendars[rnOffset]),
                             OUString(allCalendars[rnOffset+1]),
                             OUString(allCalendars[rnOffset+2]), OUString(allCalendars[rnOffset+3])};
-                    rItem = item;
                     rnOffset += 4;
                 }
                 break;
@@ -730,10 +729,9 @@ Sequence< CalendarItem2 > LocaleDataImpl::getCalendarItems(
                 // Absent narrow name.
                 for (CalendarItem2& rItem : aItems)
                 {
-                    CalendarItem2 item{ OUString(allCalendars[rnOffset]),
+                    rItem = CalendarItem2{ OUString(allCalendars[rnOffset]),
                             OUString(allCalendars[rnOffset+1]),
                             OUString(allCalendars[rnOffset+2]), OUString()};
-                    rItem = item;
                     rnOffset += 3;
                 }
                 break;
diff --git a/sc/source/ui/docshell/docsh3.cxx b/sc/source/ui/docshell/docsh3.cxx
index 8cb71adc0ac0..aec3aa7d5620 100644
--- a/sc/source/ui/docshell/docsh3.cxx
+++ b/sc/source/ui/docshell/docsh3.cxx
@@ -1124,7 +1124,7 @@ void ScDocShell::MergeDocument( ScDocument& rOtherDoc, bool bShared, bool bCheck
         pSourceAction = pSourceAction->GetNext();
     }
 
-    rMarkData = aOldMarkData;
+    rMarkData = std::move(aOldMarkData);
     pThisTrack->SetUser(aOldUser);
     pThisTrack->SetUseFixDateTime( false );
 
diff --git a/sc/source/ui/unoobj/cellsuno.cxx b/sc/source/ui/unoobj/cellsuno.cxx
index bd234108221b..7cf078fde6c5 100644
--- a/sc/source/ui/unoobj/cellsuno.cxx
+++ b/sc/source/ui/unoobj/cellsuno.cxx
@@ -999,8 +999,7 @@ void ScHelperFunctions::FillBoxItems( SvxBoxItem& rOuter, SvxBoxInfoItem& rInner
 void ScHelperFunctions::FillBorderLine( table::BorderLine& rStruct, const ::editeng::SvxBorderLine* pLine )
 {
     // Convert from Twips to 1/100mm.
-    table::BorderLine2 aStruct( SvxBoxItem::SvxLineToLine( pLine, true));
-    rStruct = aStruct;
+    rStruct = SvxBoxItem::SvxLineToLine( pLine, true);
 }
 
 void ScHelperFunctions::FillBorderLine( table::BorderLine2& rStruct, const ::editeng::SvxBorderLine* pLine )
diff --git a/svx/source/sdr/contact/objectcontactofpageview.cxx b/svx/source/sdr/contact/objectcontactofpageview.cxx
index 41717d276381..3d234b6fdd65 100644
--- a/svx/source/sdr/contact/objectcontactofpageview.cxx
+++ b/svx/source/sdr/contact/objectcontactofpageview.cxx
@@ -187,8 +187,7 @@ namespace sdr::contact
                 {
                     // get logic clip range and create discrete one
                     const tools::Rectangle aLogicClipRectangle(rDisplayInfo.GetRedrawArea().GetBoundRect());
-                    basegfx::B2DRange aLogicClipRange = vcl::unotools::b2DRectangleFromRectangle(aLogicClipRectangle);
-                    basegfx::B2DRange aDiscreteClipRange(aLogicClipRange);
+                    basegfx::B2DRange aDiscreteClipRange = vcl::unotools::b2DRectangleFromRectangle(aLogicClipRectangle);
                     aDiscreteClipRange.transform(rTargetOutDev.GetViewTransformation());
 
                     // align the discrete one to discrete boundaries (pixel bounds). Also
diff --git a/svx/source/svdraw/svdoedge.cxx b/svx/source/svdraw/svdoedge.cxx
index e31e2bf8a85e..512d7b048bdc 100644
--- a/svx/source/svdraw/svdoedge.cxx
+++ b/svx/source/svdraw/svdoedge.cxx
@@ -841,7 +841,7 @@ XPolygon SdrEdgeObj::ImpCalcEdgeTrack(const XPolygon& rTrack0, SdrObjConnection&
                         XPolygon aXP(ImpCalcEdgeTrack(aPt1,nA1,aBoundRect1,aBewareRect1,aPt2,nA2,aBoundRect2,aBewareRect2,&nQual,&aInfo));
                         if (nQual<nBestQual)
                         {
-                            aBestXP=aXP;
+                            aBestXP=std::move(aXP);
                             nBestQual=nQual;
                             aBestInfo=aInfo;
                             nBestAuto1=nNum1;
diff --git a/tools/source/generic/b3dtrans.cxx b/tools/source/generic/b3dtrans.cxx
index 1f63cc2e381d..623dc99c3c68 100644
--- a/tools/source/generic/b3dtrans.cxx
+++ b/tools/source/generic/b3dtrans.cxx
@@ -414,8 +414,7 @@ void B3dCamera::DeviceRectangleChange()
 
 void B3dCamera::CalcNewViewportValues()
 {
-    basegfx::B3DVector aViewVector(aPosition - aLookAt);
-    basegfx::B3DVector aNewVPN(aViewVector);
+    basegfx::B3DVector aNewVPN(aPosition - aLookAt);
 
     basegfx::B3DVector aNewVUV(0.0, 1.0, 0.0);
     if(aNewVPN.getLength() < aNewVPN.getY())