Detect circular dependency in formula groups to fallback to cell-based.

Change-Id: Icfb4fd4be4e0c1f6f450fb8ddce57c7b79568e6f

sc/inc/formulacell.hxx

@@ -24,6 +24,7 @@
 
 #include "formula/tokenarray.hxx"
 #include "svl/listener.hxx"
+#include "types.hxx"
 
 #include <set>
 
@@ -40,19 +41,23 @@ struct ScSimilarFormulaDelta;
 
 struct SC_DLLPUBLIC ScFormulaCellGroup
 {
-    sal_Int32 mnRefCount;
+    mutable size_t mnRefCount;
+
     SCROW mnStart;  // Start offset of that cell
     SCROW mnLength; // How many of these do we have ?
     bool mbInvariant;
+    sc::GroupCalcState meCalcState;
 
     ScFormulaCellGroup();
     ~ScFormulaCellGroup();
 };
-inline void intrusive_ptr_add_ref(ScFormulaCellGroup *p)
+
+inline void intrusive_ptr_add_ref(const ScFormulaCellGroup *p)
 {
     p->mnRefCount++;
 }
-inline void intrusive_ptr_release(ScFormulaCellGroup *p)
+
+inline void intrusive_ptr_release(const ScFormulaCellGroup *p)
 {
     if( --p->mnRefCount == 0 )
         delete p;

sc/inc/types.hxx

@@ -56,6 +56,13 @@ const sal_uInt16 MatrixEdgeTop     = 8;
 const sal_uInt16 MatrixEdgeRight   = 16;
 const sal_uInt16 MatrixEdgeOpen    = 32;
 
+enum GroupCalcState
+{
+    GroupCalcEnabled,
+    GroupCalcRunning,
+    GroupCalcDisabled
+};
+
 }
 
 #endif

sc/source/core/data/column2.cxx

@@ -2166,10 +2166,15 @@ bool appendDouble(
                     nLenRemain = 0;
                 }
 
+                sal_uInt16 nErr;
+                double fVal;
                 for (; itData != itDataEnd; ++itData)
                 {
                     ScFormulaCell& rFC = **itData;
-                    rArray.push_back(rFC.GetValue());
+                    if (!rFC.GetErrorOrValue(nErr, fVal) || nErr)
+                        return false;
+
+                    rArray.push_back(fVal);
                 }
             }
             break;
@@ -2238,6 +2243,9 @@ const double* ScColumn::FetchDoubleArray( sc::FormulaGroupContext& rCxt, SCROW n
         break;
         case sc::element_type_formula:
         {
+            sal_uInt16 nErr;
+            double fVal;
+
             rCxt.maArrays.push_back(new sc::FormulaGroupContext::DoubleArrayType);
             sc::FormulaGroupContext::DoubleArrayType& rArray = rCxt.maArrays.back();
             rArray.reserve(nLenRequested);
@@ -2252,7 +2260,10 @@ const double* ScColumn::FetchDoubleArray( sc::FormulaGroupContext& rCxt, SCROW n
                 for (; it != itEnd; ++it)
                 {
                     ScFormulaCell& rCell = **it;
-                    rArray.push_back(rCell.GetValue()); // the cell may be interpreted.
+                    if (!rCell.GetErrorOrValue(nErr, fVal) || nErr)
+                        return NULL;
+
+                    rArray.push_back(fVal);
                 }
 
                 return &rArray[0];
@@ -2264,7 +2275,10 @@ const double* ScColumn::FetchDoubleArray( sc::FormulaGroupContext& rCxt, SCROW n
             for (; it != itEnd; ++it)
             {
                 ScFormulaCell& rCell = **it;
-                rArray.push_back(rCell.GetValue()); // the cell may be interpreted.
+                if (!rCell.GetErrorOrValue(nErr, fVal) || nErr)
+                    return NULL;
+
+                rArray.push_back(fVal);
             }
 
             // Fill the remaining array with values from the following blocks.

sc/source/core/data/formulacell.cxx

@@ -382,7 +382,11 @@ void adjustDBRange(ScToken* pToken, ScDocument& rNewDoc, const ScDocument* pOldD
 }
 
 ScFormulaCellGroup::ScFormulaCellGroup() :
-    mnRefCount(0), mnStart(0), mnLength(0), mbInvariant(false)
+    mnRefCount(0),
+    mnStart(0),
+    mnLength(0),
+    mbInvariant(false),
+    meCalcState(sc::GroupCalcEnabled)
 {
 }
 
@@ -1547,6 +1551,9 @@ void ScFormulaCell::SetDirty( bool bDirtyFlag )
 void ScFormulaCell::SetDirtyVar()
 {
     bDirty = true;
+    if (xGroup)
+        xGroup->meCalcState = sc::GroupCalcEnabled;
+
     // mark the sheet of this cell to be calculated
     //#FIXME do we need to revert this remnant of old fake vba events? pDocument->AddCalculateTable( aPos.Tab() );
 }
@@ -1611,7 +1618,7 @@ void ScFormulaCell::SetErrCode( sal_uInt16 n )
 void ScFormulaCell::AddRecalcMode( ScRecalcMode nBits )
 {
     if ( (nBits & RECALCMODE_EMASK) != RECALCMODE_NORMAL )
-        bDirty = true;
+        SetDirtyVar();
     if ( nBits & RECALCMODE_ONLOAD_ONCE )
     {   // OnLoadOnce nur zum Dirty setzen nach Filter-Import
         nBits = (nBits & ~RECALCMODE_EMASK) | RECALCMODE_NORMAL;
@@ -2959,9 +2966,10 @@ class GroupTokenConverter
     ScTokenArray& mrGroupTokens;
     ScDocument& mrDoc;
     ScFormulaCell& mrCell;
+    const ScAddress& mrPos;
 public:
-    GroupTokenConverter(sc::FormulaGroupContext& rCxt, ScTokenArray& rGroupTokens, ScDocument& rDoc, ScFormulaCell& rCell) :
-        mrCxt(rCxt), mrGroupTokens(rGroupTokens), mrDoc(rDoc), mrCell(rCell) {}
+    GroupTokenConverter(sc::FormulaGroupContext& rCxt, ScTokenArray& rGroupTokens, ScDocument& rDoc, ScFormulaCell& rCell, const ScAddress& rPos) :
+        mrCxt(rCxt), mrGroupTokens(rGroupTokens), mrDoc(rDoc), mrCell(rCell), mrPos(rPos) {}
 
     bool convert(ScTokenArray& rCode)
     {
@@ -2979,8 +2987,7 @@ public:
                 case svSingleRef:
                 {
                     ScSingleRefData aRef = pToken->GetSingleRef();
-                    aRef.CalcAbsIfRel(mrCell.aPos);
-                    ScAddress aRefPos(aRef.nCol, aRef.nRow, aRef.nTab);
+                    ScAddress aRefPos = aRef.toAbs(mrPos);
                     if (aRef.IsRowRel())
                     {
                         // Fetch double array guarantees that the length of the
@@ -3108,6 +3115,13 @@ bool ScFormulaCell::InterpretFormulaGroup()
     if (!xGroup || !pCode)
         return false;
 
+    fprintf(stdout, "ScFormulaCell::InterpretFormulaGroup:   calc state = %d\n", xGroup->meCalcState);
+    if (xGroup->meCalcState == sc::GroupCalcDisabled)
+    {
+        fprintf(stdout, "ScFormulaCell::InterpretFormulaGroup:   group calc disabled.\n");
+        return false;
+    }
+
     switch (pCode->GetVectorState())
     {
         case FormulaVectorEnabled:
@@ -3126,10 +3140,26 @@ bool ScFormulaCell::InterpretFormulaGroup()
 
     sc::FormulaGroupContext aCxt;
     ScTokenArray aCode;
-    GroupTokenConverter aConverter(aCxt, aCode, *pDocument, *this);
+    ScAddress aTopPos = aPos;
+    aTopPos.SetRow(xGroup->mnStart);
+    GroupTokenConverter aConverter(aCxt, aCode, *pDocument, *this, aTopPos);
     if (!aConverter.convert(*pCode))
+    {
+        fprintf(stdout, "ScFormulaCell::InterpretFormulaGroup:   disabling group calc due to failed conversion\n");
+        xGroup->meCalcState = sc::GroupCalcDisabled;
+        return false;
+    }
+
+    xGroup->meCalcState = sc::GroupCalcRunning;
+    if (!sc::FormulaGroupInterpreter::getStatic()->interpret(*pDocument, aTopPos, xGroup, aCode))
+    {
+        fprintf(stdout, "ScFormulaCell::InterpretFormulaGroup:   disabling group calc due to failed calculation\n");
+        xGroup->meCalcState = sc::GroupCalcDisabled;
         return false;
-    return sc::FormulaGroupInterpreter::getStatic()->interpret(*pDocument, aPos, xGroup, aCode);
+    }
+
+    xGroup->meCalcState = sc::GroupCalcEnabled;
+    return true;
 }
 
 bool ScFormulaCell::InterpretInvariantFormulaGroup()

sc/source/core/tool/formulagroup.cxx

@@ -33,18 +33,14 @@ bool FormulaGroupInterpreterSoftware::interpret(ScDocument& rDoc, const ScAddres
 {
     // Decompose the group into individual cells and calculate them individually.
 
-    // Always set the top row to be the top of the group.  Grouped formula
-    // cells are to be calculated for its full segment at all times.
-
-    ScAddress aTopPos = rTopPos;
-    aTopPos.SetRow(xGroup->mnStart);
-    ScAddress aTmpPos = aTopPos;
+    // The caller must ensure that the top position is the start position of
+    // the group.
 
+    ScAddress aTmpPos = rTopPos;
     std::vector<double> aResults;
     aResults.reserve(xGroup->mnLength);
-    for (SCROW i = 0; i < xGroup->mnLength; ++i)
+    for (SCROW i = 0; i < xGroup->mnLength; ++i, aTmpPos.IncRow())
     {
-        aTmpPos.SetRow(aTopPos.Row() + i);
         ScTokenArray aCode2;
         for (const formula::FormulaToken* p = rCode.First(); p; p = rCode.Next())
         {
@@ -103,7 +99,7 @@ bool FormulaGroupInterpreterSoftware::interpret(ScDocument& rDoc, const ScAddres
     } // for loop end (xGroup->mnLength)
 
     if (!aResults.empty())
-        rDoc.SetFormulaResults(aTopPos, &aResults[0], aResults.size());
+        rDoc.SetFormulaResults(rTopPos, &aResults[0], aResults.size());
 
     return true;
 }