summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--sc/inc/recursionhelper.hxx10
-rw-r--r--sc/qa/unit/parallelism.cxx54
-rw-r--r--sc/source/core/data/column4.cxx2
-rw-r--r--sc/source/core/data/formulacell.cxx15
-rw-r--r--sc/source/core/tool/recursionhelper.cxx18
5 files changed, 95 insertions, 4 deletions
diff --git a/sc/inc/recursionhelper.hxx b/sc/inc/recursionhelper.hxx
index 10c4ddc6f02e..48508ec61e64 100644
--- a/sc/inc/recursionhelper.hxx
+++ b/sc/inc/recursionhelper.hxx
@@ -63,6 +63,7 @@ class ScRecursionHelper
bool bInIterationReturn;
bool bConverging;
bool bGroupsIndependent;
+ bool bAbortingDependencyComputation;
std::vector< ScFormulaCell* > aTemporaryGroupCells;
o3tl::sorted_vector< ScFormulaCellGroup* >* pFGSet;
@@ -76,8 +77,8 @@ public:
void IncRecursionCount() { ++nRecursionCount; }
void DecRecursionCount() { --nRecursionCount; }
sal_uInt16 GetDepComputeLevel() const { return nDependencyComputationLevel; }
- void IncDepComputeLevel() { ++nDependencyComputationLevel; }
- void DecDepComputeLevel() { --nDependencyComputationLevel; }
+ void IncDepComputeLevel();
+ void DecDepComputeLevel();
/// A pure recursion return, no iteration.
bool IsInRecursionReturn() const { return bInRecursionReturn &&
!bInIterationReturn; }
@@ -115,6 +116,11 @@ public:
bool AnyCycleMemberInDependencyEvalMode(ScFormulaCell* pCell);
bool AnyParentFGInCycle();
void SetFormulaGroupDepEvalMode(bool bSet);
+ // When dependency computation detects a cycle, it may not compute proper cell values.
+ // This sets a flag that ScFormulaCell will use to avoid setting those new values
+ // and resetting the dirty flag, until the dependency computation bails out.
+ void AbortDependencyComputation();
+ bool IsAbortingDependencyComputation() const { return bAbortingDependencyComputation; }
void AddTemporaryGroupCell(ScFormulaCell* cell);
void CleanTemporaryGroupCells();
diff --git a/sc/qa/unit/parallelism.cxx b/sc/qa/unit/parallelism.cxx
index ae5bf3c0b132..2b5b0ba73cc5 100644
--- a/sc/qa/unit/parallelism.cxx
+++ b/sc/qa/unit/parallelism.cxx
@@ -48,6 +48,7 @@ public:
void testFormulaGroupWithForwardSelfReference();
void testFormulaGroupsInCyclesAndWithSelfReference();
void testFormulaGroupsInCyclesAndWithSelfReference2();
+ void testFormulaGroupsInCyclesAndWithSelfReference3();
CPPUNIT_TEST_SUITE(ScParallelismTest);
CPPUNIT_TEST(testSUMIFS);
@@ -66,6 +67,7 @@ public:
CPPUNIT_TEST(testFormulaGroupWithForwardSelfReference);
CPPUNIT_TEST(testFormulaGroupsInCyclesAndWithSelfReference);
CPPUNIT_TEST(testFormulaGroupsInCyclesAndWithSelfReference2);
+ CPPUNIT_TEST(testFormulaGroupsInCyclesAndWithSelfReference3);
CPPUNIT_TEST_SUITE_END();
private:
@@ -979,6 +981,58 @@ void ScParallelismTest::testFormulaGroupsInCyclesAndWithSelfReference2()
m_pDoc->DeleteTab(0);
}
+void ScParallelismTest::testFormulaGroupsInCyclesAndWithSelfReference3()
+{
+ sc::AutoCalcSwitch aACSwitch(*m_pDoc, false);
+ m_pDoc->InsertTab(0, "1");
+
+ m_pDoc->SetValue(1, 1, 0, 2.0); // B2 <== 2
+ for (size_t nRow = 1; nRow < 105; ++nRow)
+ {
+ // Formula-group in B3:B104 with first cell "=D2+0.001"
+ if( nRow != 1 )
+ m_pDoc->SetFormula(ScAddress(1, nRow, 0), "=D" + OUString::number(nRow) + "+0.001",
+ formula::FormulaGrammar::GRAM_NATIVE_UI);
+ // Formula-group in C2:C104 with first cell "=B2*1.01011"
+ m_pDoc->SetFormula(ScAddress(2, nRow, 0), "=B" + OUString::number(nRow + 1) + "*1.01011",
+ formula::FormulaGrammar::GRAM_NATIVE_UI);
+ // Formula-group in D2:C104 with first cell "=C2*1.02"
+ m_pDoc->SetFormula(ScAddress(3, nRow, 0), "=C" + OUString::number(nRow + 1) + "*1.02",
+ formula::FormulaGrammar::GRAM_NATIVE_UI);
+ }
+
+ m_xDocShell->DoHardRecalc();
+
+ // What happens with tdf#132451 is that the copy&paste C6->C5 really just sets the dirty flag
+ // for C5 and all the cells that depend on it (D5,B6,C6,D6,B7,...), and it also resets
+ // flags marking the C formula group as disabled for parallel calculation because of the cycle.
+ m_pDoc->SetFormula(ScAddress(2, 4, 0), "=B5*1.01011", formula::FormulaGrammar::GRAM_NATIVE_UI);
+ m_pDoc->GetFormulaCell(ScAddress(2,4,0))->GetCellGroup()->mbPartOfCycle = false;
+ m_pDoc->GetFormulaCell(ScAddress(2,4,0))->GetCellGroup()->meCalcState = sc::GroupCalcEnabled;
+
+ m_pDoc->SetAutoCalc(true);
+ // Without the fix, getting value of C5 would try to parallel-interpret formula group in B
+ // from its first dirty cell (B6), which depends on D5, which depends on C5, where the cycle
+ // would be detected and dependency check would bail out. But the result from Interpret()-ing
+ // D5 would be used and D5's dirty flag reset, with D5 value incorrect.
+ m_pDoc->GetValue(2,4,0);
+
+ double fExpected[2][3] = {
+ { 2.19053373572776, 2.21268003179597, 2.25693363243189 },
+ { 2.25793363243189, 2.28076134145577, 2.32637656828489 }
+ };
+ for (size_t nCol = 1; nCol < 4; ++nCol)
+ {
+ for (size_t nRow = 4; nRow < 6; ++nRow)
+ {
+ OString aMsg = "Value at Cell (Col = " + OString::number(nCol) + ", Row = " + OString::number(nRow) + ", Tab = 0)";
+ ASSERT_DOUBLES_EQUAL_MESSAGE(aMsg.getStr(), fExpected[nRow - 4][nCol - 1], m_pDoc->GetValue(nCol, nRow, 0));
+ }
+ }
+
+ m_pDoc->DeleteTab(0);
+}
+
CPPUNIT_TEST_SUITE_REGISTRATION(ScParallelismTest);
CPPUNIT_PLUGIN_IMPLEMENT();
diff --git a/sc/source/core/data/column4.cxx b/sc/source/core/data/column4.cxx
index 1b23ff713ce5..3ca9a2892357 100644
--- a/sc/source/core/data/column4.cxx
+++ b/sc/source/core/data/column4.cxx
@@ -1677,8 +1677,6 @@ static bool lcl_InterpretSpan(sc::formula_block::const_iterator& rSpanIter, SCRO
// 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();
bAllowThreading = false;
return bAnyDirty;
}
diff --git a/sc/source/core/data/formulacell.cxx b/sc/source/core/data/formulacell.cxx
index 448c778c8995..e20a147ef528 100644
--- a/sc/source/core/data/formulacell.cxx
+++ b/sc/source/core/data/formulacell.cxx
@@ -1528,6 +1528,11 @@ bool ScFormulaCell::Interpret(SCROW nStartOffset, SCROW nEndOffset)
ScRecursionHelper& rRecursionHelper = pDocument->GetRecursionHelper();
bool bGroupInterpreted = false;
+ // The result would possibly depend on a cell without a valid value, bail out
+ // the entire dependency computation.
+ if (rRecursionHelper.IsAbortingDependencyComputation())
+ return false;
+
if ((mxGroup && !rRecursionHelper.CheckFGIndependence(mxGroup.get())) || !rRecursionHelper.AreGroupsIndependent())
return bGroupInterpreted;
@@ -1545,6 +1550,10 @@ bool ScFormulaCell::Interpret(SCROW nStartOffset, SCROW nEndOffset)
// Reaching here does not necessarily mean a circular reference, so don't set Err:522 here yet.
// If there is a genuine circular reference, it will be marked so when all groups
// in the cycle get out of dependency evaluation mode.
+ // But returning without calculation a new value means other cells depending
+ // on this one would use a possibly invalid value, so ensure the dependency
+ // computation is aborted without resetting the dirty flag of any cell.
+ rRecursionHelper.AbortDependencyComputation();
return bGroupInterpreted;
}
@@ -1955,6 +1964,12 @@ void ScFormulaCell::InterpretTail( ScInterpreterContext& rContext, ScInterpretTa
}
bRunning = bOldRunning;
+ // The result may be invalid or depend on another invalid result, just abort
+ // without updating the cell value. Since the dirty flag will not be reset,
+ // the proper value will be computed later.
+ if(pDocument->GetRecursionHelper().IsAbortingDependencyComputation())
+ return;
+
// #i102616# For single-sheet saving consider only content changes, not format type,
// because format type isn't set on loading (might be changed later)
bool bContentChanged = false;
diff --git a/sc/source/core/tool/recursionhelper.cxx b/sc/source/core/tool/recursionhelper.cxx
index efbf924ab853..d50fca1d87f2 100644
--- a/sc/source/core/tool/recursionhelper.cxx
+++ b/sc/source/core/tool/recursionhelper.cxx
@@ -15,6 +15,7 @@ void ScRecursionHelper::Init()
nRecursionCount = 0;
nDependencyComputationLevel = 0;
bInRecursionReturn = bDoingRecursion = bInIterationReturn = false;
+ bAbortingDependencyComputation = false;
aInsertPos = GetIterationEnd();
ResetIteration();
// Must not force clear aFGList ever.
@@ -195,6 +196,23 @@ void ScRecursionHelper::SetFormulaGroupDepEvalMode(bool bSet)
aInDependencyEvalMode.back() = bSet;
}
+void ScRecursionHelper::AbortDependencyComputation()
+{
+ assert( nDependencyComputationLevel > 0 );
+ bAbortingDependencyComputation = true;
+}
+
+void ScRecursionHelper::IncDepComputeLevel()
+{
+ ++nDependencyComputationLevel;
+}
+
+void ScRecursionHelper::DecDepComputeLevel()
+{
+ --nDependencyComputationLevel;
+ bAbortingDependencyComputation = false;
+}
+
void ScRecursionHelper::AddTemporaryGroupCell(ScFormulaCell* cell)
{
aTemporaryGroupCells.push_back( cell );
generated by cgit (git 2.34.1) at 2025-01-11 17:48:52 +0000