From 7bafe2441480e2b88d999b30b7f117f05e72c3b3 Mon Sep 17 00:00:00 2001 From: Dennis Francis Date: Thu, 4 Oct 2018 12:28:14 +0530 Subject: 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 --- sc/qa/unit/parallelism.cxx | 25 +++++++++++++++++++++ sc/source/core/tool/formulagroup.cxx | 43 +++++++++++++++++++++++++----------- 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(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(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 #include +#include +#include #include #include #include @@ -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(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 -- cgit v1.2.3