new loplugin rangedforcopy - use reference in range based for

Inspired by 6e6ae9803796b120e95f6e89575e03c5fd0ed3c2

Change-Id: Ia0f264d3a6bbf076aa5080e3398683e50bc6ef01
Reviewed-on: https://gerrit.libreoffice.org/20190
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>

23 files changed, 104 insertions, 35 deletions

diff --git a/canvas/source/vcl/spritecanvashelper.cxx b/canvas/source/vcl/spritecanvashelper.cxx
index bba2e051bb33..892b241c7281 100644
--- a/canvas/source/vcl/spritecanvashelper.cxx
+++ b/canvas/source/vcl/spritecanvashelper.cxx
@@ -375,7 +375,7 @@ namespace vclcanvas
             // opaque sprite content)
 
             // repaint all affected sprites directly to output device
-            for( const auto rComponent : rUpdateArea.maComponentList )
+            for( const auto& rComponent : rUpdateArea.maComponentList )
             {
                 const ::canvas::Sprite::Reference& rSprite( rComponent.second.getSprite() );
 
diff --git a/comphelper/source/misc/accessiblewrapper.cxx b/comphelper/source/misc/accessiblewrapper.cxx
index 5e5262156e0d..2bb538fa5cbd 100644
--- a/comphelper/source/misc/accessiblewrapper.cxx
+++ b/comphelper/source/misc/accessiblewrapper.cxx
@@ -137,7 +137,7 @@ namespace comphelper
     void OWrappedAccessibleChildrenManager::dispose()
     {
         // dispose our children
-        for( const auto rChild : m_aChildrenMap )
+        for( const auto& rChild : m_aChildrenMap )
         {
             Reference< XComponent > xComp( rChild.first, UNO_QUERY );
             if( xComp.is() )
diff --git a/compilerplugins/clang/rangedforcopy.cxx b/compilerplugins/clang/rangedforcopy.cxx
new file mode 100644
index 000000000000..4c86fe34420e
--- /dev/null
+++ b/compilerplugins/clang/rangedforcopy.cxx
@@ -0,0 +1,66 @@
+
+/* -*- 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 <string>
+#include <iostream>
+
+#include "plugin.hxx"
+#include "compat.hxx"
+#include "clang/AST/CXXInheritance.h"
+
+// Check that we're not unnecessarily copying variables in a range based for loop
+// e.g. "for (OUString a: aList)" results in a copy of each string being made,
+// whereas "for (const OUString& a: aList)" does not.
+
+namespace
+{
+
+class RangedForCopy:
+    public RecursiveASTVisitor<RangedForCopy>, public loplugin::Plugin
+{
+public:
+    explicit RangedForCopy(InstantiationData const & data): Plugin(data) {}
+
+    virtual void run() override {
+        TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
+    }
+
+    bool VisitCXXForRangeStmt( const CXXForRangeStmt* stmt );
+};
+
+bool RangedForCopy::VisitCXXForRangeStmt( const CXXForRangeStmt* stmt )
+{
+    if (ignoreLocation( stmt ))
+        return true;
+
+    const VarDecl* varDecl = stmt->getLoopVariable();
+    if (!varDecl)
+      return true;
+
+    const QualType type = varDecl->getType();
+    if (type->isRecordType() && !type->isReferenceType() && !type->isPointerType())
+    {
+        std::string name = type.getAsString();
+        report(
+               DiagnosticsEngine::Warning,
+               "Loop variable passed by value, pass by reference instead, e.g. 'const %0&'",
+               varDecl->getLocStart())
+               << name << varDecl->getSourceRange();
+    }
+
+    return true;
+}
+
+
+loplugin::Plugin::Registration< RangedForCopy > X("rangedforcopy");
+
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/cui/source/options/personalization.cxx b/cui/source/options/personalization.cxx
index eb44d5fa4c6c..5ecc9f7eba32 100644
--- a/cui/source/options/personalization.cxx
+++ b/cui/source/options/personalization.cxx
@@ -102,9 +102,9 @@ void SelectPersonaDialog::dispose()
     m_pEdit.clear();
     m_pSearchButton.clear();
     m_pProgressLabel.clear();
-    for (VclPtr<PushButton> vp : m_vResultList)
+    for (VclPtr<PushButton>& vp : m_vResultList)
         vp.clear();
-    for (VclPtr<PushButton> vp : m_vSearchSuggestions)
+    for (VclPtr<PushButton>& vp : m_vSearchSuggestions)
         vp.clear();
     m_pOkButton.clear();
     m_pCancelButton.clear();
diff --git a/dbaccess/source/ui/dlg/queryorder.cxx b/dbaccess/source/ui/dlg/queryorder.cxx
index 9d7ff34d15c9..13fb716b4296 100644
--- a/dbaccess/source/ui/dlg/queryorder.cxx
+++ b/dbaccess/source/ui/dlg/queryorder.cxx
@@ -138,8 +138,8 @@ void DlgOrderCrit::dispose()
     m_pLB_ORDERVALUE2.clear();
     m_pLB_ORDERFIELD3.clear();
     m_pLB_ORDERVALUE3.clear();
-    for (auto a : m_aColumnList) a.clear();
-    for (auto a : m_aValueList) a.clear();
+    for (auto& a : m_aColumnList) a.clear();
+    for (auto& a : m_aValueList) a.clear();
     ModalDialog::dispose();
 }
 
diff --git a/dbaccess/source/ui/querydesign/JoinTableView.cxx b/dbaccess/source/ui/querydesign/JoinTableView.cxx
index 70379a872a0f..bfff9a8874a0 100644
--- a/dbaccess/source/ui/querydesign/JoinTableView.cxx
+++ b/dbaccess/source/ui/querydesign/JoinTableView.cxx
@@ -950,7 +950,7 @@ void OJoinTableView::InvalidateConnections()
 void OJoinTableView::DrawConnections(vcl::RenderContext& rRenderContext, const Rectangle& rRect)
 {
     // draw Joins
-    for(auto connection : m_vTableConnection)
+    for(const auto& connection : m_vTableConnection)
         connection->Draw(rRenderContext, rRect);
     // finally redraw the selected one above all others
     if (GetSelectedConn())
diff --git a/dbaccess/source/ui/querydesign/QueryTableView.cxx b/dbaccess/source/ui/querydesign/QueryTableView.cxx
index fb53cd47acb2..40f43aae2b7b 100644
--- a/dbaccess/source/ui/querydesign/QueryTableView.cxx
+++ b/dbaccess/source/ui/querydesign/QueryTableView.cxx
@@ -893,7 +893,7 @@ bool OQueryTableView::ShowTabWin( OQueryTableWindow* pTabWin, OQueryTabWinUndoAc
 
             // the Connections
             auto rTableCon = pUndoAction->GetTabConnList();
-            for(auto conn : rTableCon)
+            for(const auto& conn : rTableCon)
                 addConnection(conn); // add all connections from the undo action
 
             rTableCon.clear();
@@ -935,7 +935,7 @@ void OQueryTableView::InsertField(const OTableFieldDescRef& rInfo)
 
 bool OQueryTableView::ExistsAVisitedConn(const OQueryTableWindow* pFrom) const
 {
-    for(auto conn : getTableConnections())
+    for(const auto& conn : getTableConnections())
     {
         OQueryTableConnection* pTemp = static_cast<OQueryTableConnection*>(conn.get());
         if (pTemp->IsVisited() &&
diff --git a/dbaccess/source/ui/querydesign/TableWindowTitle.cxx b/dbaccess/source/ui/querydesign/TableWindowTitle.cxx
index 9db6d52b979c..8da722265150 100644
--- a/dbaccess/source/ui/querydesign/TableWindowTitle.cxx
+++ b/dbaccess/source/ui/querydesign/TableWindowTitle.cxx
@@ -142,7 +142,7 @@ void OTableWindowTitle::MouseButtonDown( const MouseEvent& rEvt )
 
                 OJoinTableView* pView = static_cast<OJoinTableView*>(m_pTabWin->getTableView());
                 OSL_ENSURE(pView,"No OJoinTableView!");
-                for (auto conn : pView->getTableConnections())
+                for (auto& conn : pView->getTableConnections())
                     conn->RecalcLines();
 
                 pView->InvalidateConnections();
diff --git a/desktop/source/deployment/registry/component/dp_component.cxx b/desktop/source/deployment/registry/component/dp_component.cxx
index b309f1882912..f739a67b11f4 100644
--- a/desktop/source/deployment/registry/component/dp_component.cxx
+++ b/desktop/source/deployment/registry/component/dp_component.cxx
@@ -1109,7 +1109,7 @@ Reference<XComponentContext> raise_uno_process(
     }
     catch (...) {
         OUString sMsg = "error starting process: " + url;
-        for(auto arg : args)
+        for(const auto& arg : args)
             sMsg += " " + arg;
         throw uno::RuntimeException(sMsg);
     }
diff --git a/pyuno/source/module/pyuno_struct.cxx b/pyuno/source/module/pyuno_struct.cxx
index 66d5d5630b26..9e58cb7a0e62 100644
--- a/pyuno/source/module/pyuno_struct.cxx
+++ b/pyuno/source/module/pyuno_struct.cxx
@@ -111,7 +111,7 @@ PyObject* PyUNOStruct_dir( PyObject *self )
     try
     {
         member_list = PyList_New( 0 );
-        for( auto aMember : me->members->xInvocation->getMemberNames() )
+        for( const auto& aMember : me->members->xInvocation->getMemberNames() )
         {
             // setitem steals a reference
             PyList_Append( member_list, ustring2PyString( aMember ).getAcquired() );
diff --git a/sc/source/filter/oox/worksheetbuffer.cxx b/sc/source/filter/oox/worksheetbuffer.cxx
index e4319194e785..8104f367ead7 100644
--- a/sc/source/filter/oox/worksheetbuffer.cxx
+++ b/sc/source/filter/oox/worksheetbuffer.cxx
@@ -234,7 +234,7 @@ void WorksheetBuffer::finalizeImport( sal_Int16 nActiveSheet )
 {
     ScDocument& rDoc = getScDocument();
 
-    for ( auto aSheetInfo: maSheetInfos )
+    for ( const auto& aSheetInfo: maSheetInfos )
     {
         // make sure at least 1 sheet (the active one) is visible
         if ( aSheetInfo->mnCalcSheet == nActiveSheet)
diff --git a/sc/source/ui/dbgui/pfiltdlg.cxx b/sc/source/ui/dbgui/pfiltdlg.cxx
index c7c8b758c9ca..6de187cfe378 100644
--- a/sc/source/ui/dbgui/pfiltdlg.cxx
+++ b/sc/source/ui/dbgui/pfiltdlg.cxx
@@ -107,9 +107,9 @@ void ScPivotFilterDlg::dispose()
     m_pBtnRegExp.clear();
     m_pBtnUnique.clear();
     m_pFtDbArea.clear();
-    for (auto a : aValueEdArr) a.clear();
-    for (auto a : aFieldLbArr) a.clear();
-    for (auto a : aCondLbArr) a.clear();
+    for (auto& a : aValueEdArr) a.clear();
+    for (auto& a : aFieldLbArr) a.clear();
+    for (auto& a : aCondLbArr) a.clear();
     ModalDialog::dispose();
 }
 
diff --git a/sc/source/ui/miscdlgs/optsolver.cxx b/sc/source/ui/miscdlgs/optsolver.cxx
index 76eac3a7538b..0744602ca764 100644
--- a/sc/source/ui/miscdlgs/optsolver.cxx
+++ b/sc/source/ui/miscdlgs/optsolver.cxx
@@ -346,13 +346,13 @@ void ScOptSolverDlg::dispose()
     m_pBtnCancel.clear();
     m_pBtnSolve.clear();
     mpEdActive.clear();
-    for (auto p : mpLeftButton)
+    for (auto& p : mpLeftButton)
         p.clear();
-    for (auto p : mpRightButton)
+    for (auto& p : mpRightButton)
         p.clear();
-    for (auto p : mpOperator)
+    for (auto& p : mpOperator)
         p.clear();
-    for (auto p : mpDelButton)
+    for (auto& p : mpDelButton)
         p.clear();
     ScAnyRefDlg::dispose();
 }
diff --git a/sc/source/ui/view/gridwin.cxx b/sc/source/ui/view/gridwin.cxx
index ecfd538ad142..bb1ae4311d43 100644
--- a/sc/source/ui/view/gridwin.cxx
+++ b/sc/source/ui/view/gridwin.cxx
@@ -5953,8 +5953,11 @@ static void updateLibreOfficeKitSelection(ScViewData* pViewData, ScDrawLayer* pD
     Rectangle aBoundingBox;
     std::vector<OString> aRectangles;
 
-    for (auto aRectangle : rRectangles)
+    for (const auto& rRectangle : rRectangles)
     {
+        // We explicitly create a copy, since we need to expand
+        // the rectangle before coordinate conversion
+        Rectangle aRectangle(rRectangle);
         aRectangle.Right() += 1;
         aRectangle.Bottom() += 1;
 
diff --git a/sd/source/filter/eppt/pptx-epptooxml.cxx b/sd/source/filter/eppt/pptx-epptooxml.cxx
index 0e5cdf3296c1..1e483ab58fc7 100644
--- a/sd/source/filter/eppt/pptx-epptooxml.cxx
+++ b/sd/source/filter/eppt/pptx-epptooxml.cxx
@@ -1419,7 +1419,7 @@ void PowerPointExport::WriteAuthors()
                          FSNS( XML_xmlns, XML_p ), "http://schemas.openxmlformats.org/presentationml/2006/main",
                          FSEND );
 
-    for( AuthorsMap::value_type i : maAuthors ) {
+    for( const AuthorsMap::value_type& i : maAuthors ) {
         pFS->singleElementNS( XML_p, XML_cmAuthor,
                               XML_id, I32S( i.second.nId ),
                               XML_name, USS( i.first ),
diff --git a/sd/source/ui/animations/SlideTransitionPane.cxx b/sd/source/ui/animations/SlideTransitionPane.cxx
index 2cb163cfb358..1f3cb27c719b 100644
--- a/sd/source/ui/animations/SlideTransitionPane.cxx
+++ b/sd/source/ui/animations/SlideTransitionPane.cxx
@@ -361,7 +361,7 @@ size_t getPresetOffset( const sd::impl::TransitionEffect &rEffect )
     sd::TransitionPresetPtr pFound;
 
     size_t nIdx = 0;
-    for( auto aIt: rPresetList )
+    for( const auto& aIt: rPresetList )
     {
         if( rEffect.operator==( *aIt ))
             break;
@@ -782,7 +782,7 @@ impl::TransitionEffect SlideTransitionPane::getTransitionEffectFromControls() co
         {
             int nVariant = 0;
             bool bFound = false;
-            for( auto aIter: rPresetList )
+            for( const auto& aIter: rPresetList )
             {
                 if( aIter->getSetId() == (*aSelected)->getSetId() )
                 {
@@ -1039,7 +1039,7 @@ void SlideTransitionPane::updateVariants( size_t nPresetOffset )
 
         // Fill in the variant listbox
         size_t nFirstItem = 0, nItem = 1;
-        for( auto aIt: rPresetList )
+        for( const auto& aIt: rPresetList )
         {
             if( aIt->getSetId().equals( (*pFound)->getSetId() ) )
             {
@@ -1117,7 +1117,7 @@ IMPL_LINK_NOARG_TYPED(SlideTransitionPane, LateInitCallback, Timer *, void)
     const TransitionPresetList& rPresetList = TransitionPreset::getTransitionPresetList();
 
     size_t nPresetOffset = 0;
-    for( auto aIter: rPresetList )
+    for( const auto& aIter: rPresetList )
     {
         TransitionPresetPtr pPreset = aIter;
         const OUString sLabel( pPreset->getSetLabel() );
@@ -1151,7 +1151,7 @@ IMPL_LINK_NOARG_TYPED(SlideTransitionPane, LateInitCallback, Timer *, void)
 
     nPresetOffset = 0;
     SAL_INFO( "sd.transitions", "Transition presets by offsets:");
-    for( auto aIter: rPresetList )
+    for( const auto& aIter: rPresetList )
     {
         SAL_INFO( "sd.transitions", nPresetOffset++ << " " <<
                   aIter->getPresetId() << ": " << aIter->getSetId() );
diff --git a/slideshow/source/engine/slideshowimpl.cxx b/slideshow/source/engine/slideshowimpl.cxx
index df655d8760d7..e837fd17f36d 100644
--- a/slideshow/source/engine/slideshowimpl.cxx
+++ b/slideshow/source/engine/slideshowimpl.cxx
@@ -1447,7 +1447,7 @@ void SlideShowImpl::registerUserPaintPolygons( const uno::Reference< lang::XMult
         //Get shapes for the slide
         css::uno::Reference< css::drawing::XShapes > Shapes(rPoly.first, css::uno::UNO_QUERY);
         //Retrieve polygons for one slide
-        for( const auto pPolyPoly : aPolygons )
+        for( const auto& pPolyPoly : aPolygons )
         {
             ::basegfx::B2DPolyPolygon b2DPolyPoly = ::basegfx::unotools::b2DPolyPolygonFromXPolyPolygon2D(pPolyPoly->getUNOPolyPolygon());
 
diff --git a/sw/source/core/doc/notxtfrm.cxx b/sw/source/core/doc/notxtfrm.cxx
index 1caa817cbe0b..8d128f6c831b 100644
--- a/sw/source/core/doc/notxtfrm.cxx
+++ b/sw/source/core/doc/notxtfrm.cxx
@@ -588,7 +588,7 @@ void SwNoTextFrame::Modify( const SfxPoolItem* pOld, const SfxPoolItem* pNew )
                 if( pNd->GetGrfObj().IsCached( pVSh->GetOut(), Point(),
                             Prt().SSize(), &pNd->GetGraphicAttr( aAttr, this ) ))
                 {
-                    for(SwViewShell rShell : pVSh->GetRingContainer())
+                    for(SwViewShell& rShell : pVSh->GetRingContainer())
                     {
                         SET_CURR_SHELL( &rShell );
                         if( rShell.GetWin() )
diff --git a/sw/source/core/frmedt/tblsel.cxx b/sw/source/core/frmedt/tblsel.cxx
index 40cf9bb9e488..94970d1b097d 100644
--- a/sw/source/core/frmedt/tblsel.cxx
+++ b/sw/source/core/frmedt/tblsel.cxx
@@ -1973,7 +1973,7 @@ bool CheckSplitCells( const SwCursor& rCursor, sal_uInt16 nDiv,
     ::MakeSelUnions( aUnions, pStart, pEnd, eSearchType );
 
     // now search boxes for each entry and emit
-    for ( auto rSelUnion : aUnions )
+    for ( const auto& rSelUnion : aUnions )
     {
         const SwTabFrame *pTable = rSelUnion.GetTable();
 
diff --git a/sw/source/core/unocore/unostyle.cxx b/sw/source/core/unocore/unostyle.cxx
index 6ced44aed257..48c3c9c68809 100644
--- a/sw/source/core/unocore/unostyle.cxx
+++ b/sw/source/core/unocore/unostyle.cxx
@@ -987,7 +987,7 @@ public:
             { m_vPropertyValues.clear(); }
     void Apply(SwXStyle& rStyle)
     {
-        for(auto pPropertyPair : m_vPropertyValues)
+        for(auto& pPropertyPair : m_vPropertyValues)
         {
             if(pPropertyPair.second.hasValue())
                 rStyle.setPropertyValue(pPropertyPair.first, pPropertyPair.second);
diff --git a/sw/source/core/unocore/unotbl.cxx b/sw/source/core/unocore/unotbl.cxx
index 2bf55f160156..bd6ced180468 100644
--- a/sw/source/core/unocore/unotbl.cxx
+++ b/sw/source/core/unocore/unotbl.cxx
@@ -3692,7 +3692,7 @@ void SwXCellRange::setLabelDescriptions(const uno::Sequence<OUString>& rDesc, bo
     if (sal::static_int_cast<sal_uInt32>(rDesc.getLength()) != vCells.size())
         throw uno::RuntimeException("Too few or too many descriptions", static_cast<cppu::OWeakObject*>(this));
     auto pDescIterator(rDesc.begin());
-    for(auto xCell : vCells)
+    for(auto& xCell : vCells)
         uno::Reference<text::XText>(xCell, uno::UNO_QUERY_THROW)->setString(*pDescIterator++);
 }
 void SwXCellRange::setRowDescriptions(const uno::Sequence<OUString>& rRowDesc)
diff --git a/sw/source/ui/table/tabledlg.cxx b/sw/source/ui/table/tabledlg.cxx
index 61a1d8d38f0b..bd335e1892d4 100644
--- a/sw/source/ui/table/tabledlg.cxx
+++ b/sw/source/ui/table/tabledlg.cxx
@@ -787,7 +787,7 @@ void SwTableColumnPage::dispose()
     m_pSpaceED.clear();
     m_pUpBtn.clear();
     m_pDownBtn.clear();
-    for (auto p : m_pTextArr)
+    for (auto& p : m_pTextArr)
         p.clear();
     SfxTabPage::dispose();
 }
diff --git a/unotools/source/misc/ServiceDocumenter.cxx b/unotools/source/misc/ServiceDocumenter.cxx
index 0cc5af5cbdd7..ad24713ec196 100644
--- a/unotools/source/misc/ServiceDocumenter.cxx
+++ b/unotools/source/misc/ServiceDocumenter.cxx
@@ -31,7 +31,7 @@ void unotools::misc::ServiceDocumenter::showInterfaceDocs(const Reference<XTypeP
         return;
     auto xMSF(m_xContext->getServiceManager());
     Reference<system::XSystemShellExecute> xShell(xMSF->createInstanceWithContext("com.sun.star.system.SystemShellExecute", m_xContext), uno::UNO_QUERY);
-    for(auto aType : xTypeProvider->getTypes())
+    for(const auto& aType : xTypeProvider->getTypes())
     {
         auto sUrl = aType.getTypeName();
         sal_Int32 nIdx = 0;
@@ -48,7 +48,7 @@ void unotools::misc::ServiceDocumenter::showServiceDocs(const Reference<XService
         return;
     auto xMSF(m_xContext->getServiceManager());
     Reference<system::XSystemShellExecute> xShell(xMSF->createInstanceWithContext("com.sun.star.system.SystemShellExecute", m_xContext), uno::UNO_QUERY);
-    for(auto sService : xService->getSupportedServiceNames())
+    for(const auto& sService : xService->getSupportedServiceNames())
     {
         auto sUrl = sService;
         sal_Int32 nIdx = 0;