diff options
author | Luboš Luňák <l.lunak@collabora.com> | 2018-08-23 12:27:24 +0200 |
---|---|---|
committer | Eike Rathke <erack@redhat.com> | 2018-08-31 10:52:31 +0200 |
commit | 8eafa504e99bdf946e006527092d1974f18b66cc (patch) | |
tree | 18a0aacb289a3700866c40d15c68a801050655c1 /sc | |
parent | 965ac9915280e3d570d7b32ff20799507f4e42eb (diff) |
properly reset nSeenInIteration when iterating cell cycles
ooo#956-3 has a somewhat complex cell cycle structure. Calc detects
cycle G40, H40, H47, H37, D40 -> G40, marks these cells as bIsIterCell,
adds them to the list of cells in RecursionHelper and starts iterating.
However, there are more cells involved, e.g. there's another cycle
D41, G41, H41, H47, H37 -> D41, which Calc doesn't detect (probably
because it partially overlaps with the detected cycle).
This means that InterpretTail() was setting nSeenInIteration for every
involved cell, but it wasn't unset, because some of the cells weren't
know to be part of the iteration. And so on the next recalc, these
cells weren't interpreted, because Interpret() aborted early because
of the stale nSeenInIteration value. And in threaded calculation, this
eventually led to hitting an assert in MaybeInterpret().
And obvious fix for this is to set nSeenInIteration only for cells
that Calc treats as being part of the iteration. Which is what this
commit does.
That's however not a complete fix. Doing a recalc with ooo#956-3 now
at least gives somewhat sensible values, but it needs repeated hard
recalcs to actually reach the correct values that converge, or the delta
change in the iteration settings need to be (needlessly) small, 1E-6,
while Excel manages with just 1E-2. So what also should be done is probably
detecting all cells involved in the cycle and treating them as such.
Change-Id: I05fd149b453363c335f1f5d5d3748354ae624613
Reviewed-on: https://gerrit.libreoffice.org/59495
Tested-by: Jenkins
Reviewed-by: Eike Rathke <erack@redhat.com>
Diffstat (limited to 'sc')
-rw-r--r-- | sc/qa/unit/data/xls/pass/ooo956-3.xls | bin | 0 -> 34304 bytes | |||
-rw-r--r-- | sc/source/core/data/formulacell.cxx | 18 |
2 files changed, 16 insertions, 2 deletions
diff --git a/sc/qa/unit/data/xls/pass/ooo956-3.xls b/sc/qa/unit/data/xls/pass/ooo956-3.xls Binary files differnew file mode 100644 index 000000000000..dc8f353b2bc9 --- /dev/null +++ b/sc/qa/unit/data/xls/pass/ooo956-3.xls diff --git a/sc/source/core/data/formulacell.cxx b/sc/source/core/data/formulacell.cxx index 660ea65f0a1a..9ad6d4b9cb86 100644 --- a/sc/source/core/data/formulacell.cxx +++ b/sc/source/core/data/formulacell.cxx @@ -1457,8 +1457,15 @@ class RecursionCounter { ScRecursionHelper& rRec; bool bStackedInIteration; +#ifdef DBG_UTIL + const ScFormulaCell* cell; +#endif public: - RecursionCounter( ScRecursionHelper& r, ScFormulaCell* p ) : rRec(r) + RecursionCounter( ScRecursionHelper& r, ScFormulaCell* p ) + : rRec(r) +#ifdef DBG_UTIL + , cell(p) +#endif { bStackedInIteration = rRec.IsDoingIteration(); if (bStackedInIteration) @@ -1469,7 +1476,12 @@ public: { rRec.DecRecursionCount(); if (bStackedInIteration) + { +#ifdef DBG_UTIL + assert(rRec.GetRecursionInIterationStack().top() == cell); +#endif rRec.GetRecursionInIterationStack().pop(); + } } }; } @@ -1806,7 +1818,9 @@ void ScFormulaCell::Interpret() void ScFormulaCell::InterpretTail( ScInterpreterContext& rContext, ScInterpretTailParameter eTailParam ) { RecursionCounter aRecursionCounter( pDocument->GetRecursionHelper(), this); - nSeenInIteration = pDocument->GetRecursionHelper().GetIteration(); + // TODO If this cell is not an iteration cell, add it to the list of iteration cells? + if(bIsIterCell) + nSeenInIteration = pDocument->GetRecursionHelper().GetIteration(); if( !pCode->GetCodeLen() && pCode->GetCodeError() == FormulaError::NONE ) { // #i11719# no RPN and no error and no token code but result string present |