summaryrefslogtreecommitdiff
path: root/sc
diff options
context:
space:
mode:
authorLuboš Luňák <l.lunak@collabora.com>2018-08-23 12:27:24 +0200
committerEike Rathke <erack@redhat.com>2018-08-31 10:52:31 +0200
commit8eafa504e99bdf946e006527092d1974f18b66cc (patch)
tree18a0aacb289a3700866c40d15c68a801050655c1 /sc
parent965ac9915280e3d570d7b32ff20799507f4e42eb (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.xlsbin0 -> 34304 bytes
-rw-r--r--sc/source/core/data/formulacell.cxx18
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
new file mode 100644
index 000000000000..dc8f353b2bc9
--- /dev/null
+++ b/sc/qa/unit/data/xls/pass/ooo956-3.xls
Binary files differ
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