tdf#119904 : Generalize the fix for tdf#115093

The point is, if one of the FormulaTokens in the formula
is returned as the "result" FormulaToken, then we should not reuse
that same FormulaToken for each formula-cell in the group. If we do,
then we end up with whole group/batch having the same result.

Also adds a unit test specific to this bug.

This issue is non existent in master because "SoftwareInterpreter"
and related code were removed from master long after branch-off
to 6.1.

Change-Id: I10265211b5f14c82ed385401aa3fb838c492872d
Reviewed-on: https://gerrit.libreoffice.org/61362
Tested-by: Jenkins
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>

2 files changed, 55 insertions, 13 deletions

diff --git a/sc/qa/unit/parallelism.cxx b/sc/qa/unit/parallelism.cxx
index c5c196154123..2e74840a215d 100644
--- a/sc/qa/unit/parallelism.cxx
+++ b/sc/qa/unit/parallelism.cxx
@@ -52,6 +52,7 @@ public:
     void testVLOOKUPSUM();
     void testSingleRef();
     void testSUMIFImplicitRange();
+    void testTdf119904();
 
     CPPUNIT_TEST_SUITE(ScParallelismTest);
     CPPUNIT_TEST(testSUMIFS);
@@ -60,6 +61,7 @@ public:
     CPPUNIT_TEST(testVLOOKUPSUM);
     CPPUNIT_TEST(testSingleRef);
     CPPUNIT_TEST(testSUMIFImplicitRange);
+    CPPUNIT_TEST(testTdf119904);
     CPPUNIT_TEST_SUITE_END();
 
 private:
@@ -396,6 +398,29 @@ void ScParallelismTest::testSUMIFImplicitRange()
     m_pDoc->DeleteTab(0);
 }
 
+void ScParallelismTest::testTdf119904()
+{
+    m_pDoc->InsertTab(0, "1");
+
+    const size_t nNumRows = 200;
+    for (size_t i = 0; i < nNumRows; ++i)
+    {
+        m_pDoc->SetValue(0, i, 0, static_cast<double>(i));
+        m_pDoc->SetFormula(ScAddress(1, i, 0), (i % 2) ? OUString("=TRUE()") : OUString("=FALSE()"), formula::FormulaGrammar::GRAM_NATIVE_UI);
+        m_pDoc->SetFormula(ScAddress(2, i, 0), "=IF(B" + OUString::number(i+1) +
+                           "; A" + OUString::number(i+1) + "; \"\")", formula::FormulaGrammar::GRAM_NATIVE_UI);
+    }
+
+    m_xDocShell->DoHardRecalc();
+
+    for (size_t i = 0; i < nNumRows; ++i)
+    {
+        OString aMsg = "At row " + OString::number(i);
+        CPPUNIT_ASSERT_EQUAL_MESSAGE(aMsg.getStr(), (i % 2) ? i : 0, static_cast<size_t>(m_pDoc->GetValue(2, i, 0)));
+    }
+    m_pDoc->DeleteTab(0);
+}
+
 CPPUNIT_TEST_SUITE_REGISTRATION(ScParallelismTest);
 
 CPPUNIT_PLUGIN_IMPLEMENT();
diff --git a/sc/source/core/tool/formulagroup.cxx b/sc/source/core/tool/formulagroup.cxx
index ffd961ae7b99..8d0da740508d 100644
--- a/sc/source/core/tool/formulagroup.cxx
+++ b/sc/source/core/tool/formulagroup.cxx
@@ -29,7 +29,9 @@
 #endif
 #include <o3tl/make_unique.hxx>
 #include <rtl/bootstrap.hxx>
+#include <sal/alloca.h>
 
+#include <algorithm>
 #include <cstdio>
 #include <unordered_map>
 #include <vector>
@@ -177,7 +179,12 @@ public:
         ScTokenArray aCode2;
 
         ScInterpreterContext aContext(mrDoc, mpFormatter);
-        sal_uInt16 nNumNonOpenClose = mrCode.GetLen();
+        const formula::FormulaToken* pLastDoubleResultToken = nullptr;
+        const formula::FormulaToken* pLastStringResultToken = nullptr;
+        size_t nNumToks = mrCode.GetLen();
+        bool* pReuseFlags = static_cast<bool*>(alloca(sizeof(bool)*nNumToks));
+        if (pReuseFlags)
+            std::fill_n(pReuseFlags, nNumToks, true);
 
         for (SCROW i = mnIdx; i <= mnLastIdx; ++i, maBatchTopPos.IncRow())
         {
@@ -213,7 +220,12 @@ public:
                                 aCode2.AddString(rPool.intern(OUString(pStr)));
                             else
                             {
-                                if ( ( pTargetTok->GetType() == formula::svString ) && ( nNumNonOpenClose > 1 ) )
+                                bool bReuseToken = pReuseFlags && pReuseFlags[nTokIdx];
+                                if (bReuseToken && pTargetTok == pLastStringResultToken)
+                                    // Once pReuseFlags[nTokIdx] is set to false, it will continue to be so.
+                                    bReuseToken = pReuseFlags[nTokIdx] = false;
+
+                                if ( ( pTargetTok->GetType() == formula::svString ) && bReuseToken )
                                     pTargetTok->SetString(rPool.intern(OUString(pStr)));
                                 else
                                 {
@@ -227,7 +239,7 @@ public:
                             // Value of NaN represents an empty cell.
                             if ( !pTargetTok )
                                 aCode2.AddToken(ScEmptyCellToken(false, false));
-                            else if ( ( pTargetTok->GetType() != formula::svEmptyCell ) || ( nNumNonOpenClose == 1 ) )
+                            else if ( pTargetTok->GetType() != formula::svEmptyCell )
                             {
                                 ScEmptyCellToken* pEmptyTok = new ScEmptyCellToken(false, false);
                                 aCode2.ReplaceToken(nTokIdx, pEmptyTok, formula::FormulaTokenArray::CODE_ONLY);
@@ -240,7 +252,12 @@ public:
                                 aCode2.AddDouble(fVal);
                             else
                             {
-                                if ( ( pTargetTok->GetType() == formula::svDouble ) && ( nNumNonOpenClose > 1 ) )
+                                bool bReuseToken = pReuseFlags && pReuseFlags[nTokIdx];
+                                if (bReuseToken && pTargetTok == pLastDoubleResultToken)
+                                    // Once pReuseFlags[nTokIdx] is set to false, it will continue to be so.
+                                    bReuseToken = pReuseFlags[nTokIdx] = false;
+
+                                if ( ( pTargetTok->GetType() == formula::svDouble ) && bReuseToken )
                                     pTargetTok->GetDoubleAsReference() = fVal;
                                 else
                                 {
@@ -292,16 +309,8 @@ public:
                     break;
                     default:
                         if ( !pTargetTok )
-                        {
-                            if ( p->GetType() == formula::svSep )
-                            {
-                                OpCode eOp = p->GetOpCode();
-                                if ( eOp == ocOpen || eOp == ocClose )
-                                    --nNumNonOpenClose;
-                            }
-
                             aCode2.AddToken(*p);
-                        }
+
                 } // end of switch statement
             } // end of formula token for loop
 
@@ -314,6 +323,14 @@ public:
             ScInterpreter aInterpreter(pDest, &mrDoc, aContext, maBatchTopPos, aCode2);
             aInterpreter.Interpret();
             mrResults[i] = aInterpreter.GetResultToken();
+            const auto* pResultToken = mrResults[i].get();
+            if (pResultToken)
+            {
+                if (pResultToken->GetType() == formula::svDouble)
+                    pLastDoubleResultToken = pResultToken;
+                else if (pResultToken->GetType() == formula::svString)
+                    pLastStringResultToken = pResultToken;
+            }
         } // Row iteration for loop end
     } // operator () end