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

[Dennis Francis (via logerrit)]

 sc/source/core/data/column4.cxx     |   17 ++++++++++++-----
 sc/source/core/data/formulacell.cxx |   16 ++++++++++++++++
 2 files changed, 28 insertions(+), 5 deletions(-)

New commits:
commit d4053e492af3815870e1b4e765486d2aaa29179d
Author:     Dennis Francis <dennis.francis at collabora.com>
AuthorDate: Fri Oct 25 13:36:45 2019 +0530
Commit:     Dennis Francis <dennis.francis at collabora.com>
CommitDate: Sat Oct 26 15:48:22 2019 +0200

    Fallback gracefully if intergroup dependencies found when...
    
    trying to do multi-formula-group threading. Do this like
    what we do for detecting formula-group cycles.
    
    This fixes the crash on the crashtest-document fdo31831-1.ods.
    
    Change-Id: I8115acc7dac39e12afb6fd7aafb9e7ce88d30ad4
    Reviewed-on: https://gerrit.libreoffice.org/81490
    Tested-by: Jenkins
    Reviewed-by: Dennis Francis <dennis.francis at collabora.com>

diff --git a/sc/source/core/data/column4.cxx b/sc/source/core/data/column4.cxx
index ed8608e6b750..de603298655d 100644
--- a/sc/source/core/data/column4.cxx
+++ b/sc/source/core/data/column4.cxx
@@ -1638,7 +1638,7 @@ std::unique_ptr<sc::ColumnIterator> ScColumn::GetColumnIterator( SCROW nRow1, SC
 }
 
 static bool lcl_InterpretSpan(sc::formula_block::const_iterator& rSpanIter, SCROW nStartOffset, SCROW nEndOffset,
-                              const ScFormulaCellGroupRef& mxParentGroup, bool& bAllowThreading)
+                              const ScFormulaCellGroupRef& mxParentGroup, bool& bAllowThreading, ScDocument& rDoc)
 {
     bAllowThreading = true;
     ScFormulaCell* pCellStart = nullptr;
@@ -1665,10 +1665,14 @@ static bool lcl_InterpretSpan(sc::formula_block::const_iterator& rSpanIter, SCRO
             // Found a completely dirty sub span [nSpanStart, nSpanEnd] inside the required span [nStartOffset, nEndOffset]
             bool bGroupInterpreted = pCellStart->Interpret(nSpanStart, nSpanEnd);
 
+            ScRecursionHelper& rRecursionHelper = rDoc.GetRecursionHelper();
             // child cell's Interpret could result in calling dependency calc
             // and that could detect a cycle involving mxGroup
             // and do early exit in that case.
-            if (mxParentGroup && mxParentGroup->mbPartOfCycle)
+            // OR
+            // this call resulted from a dependency calculation for a multi-formula-group-threading and
+            // if intergroup dependency is found, return early.
+            if ((mxParentGroup && mxParentGroup->mbPartOfCycle) || !rRecursionHelper.AreGroupsIndependent())
             {
                 // Set pCellStart as dirty as pCellStart may be interpreted in InterpretTail()
                 pCellStart->SetDirtyVar();
@@ -1695,6 +1699,7 @@ static void lcl_EvalDirty(sc::CellStoreType& rCells, SCROW nRow1, SCROW nRow2, S
                           const ScFormulaCellGroupRef& mxGroup, bool bThreadingDepEval, bool bSkipRunning,
                           bool& bIsDirty, bool& bAllowThreading)
 {
+    ScRecursionHelper& rRecursionHelper = rDoc.GetRecursionHelper();
     std::pair<sc::CellStoreType::const_iterator,size_t> aPos = rCells.position(nRow1);
     sc::CellStoreType::const_iterator it = aPos.first;
     size_t nOffset = aPos.second;
@@ -1734,7 +1739,6 @@ static void lcl_EvalDirty(sc::CellStoreType& rCells, SCROW nRow1, SCROW nRow2, S
                     // and return false
                     if (bThreadingDepEval && pChildTopCell->GetSeenInPath())
                     {
-                        ScRecursionHelper& rRecursionHelper = rDoc.GetRecursionHelper();
                         ScFormulaGroupCycleCheckGuard aCycleCheckGuard(rRecursionHelper, pChildTopCell);
                         bAllowThreading = false;
                         return;
@@ -1759,7 +1763,7 @@ static void lcl_EvalDirty(sc::CellStoreType& rCells, SCROW nRow1, SCROW nRow2, S
                         // The (main) span required to be evaluated is [nFGStartOffset, nFGEndOffset], but this span may contain
                         // non-dirty cells, so split this into sets of completely-dirty spans and try evaluate each of them in grouped-style.
 
-                        bool bAnyDirtyInSpan = lcl_InterpretSpan(itCell, nFGStartOffset, nFGEndOffset, mxGroup, bAllowThreading);
+                        bool bAnyDirtyInSpan = lcl_InterpretSpan(itCell, nFGStartOffset, nFGEndOffset, mxGroup, bAllowThreading, rDoc);
                         if (!bAllowThreading)
                             return;
                         // itCell will now point to cell just after the end of span [nFGStartOffset, nFGEndOffset].
@@ -1780,7 +1784,10 @@ static void lcl_EvalDirty(sc::CellStoreType& rCells, SCROW nRow1, SCROW nRow2, S
                         // child cell's Interpret could result in calling dependency calc
                         // and that could detect a cycle involving mxGroup
                         // and do early exit in that case.
-                        if (bThreadingDepEval && mxGroup && mxGroup->mbPartOfCycle)
+                        // OR
+                        // we are trying multi-formula-group-threading, but found intergroup dependency.
+                        if (bThreadingDepEval && mxGroup &&
+                            (mxGroup->mbPartOfCycle || !rRecursionHelper.AreGroupsIndependent()))
                         {
                             // Set itCell as dirty as itCell may be interpreted in InterpretTail()
                             (*itCell)->SetDirtyVar();
diff --git a/sc/source/core/data/formulacell.cxx b/sc/source/core/data/formulacell.cxx
index fafccf9dc769..a70fa9d702d7 100644
--- a/sc/source/core/data/formulacell.cxx
+++ b/sc/source/core/data/formulacell.cxx
@@ -1601,6 +1601,13 @@ bool ScFormulaCell::Interpret(SCROW nStartOffset, SCROW nEndOffset)
 #endif
         if (!bGroupInterpreted)
         {
+            // This call resulted from a dependency calculation for a multigroup-threading attempt,
+            // but found dependency among the groups.
+            if (!rRecursionHelper.AreGroupsIndependent())
+            {
+                pDocument->DecInterpretLevel();
+                return bGroupInterpreted;
+            }
             // Dependency calc inside InterpretFormulaGroup() failed due to
             // detection of a cycle and there are parent FG's in the cycle.
             // Skip InterpretTail() in such cases, only run InterpretTail for the "cycle-starting" FG
@@ -4657,6 +4664,7 @@ bool ScFormulaCell::CheckComputeDependencies(sc::FormulaLogger::GroupScope& rSco
         ScFormulaGroupDependencyComputeGuard aDepComputeGuard(rRecursionHelper);
         ScDependantsCalculator aCalculator(*pDocument, *pCode, *this, mxGroup->mpTopCell->aPos, fromFirstRow, nStartOffset, nEndOffset);
         bOKToParallelize = aCalculator.DoIt();
+
     }
 
     if (rRecursionHelper.IsInRecursionReturn())
@@ -4673,6 +4681,14 @@ bool ScFormulaCell::CheckComputeDependencies(sc::FormulaLogger::GroupScope& rSco
         return false;
     }
 
+    if (!rRecursionHelper.AreGroupsIndependent())
+    {
+        // This call resulted from a dependency calculation for a multigroup-threading attempt,
+        // but found dependency among the groups.
+        rScope.addMessage("multi-group-dependency failed");
+        return false;
+    }
+
     if (!bOKToParallelize)
     {
         mxGroup->meCalcState = sc::GroupCalcDisabled;