[Libreoffice-commits] core.git: Branch 'libreoffice-6-1' - sc/qa sc/source
Libreoffice Gerrit user
logerrit at kemper.freedesktop.org
Mon Oct 8 08:30:50 UTC 2018
sc/qa/unit/parallelism.cxx | 25 ++++++++++++++++++++
sc/source/core/tool/formulagroup.cxx | 43 ++++++++++++++++++++++++-----------
2 files changed, 55 insertions(+), 13 deletions(-)
New commits:
commit 7bafe2441480e2b88d999b30b7f117f05e72c3b3
Author: Dennis Francis <dennis.francis at collabora.co.uk>
AuthorDate: Thu Oct 4 12:28:14 2018 +0530
Commit: Michael Meeks <michael.meeks at collabora.com>
CommitDate: Mon Oct 8 10:30:29 2018 +0200
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 at collabora.com>
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
More information about the Libreoffice-commits
mailing list