[Libreoffice-commits] core.git: sc/qa sc/source

[Libreoffice Gerrit user]

 sc/qa/unit/data/xls/pass/ooo956-3.xls |binary
 sc/source/core/data/formulacell.cxx   |   18 ++++++++++++++++--
 2 files changed, 16 insertions(+), 2 deletions(-)

New commits:
commit 8eafa504e99bdf946e006527092d1974f18b66cc
Author:     Luboš Luňák <l.lunak at collabora.com>
AuthorDate: Thu Aug 23 12:27:24 2018 +0200
Commit:     Eike Rathke <erack at redhat.com>
CommitDate: Fri Aug 31 10:52:31 2018 +0200

    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 at redhat.com>

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
Binary files /dev/null and b/sc/qa/unit/data/xls/pass/ooo956-3.xls 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