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>
diff --git a/sc/qa/unit/parallelism.cxx b/sc/qa/unit/parallelism.cxx
index c5c1961..2e74840 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 ffd961a..8d0da74 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
