tdf#164997 - sc refactor and optimize LET function calculation

With the original solution the embeded LET function into another
functions caused unexpected error values.

Such as: IFERROR(LET(...)...) gave error for non-error LET result

Change-Id: I5f09a8771ce38492269cfeb567b9c3357cfd510c
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/181662
Reviewed-by: Balazs Varga <balazs.varga.extern@allotropia.de>
Tested-by: Jenkins

formula/source/core/api/token.cxx

@@ -789,6 +789,24 @@ FormulaToken* FormulaTokenArray::ReplaceToken( sal_uInt16 nOffset, FormulaToken*
     }
 }
 
+FormulaToken* FormulaTokenArray::ReplaceRPNToken( sal_uInt16 nOffset, FormulaToken* t )
+{
+    if (nOffset < nRPN)
+    {
+        CheckToken(*t);
+        t->IncRef();
+        FormulaToken* p = pRPN[nOffset];
+        pRPN[nOffset] = t;
+        p->DecRef();    // may be dead now
+        return t;
+    }
+    else
+    {
+        t->DeleteIfZeroRef();
+        return nullptr;
+    }
+}
+
 sal_uInt16 FormulaTokenArray::RemoveToken( sal_uInt16 nOffset, sal_uInt16 nCount )
 {
     if (nOffset < nLen)
@@ -888,6 +906,11 @@ FormulaToken* FormulaTokenArray::AddString( const svl::SharedString& rStr )
     return Add( new FormulaStringToken( rStr ) );
 }
 
+FormulaToken* FormulaTokenArray::AddStringName( const svl::SharedString& rStr )
+{
+    return Add( new FormulaStringNameToken( rStr ) );
+}
+
 FormulaToken* FormulaTokenArray::AddDouble( double fVal )
 {
     return Add( new FormulaDoubleToken( fVal ) );
@@ -1619,13 +1642,13 @@ void FormulaTokenArray::ReinternStrings( svl::SharedStringPool& rPool )
 
 /*----------------------------------------------------------------------*/
 
-FormulaTokenIterator::Item::Item(const FormulaTokenArray* pArray, short pc, short stop) :
-    pArr(pArray), nPC(pc), nStop(stop)
+FormulaTokenIterator::Item::Item(const FormulaTokenArray* pArray, short pc, short stop, bool lambda) :
+    pArr(pArray), nPC(pc), nStop(stop), bLambda(lambda)
 {
 }
 
 FormulaTokenIterator::FormulaTokenIterator( const FormulaTokenArray& rArr )
-    : maStack{ FormulaTokenIterator::Item(&rArr, -1, SHRT_MAX) }
+    : maStack{ FormulaTokenIterator::Item(&rArr, -1, SHRT_MAX, false) }
 {
 }
 
@@ -1635,7 +1658,7 @@ FormulaTokenIterator::~FormulaTokenIterator()
 
 void FormulaTokenIterator::Push( const FormulaTokenArray* pArr )
 {
-    FormulaTokenIterator::Item item(pArr, -1, SHRT_MAX);
+    FormulaTokenIterator::Item item(pArr, -1, SHRT_MAX, false);
 
     maStack.push_back(item);
 }
@@ -1645,12 +1668,23 @@ void FormulaTokenIterator::Pop()
     maStack.pop_back();
 }
 
+void FormulaTokenIterator::FrontPop()
+{
+    maStack.erase(maStack.begin());
+}
+
+void FormulaTokenIterator::Lambda(bool bOpt)
+{
+    maStack.back().bLambda = bOpt;
+}
+
 void FormulaTokenIterator::Reset()
 {
     while( maStack.size() > 1 )
         maStack.pop_back();
 
-    maStack.back().nPC = -1;
+    if (!maStack.back().bLambda)
+        maStack.back().nPC = -1;
 }
 
 FormulaToken* FormulaTokenArrayPlainIterator::GetNextName()
@@ -1669,12 +1703,12 @@ FormulaToken* FormulaTokenArrayPlainIterator::GetNextName()
 
 FormulaToken* FormulaTokenArrayPlainIterator::GetNextStringName()
 {
-    if (mpFTA->GetArray())
+    if (mpFTA->GetCode())
     {
-        while (mnIndex < mpFTA->GetLen())
+        while (mnIndex < mpFTA->GetCodeLen())
         {
-            FormulaToken* t = mpFTA->GetArray()[mnIndex++];
-            if (t->GetType() == svString && t->GetOpCode() == ocStringName)
+            FormulaToken* t = mpFTA->GetCode()[ mnIndex++ ];
+            if (t->GetType() == svStringName)
                 return t;
         }
     }
@@ -2000,6 +2034,35 @@ bool FormulaStringOpToken::operator==( const FormulaToken& r ) const
     return FormulaByteToken::operator==( r ) && maString == r.GetString();
 }
 
+FormulaStringNameToken::FormulaStringNameToken( svl::SharedString r ) :
+    FormulaToken( svStringName ), maString(std::move( r ))
+{
+}
+
+FormulaStringNameToken::FormulaStringNameToken( const FormulaStringNameToken& r ) :
+    FormulaToken( r ), maString( r.maString ) {
+}
+
+FormulaToken* FormulaStringNameToken::Clone() const
+{
+    return new FormulaStringNameToken(*this);
+}
+
+const svl::SharedString& FormulaStringNameToken::GetString() const
+{
+    return maString;
+}
+
+void FormulaStringNameToken::SetString( const svl::SharedString& rStr )
+{
+    maString = rStr;
+}
+
+bool FormulaStringNameToken::operator==( const FormulaToken& r ) const
+{
+    return FormulaToken::operator==( r ) && maString == r.GetString();
+}
+
 sal_uInt16  FormulaIndexToken::GetIndex() const             { return nIndex; }
 void        FormulaIndexToken::SetIndex( sal_uInt16 n )     { nIndex = n; }
 sal_Int16   FormulaIndexToken::GetSheet() const             { return mnSheet; }

include/formula/token.hxx

@@ -50,6 +50,7 @@ enum StackVar : sal_uInt8
     svByte,
     svDouble,
     svString,
+    svStringName,
     svSingleRef,
     svDoubleRef,
     svMatrix,
@@ -93,6 +94,7 @@ inline std::string StackVarEnumToString(StackVar const e)
         case svByte:              return "Byte";
         case svDouble:            return "Double";
         case svString:            return "String";
+        case svStringName:        return "StringName";
         case svSingleRef:         return "SingleRef";
         case svDoubleRef:         return "DoubleRef";
         case svMatrix:            return "Matrix";
@@ -396,6 +398,21 @@ public:
     virtual bool operator==( const FormulaToken& rToken ) const override;
 };
 
+// FormulaStringNameToken
+class FORMULA_DLLPUBLIC FormulaStringNameToken final : public FormulaToken
+{
+    svl::SharedString maString;
+public:
+    FormulaStringNameToken(svl::SharedString r);
+    FormulaStringNameToken(const FormulaStringNameToken& r);
+
+    virtual FormulaToken* Clone() const override;
+    virtual const svl::SharedString& GetString() const override;
+    virtual void SetString(const svl::SharedString& rStr) override;
+    virtual bool operator==(const FormulaToken& rToken) const override;
+};
+
+
 class FORMULA_DLLPUBLIC FormulaIndexToken final : public FormulaToken
 {
 private:

include/formula/tokenarray.hxx

@@ -271,6 +271,7 @@ public:
                 token as well.
      */
     FormulaToken*           ReplaceToken( sal_uInt16 nOffset, FormulaToken*, ReplaceMode eMode );
+    FormulaToken*           ReplaceRPNToken( sal_uInt16 nOffset, FormulaToken* );
 
     /** Remove a sequence of tokens from pCode array, and pRPN array if the
         tokens are referenced there.
@@ -480,6 +481,7 @@ public:
     FormulaToken* AddToken( const FormulaToken& );
 
     FormulaToken* AddString( const svl::SharedString& rStr );
+    FormulaToken* AddStringName( const svl::SharedString& rStr );
     FormulaToken* AddDouble( double fVal );
     void          AddExternal( const sal_Unicode* pStr );
     /** Xcl import may play dirty tricks with OpCode!=ocExternal.
@@ -540,8 +542,9 @@ class FORMULA_DLLPUBLIC FormulaTokenIterator
         const FormulaTokenArray* pArr;
         short nPC;
         short nStop;
+        bool bLambda;
 
-        Item(const FormulaTokenArray* arr, short pc, short stop);
+        Item(const FormulaTokenArray* arr, short pc, short stop, bool lambda);
     };
 
     std::vector<Item> maStack;
@@ -572,6 +575,8 @@ public:
     void Jump( short nStart, short nNext, short nStop = SHRT_MAX );
     void Push( const FormulaTokenArray* );
     void Pop();
+    void FrontPop();
+    void Lambda( bool bOpt );
 
     /** Reconstruct the iterator afresh from a token array
     */

sc/source/core/inc/interpre.hxx

@@ -519,9 +519,10 @@ private:
     sc::RangeMatrix GetRangeMatrix();
 
     // Get tokens at specific parameters for LET (lambda) function
-    void getTokensAtParameter( std::unique_ptr<ScTokenArray>& pTokens, short nPos );
     static void replaceNamesToResult( const std::unordered_map<OUString, formula::FormulaToken*>& rResultIndexes,
-        std::unique_ptr<ScTokenArray>& pTokens );
+        std::unique_ptr<ScTokenArray>& pTokens, short nStartPos, short nEndPos );
+    std::unique_ptr<ScTokenArray> checkPushTokens( const std::unique_ptr<ScTokenArray>& pTokens,
+        short nStartPos, short nEndPos );
 
     void ScTableOp();                                       // repeated operations
 

sc/source/core/tool/compiler.cxx

@@ -5155,7 +5155,7 @@ ScRangeData* ScCompiler::GetRangeData( const FormulaToken& rToken ) const
 bool ScCompiler::HandleStringName()
 {
     ScTokenArray* pNew = new ScTokenArray(rDoc);
-    pNew->AddString(mpToken->GetString());
+    pNew->AddStringName(mpToken->GetString());
     PushTokenArray(pNew, true);
     return GetToken();
 }

sc/source/core/tool/interpr1.cxx

@@ -9545,44 +9545,34 @@ void ScInterpreter::ScUnique()
     PushMatrix(pResMat);
 }
 
-void ScInterpreter::getTokensAtParameter( std::unique_ptr<ScTokenArray>& pTokens, short nPos )
+void ScInterpreter::replaceNamesToResult( const std::unordered_map<OUString, formula::FormulaToken*>& rResultIndexes,
+    std::unique_ptr<ScTokenArray>& pTokens, short nStartPos, short nEndPos )
 {
-    sal_uInt16 nOpen = 0;
-    sal_uInt16 nSepCount = 0;
-    formula::FormulaTokenArrayPlainIterator aIter(*pArr);
-    formula::FormulaToken* t = aIter.First();
-    for (t = aIter.NextNoSpaces(); t; t = aIter.NextNoSpaces())
+    formula::FormulaTokenArrayPlainIterator aIterResult(*pTokens);
+    aIterResult.Jump(nStartPos + 1);
+    for (FormulaToken* t = aIterResult.GetNextStringName(); t; t = aIterResult.GetNextStringName())
     {
-        OpCode aOpCode = t->GetOpCode();
-        formula::StackVar aIntType = t->GetType();
-        if ((aOpCode == ocOpen || aOpCode == ocArrayOpen || aOpCode == ocTableRefOpen) && aIntType == formula::StackVar::svSep)
-            nOpen++;
-        else if ((aOpCode == ocClose || aOpCode == ocArrayClose || aOpCode == ocTableRefClose) && aIntType == formula::StackVar::svSep)
-            nOpen--;
-        else if (aOpCode == ocSep && aIntType == formula::StackVar::svSep && nOpen == 1)
-        {
-            nSepCount++;
-            continue;
-        }
-
-        if (nSepCount == nPos && nOpen > 0)
-        {
-            pTokens->AddToken(*t->Clone());
-        }
+        if (aIterResult.GetIndex() > nEndPos)
+            break;
+        auto iRes = rResultIndexes.find(t->GetString().getString());
+        if (iRes != rResultIndexes.end())
+            pTokens->ReplaceRPNToken(aIterResult.GetIndex() - 1, iRes->second->Clone());
     }
 }
 
-void ScInterpreter::replaceNamesToResult(const std::unordered_map<OUString, formula::FormulaToken*>& rResultIndexes,
-    std::unique_ptr<ScTokenArray>& pTokens )
+std::unique_ptr<ScTokenArray> ScInterpreter::checkPushTokens(const std::unique_ptr<ScTokenArray>& pTokens, short nStartPos, short nEndPos)
 {
     formula::FormulaTokenArrayPlainIterator aIterResult(*pTokens);
-    for (FormulaToken* t = aIterResult.GetNextStringName(); t; t = aIterResult.GetNextStringName())
+    aIterResult.Jump(nStartPos + 1);
+    unique_ptr<ScTokenArray> pTempTokens(new ScTokenArray(mrDoc));
+    for (FormulaToken* t = aIterResult.NextRPN(); t; t = aIterResult.NextRPN())
     {
-        auto iRes = rResultIndexes.find(t->GetString().getString());
-        if (iRes != rResultIndexes.end())
-            pTokens->ReplaceToken(aIterResult.GetIndex() - 1, iRes->second->Clone(),
-                FormulaTokenArray::ReplaceMode::CODE_ONLY);
+        if (aIterResult.GetIndex() > nEndPos)
+            break;
+
+        pTempTokens->AddToken(*t->Clone());
     }
+    return pTempTokens;
 }
 
 void ScInterpreter::ScLet()
@@ -9601,7 +9591,8 @@ void ScInterpreter::ScLet()
     OUString aStrName;
     std::unordered_map<OUString, formula::FormulaToken*> nResultIndexes;
     formula::FormulaTokenArrayPlainIterator aIter(*pArr);
-    unique_ptr<ScTokenArray> pValueTokens(new ScTokenArray(mrDoc));
+    // clone tokens for replacing string name tokens
+    unique_ptr<ScTokenArray> pValueTokens = pArr->Clone();
 
     // name and function pairs parameter
     while (nJumpCount > 1)
@@ -9624,17 +9615,21 @@ void ScInterpreter::ScLet()
         }
         nJumpCount--;
 
-        // get value tokens
-        getTokensAtParameter(pValueTokens, nOrgJumpCount - nJumpCount);
-        nJumpCount--;
-
         // replace names with result tokens
-        replaceNamesToResult(nResultIndexes, pValueTokens);
+        replaceNamesToResult(nResultIndexes, pValueTokens, pJump[nOrgJumpCount - nJumpCount], pJump[nOrgJumpCount - nJumpCount + 1]);
+
+        unique_ptr<ScTokenArray> pTempTokens = checkPushTokens(pValueTokens, pJump[nOrgJumpCount - nJumpCount], pJump[nOrgJumpCount - nJumpCount + 1]);
 
         // calculate the inner results unless we already have a push result token
-        if (pValueTokens->GetLen() == 1 && pValueTokens->GetArray()[0]->GetOpCode() == ocPush)
+        if (pTempTokens->GetLen() == 0)
         {
-            if (!nResultIndexes.insert(std::make_pair(aStrName, pValueTokens->GetArray()[0]->Clone())).second)
+            PushIllegalParameter();
+            aCode.Jump(pJump[nOrgJumpCount], pJump[nOrgJumpCount]);
+            return;
+        }
+        else if (pTempTokens->GetLen() == 1 && pTempTokens->GetArray()[0]->GetOpCode() == ocPush)
+        {
+            if (!nResultIndexes.insert(std::make_pair(aStrName, pTempTokens->GetArray()[0]->Clone())).second)
             {
                 PushIllegalParameter();
                 aCode.Jump(pJump[nOrgJumpCount], pJump[nOrgJumpCount]);
@@ -9643,11 +9638,15 @@ void ScInterpreter::ScLet()
         }
         else
         {
-            ScCompiler aComp(mrDoc, aPos, *pValueTokens, mrDoc.GetGrammar(), false, false, &mrContext);
-            aComp.CompileTokenArray();
             ScInterpreter aInt(mrDoc.GetFormulaCell(aPos), mrDoc, mrContext, aPos, *pValueTokens);
+            aInt.aCode.Jump(pJump[nOrgJumpCount - nJumpCount], pJump[nOrgJumpCount - nJumpCount + 1], pJump[nOrgJumpCount - nJumpCount + 1]);
+            while (aInt.aCode.HasStacked())
+                aInt.aCode.FrontPop();
+            aInt.aCode.Lambda(true);
+
             sfx2::LinkManager aNewLinkMgr(mrDoc.GetDocumentShell());
             aInt.SetLinkManager(&aNewLinkMgr);
+
             formula::StackVar aIntType = aInt.Interpret();
 
             if (aIntType == formula::svMatrixCell)
@@ -9671,20 +9670,20 @@ void ScInterpreter::ScLet()
                 }
             }
         }
-        pValueTokens->Clear();
+        nJumpCount--;
     }
 
     // last parameter: calculation
-    getTokensAtParameter(pValueTokens, nOrgJumpCount - nJumpCount);
-    nJumpCount--;
-
     // replace names with result tokens
-    replaceNamesToResult(nResultIndexes, pValueTokens);
+    replaceNamesToResult(nResultIndexes, pValueTokens, pJump[nOrgJumpCount - nJumpCount], pJump[nOrgJumpCount - nJumpCount + 1]);
 
     // calculate the final result
-    ScCompiler aComp(mrDoc, aPos, *pValueTokens, mrDoc.GetGrammar(), false, false, &mrContext);
-    aComp.CompileTokenArray();
     ScInterpreter aInt(mrDoc.GetFormulaCell(aPos), mrDoc, mrContext, aPos, *pValueTokens);
+    aInt.aCode.Jump(pJump[nOrgJumpCount - nJumpCount], pJump[nOrgJumpCount - nJumpCount + 1], pJump[nOrgJumpCount - nJumpCount + 1]);
+    while (aInt.aCode.HasStacked())
+        aInt.aCode.FrontPop();
+    aInt.aCode.Lambda(true);
+
     sfx2::LinkManager aNewLinkMgr(mrDoc.GetDocumentShell());
     aInt.SetLinkManager(&aNewLinkMgr);
     formula::StackVar aIntType = aInt.Interpret();
@@ -9707,6 +9706,7 @@ void ScInterpreter::ScLet()
         }
     }
 
+    nJumpCount--;
     pValueTokens.reset();
     aCode.Jump(pJump[nOrgJumpCount], pJump[nOrgJumpCount]);
 }

sc/source/core/tool/interpr4.cxx

@@ -827,6 +827,7 @@ const svl::SharedString & ScInterpreter::PopString()
                 nGlobalError = p->GetError();
                 break;
             case svString:
+            case svStringName:
                 return p->GetString();
             case svEmptyCell:
             case svMissing:
@@ -2344,6 +2345,7 @@ svl::SharedString ScInterpreter::GetString()
             return GetStringFromDouble( PopDouble() );
         }
         case svString:
+        case svStringName:
             return PopString();
         case svSingleRef:
         {