formula: remove indirection and use unique_ptr

Change-Id: Ib0c083803024d223f62b91ec54850b84eb68a758
Reviewed-on: https://gerrit.libreoffice.org/39033
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Jochen Nitschke <j.nitschke+logerrit@ok.de>

5 files changed, 45 insertions, 53 deletions

diff --git a/formula/source/core/api/FormulaCompiler.cxx b/formula/source/core/api/FormulaCompiler.cxx
index 24b7d15d723b..2f82f3b022a6 100644
--- a/formula/source/core/api/FormulaCompiler.cxx
+++ b/formula/source/core/api/FormulaCompiler.cxx
@@ -389,8 +389,8 @@ uno::Sequence< sheet::FormulaToken > FormulaCompiler::OpCodeMap::createSequenceO
     OUString const * const pStop = pName + nLen;
     for ( ; pName < pStop; ++pName, ++pToken)
     {
-        OpCodeHashMap::const_iterator iLook( mpHashMap->find( *pName));
-        if (iLook != mpHashMap->end())
+        OpCodeHashMap::const_iterator iLook( maHashMap.find( *pName));
+        if (iLook != maHashMap.end())
             pToken->OpCode = (*iLook).second;
         else
         {
@@ -491,7 +491,7 @@ uno::Sequence< sheet::FormulaOpCodeMapEntry > FormulaCompiler::OpCodeMap::create
                 SC_OPCODE_CLOSE,
                 SC_OPCODE_SEP,
             };
-            lclPushOpCodeMapEntries( aVec, mpTable, aOpCodes, SAL_N_ELEMENTS(aOpCodes) );
+            lclPushOpCodeMapEntries( aVec, mpTable.get(), aOpCodes, SAL_N_ELEMENTS(aOpCodes) );
         }
         if ((nGroups & FormulaMapGroup::ARRAY_SEPARATORS) != 0)
         {
@@ -501,16 +501,16 @@ uno::Sequence< sheet::FormulaOpCodeMapEntry > FormulaCompiler::OpCodeMap::create
                 SC_OPCODE_ARRAY_ROW_SEP,
                 SC_OPCODE_ARRAY_COL_SEP
             };
-            lclPushOpCodeMapEntries( aVec, mpTable, aOpCodes, SAL_N_ELEMENTS(aOpCodes) );
+            lclPushOpCodeMapEntries( aVec, mpTable.get(), aOpCodes, SAL_N_ELEMENTS(aOpCodes) );
         }
         if ((nGroups & FormulaMapGroup::UNARY_OPERATORS) != 0)
         {
             // Due to the nature of the percent operator following its operand
             // it isn't sorted into unary operators for compiler interna.
-            lclPushOpCodeMapEntry( aVec, mpTable, ocPercentSign );
+            lclPushOpCodeMapEntry( aVec, mpTable.get(), ocPercentSign );
             // "+" can be used as unary operator too, push only if binary group is not set
             if ((nGroups & FormulaMapGroup::BINARY_OPERATORS) == 0)
-                lclPushOpCodeMapEntry( aVec, mpTable, ocAdd );
+                lclPushOpCodeMapEntry( aVec, mpTable.get(), ocAdd );
             // regular unary operators
             for (sal_uInt16 nOp = SC_OPCODE_START_UN_OP; nOp < SC_OPCODE_STOP_UN_OP && nOp < mnSymbols; ++nOp)
             {
@@ -522,7 +522,7 @@ uno::Sequence< sheet::FormulaOpCodeMapEntry > FormulaCompiler::OpCodeMap::create
                     case SC_OPCODE_NEG :
                         break;   // nothing,
                     default:
-                        lclPushOpCodeMapEntry( aVec, mpTable, nOp );
+                        lclPushOpCodeMapEntry( aVec, mpTable.get(), nOp );
                 }
             }
         }
@@ -538,7 +538,7 @@ uno::Sequence< sheet::FormulaOpCodeMapEntry > FormulaCompiler::OpCodeMap::create
                     case SC_OPCODE_OR :
                         break;   // nothing,
                     default:
-                        lclPushOpCodeMapEntry( aVec, mpTable, nOp );
+                        lclPushOpCodeMapEntry( aVec, mpTable.get(), nOp );
                 }
             }
         }
@@ -546,9 +546,9 @@ uno::Sequence< sheet::FormulaOpCodeMapEntry > FormulaCompiler::OpCodeMap::create
         {
             // Function names are not consecutive, skip the gaps between
             // functions with no parameter, functions with 1 parameter
-            lclPushOpCodeMapEntries( aVec, mpTable, SC_OPCODE_START_NO_PAR,
+            lclPushOpCodeMapEntries( aVec, mpTable.get(), SC_OPCODE_START_NO_PAR,
                     ::std::min< sal_uInt16 >( SC_OPCODE_STOP_NO_PAR, mnSymbols ) );
-            lclPushOpCodeMapEntries( aVec, mpTable, SC_OPCODE_START_1_PAR,
+            lclPushOpCodeMapEntries( aVec, mpTable.get(), SC_OPCODE_START_1_PAR,
                     ::std::min< sal_uInt16 >( SC_OPCODE_STOP_1_PAR, mnSymbols ) );
             // Additional functions not within range of functions.
             static const sal_uInt16 aOpCodes[] = {
@@ -561,7 +561,7 @@ uno::Sequence< sheet::FormulaOpCodeMapEntry > FormulaCompiler::OpCodeMap::create
                 SC_OPCODE_NOT,
                 SC_OPCODE_NEG
             };
-            lclPushOpCodeMapEntries( aVec, mpTable, aOpCodes, SAL_N_ELEMENTS(aOpCodes) );
+            lclPushOpCodeMapEntries( aVec, mpTable.get(), aOpCodes, SAL_N_ELEMENTS(aOpCodes) );
             // functions with 2 or more parameters.
             for (sal_uInt16 nOp = SC_OPCODE_START_2_PAR; nOp < SC_OPCODE_STOP_2_PAR && nOp < mnSymbols; ++nOp)
             {
@@ -571,7 +571,7 @@ uno::Sequence< sheet::FormulaOpCodeMapEntry > FormulaCompiler::OpCodeMap::create
                     case SC_OPCODE_NO_NAME :
                         break;   // nothing,
                     default:
-                        lclPushOpCodeMapEntry( aVec, mpTable, nOp );
+                        lclPushOpCodeMapEntry( aVec, mpTable.get(), nOp );
                 }
             }
             // If AddIn functions are present in this mapping, use them, and only those.
@@ -679,14 +679,14 @@ void FormulaCompiler::OpCodeMap::putOpCode( const OUString & rStr, const OpCode
         {
             OUString aUpper( pCharClass ? pCharClass->uppercase( mpTable[eOp]) : rStr.toAsciiUpperCase());
             // Ensure we remove a mapping only for the requested OpCode.
-            OpCodeHashMap::const_iterator it( mpHashMap->find( aUpper));
-            if (it != mpHashMap->end() && (*it).second == eOp)
-                mpHashMap->erase( it);
+            OpCodeHashMap::const_iterator it( maHashMap.find( aUpper));
+            if (it != maHashMap.end() && (*it).second == eOp)
+                maHashMap.erase( it);
         }
         if (bPutOp)
             mpTable[eOp] = rStr;
         OUString aUpper( pCharClass ? pCharClass->uppercase( rStr) : rStr.toAsciiUpperCase());
-        mpHashMap->insert( OpCodeHashMap::value_type( aUpper, eOp));
+        maHashMap.insert( OpCodeHashMap::value_type( aUpper, eOp));
     }
     else
     {
@@ -966,8 +966,8 @@ OpCode FormulaCompiler::GetEnglishOpCode( const OUString& rName ) const
 {
     FormulaCompiler::OpCodeMapPtr xMap = GetOpCodeMap( sheet::FormulaLanguage::ENGLISH);
 
-    formula::OpCodeHashMap::const_iterator iLook( xMap->getHashMap()->find( rName ) );
-    bool bFound = (iLook != xMap->getHashMap()->end());
+    formula::OpCodeHashMap::const_iterator iLook( xMap->getHashMap().find( rName ) );
+    bool bFound = (iLook != xMap->getHashMap().end());
     return bFound ? (*iLook).second : ocNone;
 }
 
@@ -1061,30 +1061,23 @@ bool FormulaCompiler::IsMatrixFunction( OpCode eOpCode )
 }
 
 
-FormulaCompiler::OpCodeMap::~OpCodeMap()
-{
-    delete [] mpTable;
-    delete mpHashMap;
-}
-
 void FormulaCompiler::OpCodeMap::putCopyOpCode( const OUString& rSymbol, OpCode eOp )
 {
     SAL_WARN_IF( !mpTable[eOp].isEmpty() && rSymbol.isEmpty(), "formula.core",
             "OpCodeMap::putCopyOpCode: NOT replacing OpCode " << static_cast<sal_uInt16>(eOp)
             << " '" << mpTable[eOp] << "' with empty name!");
     if (!mpTable[eOp].isEmpty() && rSymbol.isEmpty())
-        mpHashMap->insert( OpCodeHashMap::value_type( mpTable[eOp], eOp));
+        maHashMap.insert( OpCodeHashMap::value_type( mpTable[eOp], eOp));
     else
     {
         mpTable[eOp] = rSymbol;
-        mpHashMap->insert( OpCodeHashMap::value_type( rSymbol, eOp));
+        maHashMap.insert( OpCodeHashMap::value_type( rSymbol, eOp));
     }
 }
 
 void FormulaCompiler::OpCodeMap::copyFrom( const OpCodeMap& r )
 {
-    delete mpHashMap;
-    mpHashMap = new OpCodeHashMap( mnSymbols);
+    maHashMap = OpCodeHashMap( mnSymbols);
 
     sal_uInt16 n = r.getSymbolCount();
     SAL_WARN_IF( n != mnSymbols, "formula.core",
@@ -1139,8 +1132,8 @@ void FormulaCompiler::OpCodeMap::copyFrom( const OpCodeMap& r )
 FormulaError FormulaCompiler::GetErrorConstant( const OUString& rName ) const
 {
     FormulaError nError = FormulaError::NONE;
-    OpCodeHashMap::const_iterator iLook( mxSymbols->getHashMap()->find( rName));
-    if (iLook != mxSymbols->getHashMap()->end())
+    OpCodeHashMap::const_iterator iLook( mxSymbols->getHashMap().find( rName));
+    if (iLook != mxSymbols->getHashMap().end())
     {
         switch ((*iLook).second)
         {
@@ -2267,8 +2260,8 @@ const FormulaToken* FormulaCompiler::CreateStringFromToken( OUStringBuffer& rBuf
                 bool bMapped = mxSymbols->isPODF();     // ODF 1.1 directly uses programmatical name
                 if (!bMapped && mxSymbols->hasExternals())
                 {
-                    ExternalHashMap::const_iterator iLook = mxSymbols->getReverseExternalHashMap()->find( aAddIn);
-                    if (iLook != mxSymbols->getReverseExternalHashMap()->end())
+                    ExternalHashMap::const_iterator iLook = mxSymbols->getReverseExternalHashMap().find( aAddIn);
+                    if (iLook != mxSymbols->getReverseExternalHashMap().end())
                     {
                         aAddIn = (*iLook).second;
                         bMapped = true;
diff --git a/include/formula/FormulaCompiler.hxx b/include/formula/FormulaCompiler.hxx
index a9b65671e4cd..970909cd9f00 100644
--- a/include/formula/FormulaCompiler.hxx
+++ b/include/formula/FormulaCompiler.hxx
@@ -81,8 +81,8 @@ public:
     /** Mappings from strings to OpCodes and vice versa. */
     class FORMULA_DLLPUBLIC OpCodeMap final
     {
-        OpCodeHashMap         * mpHashMap;                 /// Hash map of symbols, OUString -> OpCode
-        OUString              * mpTable;                   /// Array of symbols, OpCode -> OUString, offset==OpCode
+        OpCodeHashMap           maHashMap;                  /// Hash map of symbols, OUString -> OpCode
+        std::unique_ptr<OUString[]> mpTable;                /// Array of symbols, OpCode -> OUString, offset==OpCode
         ExternalHashMap         maExternalHashMap;         /// Hash map of ocExternal, Filter String -> AddIn String
         ExternalHashMap         maReverseExternalHashMap;  /// Hash map of ocExternal, AddIn String -> Filter String
         FormulaGrammar::Grammar meGrammar;                  /// Grammar, language and reference convention
@@ -96,15 +96,14 @@ public:
     public:
 
         OpCodeMap(sal_uInt16 nSymbols, bool bCore, FormulaGrammar::Grammar eGrammar ) :
-            mpHashMap( new OpCodeHashMap( nSymbols)),
+            maHashMap(nSymbols),
             mpTable( new OUString[ nSymbols ]),
             meGrammar( eGrammar),
             mnSymbols( nSymbols),
-            mbCore( bCore)
+            mbCore( bCore),
+            mbEnglish ( FormulaGrammar::isEnglish(eGrammar) )
         {
-            mbEnglish = FormulaGrammar::isEnglish( meGrammar);
         }
-        ~OpCodeMap();
 
         /** Copy mappings from r into this map, effectively replacing this map.
 
@@ -115,13 +114,13 @@ public:
         void copyFrom( const OpCodeMap& r );
 
         /// Get the symbol String -> OpCode hash map for finds.
-        const OpCodeHashMap* getHashMap() const { return mpHashMap; }
+        const OpCodeHashMap& getHashMap() const { return maHashMap; }
 
         /// Get the symbol String -> AddIn String hash map for finds.
-        const ExternalHashMap* getExternalHashMap() const { return &maExternalHashMap; }
+        const ExternalHashMap& getExternalHashMap() const { return maExternalHashMap; }
 
         /// Get the AddIn String -> symbol String hash map for finds.
-        const ExternalHashMap* getReverseExternalHashMap() const { return &maReverseExternalHashMap; }
+        const ExternalHashMap& getReverseExternalHashMap() const { return maReverseExternalHashMap; }
 
         /// Get the symbol string matching an OpCode.
         const OUString& getSymbol( const OpCode eOp ) const
diff --git a/sc/source/core/tool/calcconfig.cxx b/sc/source/core/tool/calcconfig.cxx
index 07da3b4f2924..e80ec3fbceb0 100644
--- a/sc/source/core/tool/calcconfig.cxx
+++ b/sc/source/core/tool/calcconfig.cxx
@@ -178,7 +178,7 @@ ScCalcConfig::OpCodeSet ScStringToOpCodeSet(const OUString& rOpCodes)
     formula::FormulaCompiler aCompiler;
     formula::FormulaCompiler::OpCodeMapPtr pOpCodeMap(aCompiler.GetOpCodeMap(css::sheet::FormulaLanguage::ENGLISH));
 
-    const formula::OpCodeHashMap *pHashMap(pOpCodeMap->getHashMap());
+    const formula::OpCodeHashMap& rHashMap(pOpCodeMap->getHashMap());
 
     sal_Int32 fromIndex(0);
     sal_Int32 semicolon;
@@ -194,8 +194,8 @@ ScCalcConfig::OpCodeSet ScStringToOpCodeSet(const OUString& rOpCodes)
                 result->insert(static_cast<OpCode>(n));
             else
             {
-                auto opcode(pHashMap->find(element));
-                if (opcode != pHashMap->end())
+                auto opcode(rHashMap.find(element));
+                if (opcode != rHashMap.end())
                     result->insert(opcode->second);
                 else
                     SAL_WARN("sc.opencl", "Unrecognized OpCode " << element << " in OpCode set string");
diff --git a/sc/source/core/tool/compiler.cxx b/sc/source/core/tool/compiler.cxx
index 50775af00b57..d373516899fd 100644
--- a/sc/source/core/tool/compiler.cxx
+++ b/sc/source/core/tool/compiler.cxx
@@ -2706,8 +2706,8 @@ Label_MaskStateMachine:
 
 bool ScCompiler::IsOpCode( const OUString& rName, bool bInArray )
 {
-    OpCodeHashMap::const_iterator iLook( mxSymbols->getHashMap()->find( rName));
-    bool bFound = (iLook != mxSymbols->getHashMap()->end());
+    OpCodeHashMap::const_iterator iLook( mxSymbols->getHashMap().find( rName));
+    bool bFound = (iLook != mxSymbols->getHashMap().end());
     if (bFound)
     {
         OpCode eOp = iLook->second;
@@ -2846,8 +2846,8 @@ bool ScCompiler::IsOpCode( const OUString& rName, bool bInArray )
         {
             // If symbols are set by filters get mapping to exact name.
             ExternalHashMap::const_iterator iExt(
-                    mxSymbols->getExternalHashMap()->find( rName));
-            if (iExt != mxSymbols->getExternalHashMap()->end())
+                    mxSymbols->getExternalHashMap().find( rName));
+            if (iExt != mxSymbols->getExternalHashMap().end())
             {
                 if (ScGlobal::GetAddInCollection()->GetFuncData( (*iExt).second))
                     aIntName = (*iExt).second;
@@ -3706,8 +3706,8 @@ bool ScCompiler::IsColRowName( const OUString& rName )
 
 bool ScCompiler::IsBoolean( const OUString& rName )
 {
-    OpCodeHashMap::const_iterator iLook( mxSymbols->getHashMap()->find( rName ) );
-    if( iLook != mxSymbols->getHashMap()->end() &&
+    OpCodeHashMap::const_iterator iLook( mxSymbols->getHashMap().find( rName ) );
+    if( iLook != mxSymbols->getHashMap().end() &&
         ((*iLook).second == ocTrue ||
          (*iLook).second == ocFalse) )
     {
@@ -3733,8 +3733,8 @@ bool ScCompiler::IsErrorConstant( const OUString& rName ) const
 bool ScCompiler::IsTableRefItem( const OUString& rName ) const
 {
     bool bItem = false;
-    OpCodeHashMap::const_iterator iLook( mxSymbols->getHashMap()->find( rName));
-    if (iLook != mxSymbols->getHashMap()->end())
+    OpCodeHashMap::const_iterator iLook( mxSymbols->getHashMap().find( rName));
+    if (iLook != mxSymbols->getHashMap().end())
     {
         // Only called when there actually is a current TableRef, hence
         // accessing maTableRefs.back() is safe.
diff --git a/sc/source/core/tool/token.cxx b/sc/source/core/tool/token.cxx
index 3cbbc966a3e9..8349a781dd5c 100644
--- a/sc/source/core/tool/token.cxx
+++ b/sc/source/core/tool/token.cxx
@@ -4990,7 +4990,7 @@ void appendTokenByType( sc::TokenStringContext& rCxt, OUStringBuffer& rBuf, cons
             bool bMapped = rCxt.mxOpCodeMap->isPODF();     // ODF 1.1 directly uses programmatical name
             if (!bMapped && rCxt.mxOpCodeMap->hasExternals())
             {
-                const ExternalHashMap& rExtMap = *rCxt.mxOpCodeMap->getReverseExternalHashMap();
+                const ExternalHashMap& rExtMap = rCxt.mxOpCodeMap->getReverseExternalHashMap();
                 ExternalHashMap::const_iterator it = rExtMap.find(aAddIn);
                 if (it != rExtMap.end())
                 {