don't use heap for elements in ScRangePairList

no need to store small objects like this out of line.

And use std::array for ScRangePair, so we get range checking in debug
mode.

Change-Id: Ie4690edbb4c3fdc4e08206cb016b8167a399d95a
Reviewed-on: https://gerrit.libreoffice.org/51268
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

8 files changed, 88 insertions, 96 deletions

diff --git a/sc/inc/address.hxx b/sc/inc/address.hxx
index ddc85469d349..37a401ef0194 100644
--- a/sc/inc/address.hxx
+++ b/sc/inc/address.hxx
@@ -23,6 +23,7 @@
 #include <rtl/ustrbuf.hxx>
 #include <rtl/strbuf.hxx>
 
+#include <array>
 #include <limits>
 #include <ostream>
 
@@ -763,10 +764,10 @@ inline bool ValidRange( const ScRange& rRange )
 }
 
 //  ScRangePair
-class SAL_WARN_UNUSED ScRangePair
+class SAL_WARN_UNUSED ScRangePair final
 {
 private:
-    ScRange aRange[2];
+    std::array<ScRange,2> aRange;
 
 public:
     ScRangePair( const ScRangePair& r )
diff --git a/sc/inc/rangelst.hxx b/sc/inc/rangelst.hxx
index e137613df857..6a1afadad037 100644
--- a/sc/inc/rangelst.hxx
+++ b/sc/inc/rangelst.hxx
@@ -129,28 +129,27 @@ public:
     ScRangePairList*    Clone() const;
     void                Append( const ScRangePair& rRangePair )
                         {
-                            ScRangePair* pR = new ScRangePair( rRangePair );
-                            maPairs.push_back( pR );
+                            maPairs.push_back( rRangePair );
                         }
     void                Join( const ScRangePair&, bool bIsInList = false );
     void                UpdateReference( UpdateRefMode, const ScDocument*,
                                     const ScRange& rWhere,
                                     SCCOL nDx, SCROW nDy, SCTAB nDz );
     void                DeleteOnTab( SCTAB nTab );
-    ScRangePair*        Find( const ScAddress& ) const;
-    ScRangePair*        Find( const ScRange& ) const;
-    std::vector<ScRangePair*>
+    ScRangePair*        Find( const ScAddress& );
+    ScRangePair*        Find( const ScRange& );
+    std::vector<const ScRangePair*>
                         CreateNameSortedArray( ScDocument* ) const;
 
     void                Remove(size_t nPos);
-    void                Remove(const ScRangePair* pAdr);
+    void                Remove(const ScRangePair & rAdr);
 
     size_t              size() const;
-    ScRangePair*        operator[](size_t idx);
-    const ScRangePair*  operator[](size_t idx) const;
+    ScRangePair&        operator[](size_t idx);
+    const ScRangePair&  operator[](size_t idx) const;
 
 private:
-    ::std::vector< ScRangePair* > maPairs;
+    ::std::vector< ScRangePair > maPairs;
 };
 typedef tools::SvRef<ScRangePairList> ScRangePairListRef;
 
diff --git a/sc/source/core/data/documen3.cxx b/sc/source/core/data/documen3.cxx
index ff79d99e347e..98a3f403bc30 100644
--- a/sc/source/core/data/documen3.cxx
+++ b/sc/source/core/data/documen3.cxx
@@ -1661,9 +1661,9 @@ void ScDocument::GetFormulaEntries( ScTypedCaseStrSet& rStrings )
 
         for ( size_t i = 0, nPairs = pList->size(); i < nPairs; ++i )
         {
-            ScRangePair* pPair = (*pList)[i];
-            ScRange aRange = pPair->GetRange(0);
-            ScCellIterator aIter( this, aRange );
+            const ScRangePair & rPair = (*pList)[i];
+            const ScRange & rRange = rPair.GetRange(0);
+            ScCellIterator aIter( this, rRange );
             for (bool bHas = aIter.first(); bHas; bHas = aIter.next())
             {
                 if (!aIter.hasString())
diff --git a/sc/source/core/tool/compiler.cxx b/sc/source/core/tool/compiler.cxx
index 4b574a3a2bca..2b22c08171a8 100644
--- a/sc/source/core/tool/compiler.cxx
+++ b/sc/source/core/tool/compiler.cxx
@@ -3482,8 +3482,8 @@ bool ScCompiler::IsColRowName( const OUString& rName )
                 pRL = pDoc->GetRowNameRanges();
             for ( size_t iPair = 0, nPairs = pRL->size(); iPair < nPairs && !bInList; ++iPair )
             {
-                ScRangePair* pR = (*pRL)[iPair];
-                const ScRange& rNameRange = pR->GetRange(0);
+                const ScRangePair & rR = (*pRL)[iPair];
+                const ScRange& rNameRange = rR.GetRange(0);
                 if ( jThisTab && !(rNameRange.aStart.Tab() <= nThisTab &&
                         nThisTab <= rNameRange.aEnd.Tab()) )
                     continue;   // for
@@ -5252,11 +5252,11 @@ bool ScCompiler::HandleColRowName()
     ScRange aRange;
     for ( size_t i = 0, nPairs = pRL->size(); i < nPairs; ++i )
     {
-        ScRangePair* pR = (*pRL)[i];
-        if ( pR->GetRange(0).In( aAbs ) )
+        const ScRangePair & rR = (*pRL)[i];
+        if ( rR.GetRange(0).In( aAbs ) )
         {
             bInList = bValidName = true;
-            aRange = pR->GetRange(1);
+            aRange = rR.GetRange(1);
             if ( bColName )
             {
                 aRange.aStart.SetCol( nCol );
@@ -5300,8 +5300,8 @@ bool ScCompiler::HandleColRowName()
                 }
                 for ( size_t i = 0, nPairs = pRL->size(); i < nPairs; ++i )
                 {   // next defined ColNameRange below limits row
-                    ScRangePair* pR = (*pRL)[i];
-                    const ScRange& rRange = pR->GetRange(1);
+                    const ScRangePair & rR = (*pRL)[i];
+                    const ScRange& rRange = rR.GetRange(1);
                     if ( rRange.aStart.Col() <= nCol && nCol <= rRange.aEnd.Col() )
                     {   // identical column range
                         SCROW nTmp = rRange.aStart.Row();
@@ -5333,8 +5333,8 @@ bool ScCompiler::HandleColRowName()
                 }
                 for ( size_t i = 0, nPairs = pRL->size(); i < nPairs; ++i )
                 {   // next defined RowNameRange to the right limits column
-                    ScRangePair* pR = (*pRL)[i];
-                    const ScRange& rRange = pR->GetRange(1);
+                    const ScRangePair & rR = (*pRL)[i];
+                    const ScRange& rRange = rR.GetRange(1);
                     if ( rRange.aStart.Row() <= nRow && nRow <= rRange.aEnd.Row() )
                     {   // identical row range
                         SCCOL nTmp = rRange.aStart.Col();
diff --git a/sc/source/core/tool/rangelst.cxx b/sc/source/core/tool/rangelst.cxx
index 62a083bcab96..5380a7aa321d 100644
--- a/sc/source/core/tool/rangelst.cxx
+++ b/sc/source/core/tool/rangelst.cxx
@@ -1187,8 +1187,6 @@ ScRangeList ScRangeList::GetIntersectedRange(const ScRange& rRange) const
 //  ScRangePairList
 ScRangePairList::~ScRangePairList()
 {
-    for_each( maPairs.begin(), maPairs.end(), std::default_delete<ScRangePair>() );
-    maPairs.clear();
 }
 
 void ScRangePairList::Remove(size_t nPos)
@@ -1196,23 +1194,15 @@ void ScRangePairList::Remove(size_t nPos)
     if (maPairs.size() <= nPos)
         // Out-of-bound condition.  Bail out.
         return;
-
-    vector<ScRangePair*>::iterator itr = maPairs.begin();
-    advance(itr, nPos);
-    delete *itr;
-    maPairs.erase(itr);
+    maPairs.erase(maPairs.begin() + nPos);
 }
 
-void ScRangePairList::Remove( const ScRangePair* Adr)
+void ScRangePairList::Remove( const ScRangePair & rAdr)
 {
-    if (Adr == nullptr)
-        return;
-
-    for ( vector<ScRangePair*>::iterator itr = maPairs.begin(); itr != maPairs.end(); ++itr )
+    for ( auto itr = maPairs.begin(); itr != maPairs.end(); ++itr )
     {
-        if (Adr == *itr)
+        if (&rAdr == &*itr)
         {
-            delete *itr;
             maPairs.erase( itr );
             return;
         }
@@ -1220,12 +1210,12 @@ void ScRangePairList::Remove( const ScRangePair* Adr)
     assert(false);
 }
 
-ScRangePair* ScRangePairList::operator [](size_t idx)
+ScRangePair & ScRangePairList::operator [](size_t idx)
 {
     return maPairs[idx];
 }
 
-const ScRangePair* ScRangePairList::operator [](size_t idx) const
+const ScRangePair & ScRangePairList::operator [](size_t idx) const
 {
     return maPairs[idx];
 }
@@ -1248,11 +1238,11 @@ void ScRangePairList::UpdateReference( UpdateRefMode eUpdateRefMode,
         SCROW nRow2;
         SCTAB nTab2;
         rWhere.GetVars( nCol1, nRow1, nTab1, nCol2, nRow2, nTab2 );
-        for (ScRangePair* pR : maPairs)
+        for (ScRangePair & rR : maPairs)
         {
             for ( sal_uInt16 j=0; j<2; j++ )
             {
-                ScRange& rRange = pR->GetRange(j);
+                ScRange& rRange = rR.GetRange(j);
                 SCCOL theCol1;
                 SCROW theRow1;
                 SCTAB theTab1;
@@ -1277,38 +1267,35 @@ void ScRangePairList::UpdateReference( UpdateRefMode eUpdateRefMode,
 // Delete entries that have the labels (first range) on nTab
 void ScRangePairList::DeleteOnTab( SCTAB nTab )
 {
-    size_t nListCount = maPairs.size();
-    size_t nPos = 0;
-    while ( nPos < nListCount )
+    for (auto it = maPairs.begin(); it != maPairs.end(); )
     {
-        ScRangePair* pR = maPairs[  nPos ];
-        ScRange aRange = pR->GetRange(0);
-        if ( aRange.aStart.Tab() == nTab && aRange.aEnd.Tab() == nTab )
+        const ScRangePair & rR = *it;
+        const ScRange & rRange = rR.GetRange(0);
+        if ( rRange.aStart.Tab() == nTab && rRange.aEnd.Tab() == nTab )
         {
-            Remove( nPos );
-            nListCount = maPairs.size();
+            it = maPairs.erase(it);
         }
         else
-            ++nPos;
+            ++it;
     }
 }
 
-ScRangePair* ScRangePairList::Find( const ScAddress& rAdr ) const
+ScRangePair* ScRangePairList::Find( const ScAddress& rAdr )
 {
-    for (ScRangePair* pR : maPairs)
+    for (ScRangePair & rR : maPairs)
     {
-        if ( pR->GetRange(0).In( rAdr ) )
-            return pR;
+        if ( rR.GetRange(0).In( rAdr ) )
+            return &rR;
     }
     return nullptr;
 }
 
-ScRangePair* ScRangePairList::Find( const ScRange& rRange ) const
+ScRangePair* ScRangePairList::Find( const ScRange& rRange )
 {
-    for (ScRangePair* pR : maPairs)
+    for (ScRangePair & rR : maPairs)
     {
-        if ( pR->GetRange(0) == rRange )
-            return pR;
+        if ( rR.GetRange(0) == rRange )
+            return &rR;
     }
     return nullptr;
 }
@@ -1316,9 +1303,9 @@ ScRangePair* ScRangePairList::Find( const ScRange& rRange ) const
 ScRangePairList* ScRangePairList::Clone() const
 {
     ScRangePairList* pNew = new ScRangePairList;
-    for (const ScRangePair* pR : maPairs)
+    for (const ScRangePair & rR : maPairs)
     {
-        pNew->Append( *pR );
+        pNew->Append( rR );
     }
     return pNew;
 }
@@ -1425,15 +1412,15 @@ Label_RangePair_Join:
     size_t nOverPos = std::numeric_limits<size_t>::max();
     for (size_t i = 0; i < maPairs.size(); ++i)
     {
-        ScRangePair* p = maPairs[ i ];
-        if ( p == pOver )
+        ScRangePair & rPair = maPairs[ i ];
+        if ( &rPair == pOver )
         {
             nOverPos = i;
             continue;           // the same one, continue with the next
         }
         bool bJoined = false;
-        ScRange& rp1 = p->GetRange(0);
-        ScRange& rp2 = p->GetRange(1);
+        ScRange& rp1 = rPair.GetRange(0);
+        ScRange& rp2 = rPair.GetRange(1);
         if ( rp2 == r2 )
         {   // only if Range2 is equal
             if ( rp1.In( r1 ) )
@@ -1448,7 +1435,7 @@ Label_RangePair_Join:
             }
             else if ( r1.In( rp1 ) )
             {   // RangePair p included in RangePair pOver, make pOver the new RangePair
-                *p = *pOver;
+                rPair = *pOver;
                 bJoined = true;
             }
         }
@@ -1500,21 +1487,26 @@ Label_RangePair_Join:
             if ( bIsInList )
             {   // delete RangePair pOver within the list
                 if (nOverPos != std::numeric_limits<size_t>::max())
+                {
                     Remove(nOverPos);
+                    if (nOverPos < i)
+                        --i;
+                }
                 else
                 {
-                    for (size_t nOver = 0, nPairs = maPairs.size(); nOver < nPairs; ++nOver)
+                    for (size_t nOver = 0, nRangePairs = maPairs.size(); nOver < nRangePairs; ++nOver)
                     {
-                        if (maPairs[nOver] == pOver)
+                        if (&maPairs[nOver] == pOver)
                         {
-                            Remove(nOver);
+                            maPairs.erase(maPairs.begin() + nOver);
                             break;
                         }
                     }
+                    assert(false);
                 }
             }
             bJoinedInput = true;
-            pOver = p;
+            pOver = &maPairs[i];
             bIsInList = true;
             goto Label_RangePair_Join;
         }
@@ -1523,9 +1515,14 @@ Label_RangePair_Join:
         Append( r );
 }
 
-std::vector<ScRangePair*> ScRangePairList::CreateNameSortedArray( ScDocument* pDoc ) const
+std::vector<const ScRangePair*> ScRangePairList::CreateNameSortedArray( ScDocument* pDoc ) const
 {
-    std::vector<ScRangePair*> aSortedVec(maPairs);
+    std::vector<const ScRangePair*> aSortedVec(maPairs.size());
+    size_t i = 0;
+    for ( auto const & rPair : maPairs)
+    {
+        aSortedVec[i++] = &rPair;
+    }
 
     std::sort( aSortedVec.begin(), aSortedVec.end(), ScRangePairList_sortNameCompare(pDoc) );
 
diff --git a/sc/source/filter/excel/xecontent.cxx b/sc/source/filter/excel/xecontent.cxx
index 473200a13c5a..8cd7de4b5066 100644
--- a/sc/source/filter/excel/xecontent.cxx
+++ b/sc/source/filter/excel/xecontent.cxx
@@ -551,8 +551,8 @@ void XclExpLabelranges::FillRangeList( ScRangeList& rScRanges,
 {
     for ( size_t i = 0, nPairs = xLabelRangesRef->size(); i < nPairs; ++i )
     {
-        ScRangePair* pRangePair = (*xLabelRangesRef)[i];
-        const ScRange& rScRange = pRangePair->GetRange( 0 );
+        const ScRangePair & rRangePair = (*xLabelRangesRef)[i];
+        const ScRange& rScRange = rRangePair.GetRange( 0 );
         if( rScRange.aStart.Tab() == nScTab )
             rScRanges.push_back( rScRange );
     }
diff --git a/sc/source/ui/miscdlgs/crnrdlg.cxx b/sc/source/ui/miscdlgs/crnrdlg.cxx
index 31df267e218f..8cbdf77a2b1c 100644
--- a/sc/source/ui/miscdlgs/crnrdlg.cxx
+++ b/sc/source/ui/miscdlgs/crnrdlg.cxx
@@ -391,7 +391,7 @@ void ScColRowNameRangesDlg::UpdateNames()
     pLbRange->SetEntryData( nPos, reinterpret_cast<void*>(nEntryDataDelim) );
     if ( (nCount = xColNameRanges->size()) > 0 )
     {
-        std::vector<ScRangePair*> aSortArray(xColNameRanges->CreateNameSortedArray(
+        std::vector<const ScRangePair*> aSortArray(xColNameRanges->CreateNameSortedArray(
                pDoc ));
         nCount = aSortArray.size();
         for ( j=0; j < nCount; j++ )
@@ -435,7 +435,7 @@ void ScColRowNameRangesDlg::UpdateNames()
     pLbRange->SetEntryData( nPos, reinterpret_cast<void*>(nEntryDataDelim) );
     if ( (nCount = xRowNameRanges->size()) > 0 )
     {
-        std::vector<ScRangePair*> aSortArray(xRowNameRanges->CreateNameSortedArray(
+        std::vector<const ScRangePair*> aSortArray(xRowNameRanges->CreateNameSortedArray(
                pDoc ));
         nCount = aSortArray.size();
         for ( j=0; j < nCount; j++ )
@@ -557,11 +557,11 @@ IMPL_LINK_NOARG(ScColRowNameRangesDlg, AddBtnHdl, Button*, void)
             ScRangePair* pPair;
             if ( ( pPair = xColNameRanges->Find( theCurArea ) ) != nullptr )
             {
-                xColNameRanges->Remove( pPair );
+                xColNameRanges->Remove( *pPair );
             }
             if ( ( pPair = xRowNameRanges->Find( theCurArea ) ) != nullptr )
             {
-                xRowNameRanges->Remove( pPair );
+                xRowNameRanges->Remove( *pPair );
             }
             if ( pBtnColHead->IsChecked() )
                 xColNameRanges->Join( ScRangePair( theCurArea, theCurData ) );
@@ -619,9 +619,9 @@ IMPL_LINK_NOARG(ScColRowNameRangesDlg, RemoveBtnHdl, Button*, void)
         if (RET_YES == QUERYBOX(GetFrameWeld(), aMsg))
         {
             if ( bColName )
-                xColNameRanges->Remove( pPair );
+                xColNameRanges->Remove( *pPair );
             else
-                xRowNameRanges->Remove( pPair );
+                xRowNameRanges->Remove( *pPair );
 
             UpdateNames();
             const sal_Int32 nCnt = pLbRange->GetEntryCount();
diff --git a/sc/source/ui/unoobj/nameuno.cxx b/sc/source/ui/unoobj/nameuno.cxx
index 1caa62e9920c..e1e7d1478c28 100644
--- a/sc/source/ui/unoobj/nameuno.cxx
+++ b/sc/source/ui/unoobj/nameuno.cxx
@@ -1041,9 +1041,8 @@ ScLabelRangeObj* ScLabelRangesObj::GetObjectByIndex_Impl(size_t nIndex)
         ScRangePairList* pList = bColumn ? rDoc.GetColNameRanges() : rDoc.GetRowNameRanges();
         if ( pList && nIndex < pList->size() )
         {
-            ScRangePair* pData = (*pList)[nIndex];
-            if (pData)
-                return new ScLabelRangeObj( pDocShell, bColumn, pData->GetRange(0) );
+            ScRangePair & rData = (*pList)[nIndex];
+            return new ScLabelRangeObj( pDocShell, bColumn, rData.GetRange(0) );
         }
     }
     return nullptr;
@@ -1094,23 +1093,19 @@ void SAL_CALL ScLabelRangesObj::removeByIndex( sal_Int32 nIndex )
         {
             ScRangePairListRef xNewList(pOldList->Clone());
 
-            ScRangePair* pEntry = (*xNewList)[nIndex];
-            if (pEntry)
-            {
-                xNewList->Remove( pEntry );
+            xNewList->Remove( nIndex );
 
-                if (bColumn)
-                    rDoc.GetColNameRangesRef() = xNewList;
-                else
-                    rDoc.GetRowNameRangesRef() = xNewList;
+            if (bColumn)
+                rDoc.GetColNameRangesRef() = xNewList;
+            else
+                rDoc.GetRowNameRangesRef() = xNewList;
 
-                rDoc.CompileColRowNameFormula();
-                pDocShell->PostPaint( 0,0,0, MAXCOL,MAXROW,MAXTAB, PaintPartFlags::Grid );
-                pDocShell->SetDocumentModified();
-                bDone = true;
+            rDoc.CompileColRowNameFormula();
+            pDocShell->PostPaint( 0,0,0, MAXCOL,MAXROW,MAXTAB, PaintPartFlags::Grid );
+            pDocShell->SetDocumentModified();
+            bDone = true;
 
-                //! Undo ?!?! (here and from dialog)
-            }
+            //! Undo ?!?! (here and from dialog)
         }
     }
     if (!bDone)