[Libreoffice-commits] core.git: canvas/source compilerplugins/clang drawinglayer/source fpicker/source i18npool/source sc/source svx/source tools/source
Noel Grandin (via logerrit)
logerrit at kemper.freedesktop.org
Tue Apr 27 20:13:24 UTC 2021
canvas/source/cairo/cairo_canvashelper.cxx | 3
compilerplugins/clang/store/optmove.cxx | 161 +++++++++++++++++++++
compilerplugins/clang/test/optmove.cxx | 53 ++++++
drawinglayer/source/tools/emfpregion.cxx | 3
fpicker/source/office/OfficeFilePicker.cxx | 3
i18npool/source/localedata/localedata.cxx | 6
sc/source/ui/docshell/docsh3.cxx | 2
sc/source/ui/unoobj/cellsuno.cxx | 3
svx/source/sdr/contact/objectcontactofpageview.cxx | 3
svx/source/svdraw/svdoedge.cxx | 2
tools/source/generic/b3dtrans.cxx | 3
11 files changed, 224 insertions(+), 18 deletions(-)
New commits:
commit 1325d8161a74a3cedc169952eca10f4343e700c4
Author: Noel Grandin <noel.grandin at collabora.co.uk>
AuthorDate: Tue Apr 27 14:59:07 2021 +0200
Commit: Noel Grandin <noel.grandin at collabora.co.uk>
CommitDate: Tue Apr 27 22:11:58 2021 +0200
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 at collabora.co.uk>
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 at +1 {{can std::move value instead of copy [loplugin:optmove]}}
+ y = x;
+}
+}
+
+namespace test2
+{
+void foo(std::vector<int> x)
+{
+ // expected-error at +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 at +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())
More information about the Libreoffice-commits
mailing list