diff --git a/formula/qa/unit/OOXMLRewriterTest.cxx b/formula/qa/unit/OOXMLRewriterTest.cxx index b6c41a5cf766..47af68931752 100644 --- a/formula/qa/unit/OOXMLRewriterTest.cxx +++ b/formula/qa/unit/OOXMLRewriterTest.cxx @@ -80,6 +80,8 @@ private: return u"_xlfn.ANCHORARRAY"_ustr; case ocOffset: return u"OFFSET"_ustr; + case ocPercentSign: + return u"%"_ustr; case ocMissing: return u"missing"_ustr; case ocBad: @@ -240,6 +242,21 @@ CPPUNIT_TEST_FIXTURE(OOXMLRewriterTest, testAnAtWithNothingToEncloseIsDropped) CPPUNIT_ASSERT_EQUAL(u"missing"_ustr, describe(*pMissing)); } +CPPUNIT_TEST_FIXTURE(OOXMLRewriterTest, testASpillWithNoFactorBeforeItIsDropped) +{ + // A percent sign ends the factor, so the # that follows has nothing to enclose. The pair + // would span no tokens at all, so it goes and the # with it. + FormulaTokenArray aTokens; + addOperand(aTokens); + aTokens.AddOpCode(ocPercentSign); + aTokens.AddOpCode(ocSpill); + + const std::unique_ptr pRewritten = rewrite(aTokens); + + CPPUNIT_ASSERT(pRewritten); + CPPUNIT_ASSERT_EQUAL(u"operand %"_ustr, describe(*pRewritten)); +} + CPPUNIT_TEST_FIXTURE(OOXMLRewriterTest, testTextThatDidNotParseTakesNoWrapper) { // Parentheses cannot go around text that never parsed, so a wrapper waiting on it is diff --git a/formula/source/core/api/FormulaCompiler.cxx b/formula/source/core/api/FormulaCompiler.cxx index c4bf99ac64f9..32f62bb5b9a4 100644 --- a/formula/source/core/api/FormulaCompiler.cxx +++ b/formula/source/core/api/FormulaCompiler.cxx @@ -339,6 +339,9 @@ bool isPotentialRangeLeftOp( OpCode eOp ) switch (eOp) { case ocClose: + // The # comes after its operand, so it is the last opcode of a sub-expression that + // yields a reference. + case ocSpill: return true; default: return false; @@ -351,6 +354,9 @@ bool isRangeResultFunction( OpCode eOp ) { case ocIndirect: case ocOffset: + // In OOXML the # is written before its operand, so there it is the first opcode of a + // sub-expression that yields a reference. + case ocSpill: return true; default: return false; @@ -366,6 +372,9 @@ bool isRangeResultOpCode( OpCode eOp ) case ocIntersect: case ocIndirect: case ocOffset: + // The # postfix operator gives the spill range of the master cell before it, which + // is a reference just like the operators above. + case ocSpill: return true; default: return false; @@ -2321,15 +2330,32 @@ void FormulaCompiler::Factor() } } +void FormulaCompiler::SpillOperator() +{ + while (mpToken->GetOpCode() == ocSpill) + { // this operator _follows_ the reference it takes the spill range of, and binds + // to that reference alone + if (mbComputeII) + { + FormulaToken** pArg = mpCode - 1; + HandleIIOpCode(mpToken.get(), &pArg, 1); + } + PutCode(mpToken); + NextToken(); + } +} + void FormulaCompiler::RangeLine() { Factor(); + SpillOperator(); while (mpToken->GetOpCode() == ocRange) { FormulaToken** pCode1 = mpCode - 1; FormulaTokenRef p = mpToken; NextToken(); Factor(); + SpillOperator(); FormulaToken** pCode2 = mpCode - 1; if (!MergeRangeReference( pCode1, pCode2)) PutCode(p); @@ -2544,8 +2570,8 @@ void FormulaCompiler::UnaryLine() void FormulaCompiler::PostOpLine() { UnaryLine(); - while (mpToken->GetOpCode() == ocPercentSign || mpToken->GetOpCode() == ocSpill) - { // these operators _follow_ their operand + while (mpToken->GetOpCode() == ocPercentSign) + { // this operator _follows_ its operand if (mbComputeII) { FormulaToken** pArg = mpCode - 1; diff --git a/formula/source/core/api/OOXMLRewriter.cxx b/formula/source/core/api/OOXMLRewriter.cxx index 0d215feed790..83b19ea452b6 100644 --- a/formula/source/core/api/OOXMLRewriter.cxx +++ b/formula/source/core/api/OOXMLRewriter.cxx @@ -111,6 +111,8 @@ private: sal_uInt16 mnContentStart; // Start of the union-level expression we are reading. sal_uInt16 mnExpressionStart; + // Start of the current factor: an operand plus its parentheses and arguments. + sal_uInt16 mnFactorStart; // Start of an open union list, NO_POSITION if there is none. sal_uInt16 mnListStart; // The scope is a grouping parenthesis, not a call's argument list. @@ -232,6 +234,8 @@ void RewriteCollector::movePastToken(Scope& rScope, sal_uInt16 nPosition) rScope.mnContentStart = sal_uInt16(nPosition + 1); if (rScope.mnExpressionStart == nPosition) rScope.mnExpressionStart = sal_uInt16(nPosition + 1); + if (rScope.mnFactorStart == nPosition) + rScope.mnFactorStart = sal_uInt16(nPosition + 1); } // Mark pending operands as filled. A mark is never cleared, so walking from the back can @@ -295,7 +299,7 @@ void RewriteCollector::closeUnionList(sal_uInt16 nEnd, bool bMayTakeScopeParenth void RewriteCollector::collect() { const sal_uInt16 nLength = mrTokens.GetLen(); - maScopes.push_back({ 0, 0, NO_POSITION, false }); + maScopes.push_back({ 0, 0, 0, NO_POSITION, false }); // The next token is where an operand starts, so a plus or minus there is the unary form. bool bOperandPosition = true; // The current factor is raw text that never parsed - we cannot put wrapper parentheses @@ -315,29 +319,23 @@ void RewriteCollector::collect() if (eOp == ocSingleValue) { Scope& rScope = maScopes.back(); - // Any start sitting on the @ moves on to the operand after it. + // Any start sitting on the @ moves on to the operand after it, and so does the + // factor it begins. movePastToken(rScope, nPosition); + rScope.mnFactorStart = sal_uInt16(nPosition + 1); maPendingSingles.push_back({ maScopes.size(), sal_uInt16(nPosition + 1), false }); continue; } - // The # is written as a call around the whole expression under it, so the union list - // closes first. Text that never parsed takes no parentheses, so drop a # after one. + // The # is written as a call around the factor before it and binds tighter than any + // operator that can come before that factor. Raw text that never parsed takes no + // parentheses, so drop a # after one, and drop a # with no factor before it at all. if (eOp == ocSpill) { if (!bBadFactor) { - closeUnionList(nPosition, false); - const sal_uInt16 nBegin = maScopes.back().mnExpressionStart; - mrWrappers.push_back({ nBegin, nPosition, ocAnchorArray, true }); - // The # binds tighter than an @ before it, so an @ that starts inside the - // wrapper takes all of it as its operand. An @ on one part of a union list - // is such a case. - for (PendingSingleValue& rSingle : maPendingSingles) - { - if (rSingle.mnBegin > nBegin) - rSingle.mnBegin = nBegin; - } + mrWrappers.push_back( + { maScopes.back().mnFactorStart, nPosition, ocAnchorArray, true }); markPendingOperand(); } bOperandPosition = false; @@ -361,7 +359,8 @@ void RewriteCollector::collect() markPendingOperand(); // A grouping parenthesis is where an operand starts, a call's parenthesis comes // after the function name. - maScopes.push_back({ sal_uInt16(nPosition + 1), sal_uInt16(nPosition + 1), NO_POSITION, + maScopes.push_back({ sal_uInt16(nPosition + 1), sal_uInt16(nPosition + 1), + sal_uInt16(nPosition + 1), NO_POSITION, eOp == ocOpen && bOperandPosition }); bOperandPosition = true; bBadFactor = false; @@ -403,7 +402,7 @@ void RewriteCollector::collect() } Scope& rScope = maScopes.back(); - // Whitespace before a scope or an expression is not part of it. + // Whitespace before a scope, expression or factor is not part of it. if (bWhitespace) movePastToken(rScope, nPosition); if (!bPrefix && !bWhitespace && !lclWritesNoText(eOp)) @@ -419,6 +418,12 @@ void RewriteCollector::collect() // one does not, it belongs to the operand before it. rScope.mnExpressionStart = sal_uInt16(nPosition + 1); } + if (bPrefix || bSeparator || bEndsOperand) + { + // Every operator and separator ends the factor before it, so the next factor + // starts after. A function name does not. + rScope.mnFactorStart = sal_uInt16(nPosition + 1); + } if (bSeparator) bBadFactor = false; if (!bWhitespace) diff --git a/include/formula/FormulaCompiler.hxx b/include/formula/FormulaCompiler.hxx index 7e1185fbcd2c..bc497b6149b9 100644 --- a/include/formula/FormulaCompiler.hxx +++ b/include/formula/FormulaCompiler.hxx @@ -377,6 +377,7 @@ protected: void PutCode( FormulaTokenRef& ); void Factor(); void RangeLine(); + void SpillOperator(); void UnionLine(); void IntersectionLine(); void CallLine(); diff --git a/sc/qa/unit/subsequent_export_test4.cxx b/sc/qa/unit/subsequent_export_test4.cxx index b5ee422e4cc6..47a0d21426a8 100644 --- a/sc/qa/unit/subsequent_export_test4.cxx +++ b/sc/qa/unit/subsequent_export_test4.cxx @@ -1443,6 +1443,30 @@ CPPUNIT_TEST_FIXTURE(ScExportTest4, testParenthesizedSpilledRangeXlsxRoundTrip) CPPUNIT_ASSERT_EQUAL(u"=(A1)#"_ustr, pDoc->GetFormula(1, 0, 0)); } +CPPUNIT_TEST_FIXTURE(ScExportTest4, testIntersectedSpilledRangeXlsxRoundTrip) +{ + // OOXML writes the intersection operator as a blank and the # before its operand, so the + // two end up next to each other as "A1:A5 _xlfn.ANCHORARRAY(C1)". To read the blank back + // as the operator the import has to know that what follows yields a reference. + createScDoc(); + ScDocument* pDocument = getScDoc(); + + pDocument->SetFormula(ScAddress(1, 0, 0), u"=A1:A5!C1#"_ustr, + formula::FormulaGrammar::GRAM_NATIVE); + + save(TestFilter::XLSX); + + xmlDocUniquePtr pSheet = parseExport(u"xl/worksheets/sheet1.xml"_ustr); + CPPUNIT_ASSERT(pSheet); + + assertXPathContent(pSheet, "/x:worksheet/x:sheetData/x:row[1]/x:c[1]/x:f", + u"A1:A5 _xlfn.ANCHORARRAY(C1)"); + + saveAndReload(TestFilter::XLSX); + pDocument = getScDoc(); + CPPUNIT_ASSERT_EQUAL(u"=A1:A5!C1#"_ustr, pDocument->GetFormula(1, 0, 0)); +} + CPPUNIT_TEST_FIXTURE(ScExportTest4, testReferenceUnionXlsxRoundTrip) { // OOXML spells the union operator with the same comma that separates diff --git a/sc/qa/unit/ucalc_formula2.cxx b/sc/qa/unit/ucalc_formula2.cxx index b4e9ac2243d8..b974dbb81161 100644 --- a/sc/qa/unit/ucalc_formula2.cxx +++ b/sc/qa/unit/ucalc_formula2.cxx @@ -6877,6 +6877,32 @@ compileInNativeGrammar(ScDocument& rDocument, const OUString& rFormula, const Sc return pCode; } +// Compile a formula in native grammar down to RPN and return the parse error. +// FormulaError::NONE means the whole formula parsed as one expression. +FormulaError parseErrorInNativeGrammar(ScDocument& rDocument, const OUString& rFormula) +{ + const ScAddress aPosition(1, 0, 0); + std::unique_ptr pCode = compileInNativeGrammar(rDocument, rFormula, aPosition); + ScCompiler aCompiler(rDocument, aPosition, *pCode, formula::FormulaGrammar::GRAM_NATIVE); + aCompiler.CompileTokenArray(); + return pCode->GetCodeError(); +} + +// Compile a formula in a grammar where a blank is the intersection operator and print it as +// OOXML. Calc A1 uses an exclamation mark for that operator instead. +OUString compileInXlA1SyntaxAndPrintAsOoxml(ScDocument& rDocument, const OUString& rFormula) +{ + const ScAddress aPosition(1, 0, 0); + ScCompiler aCompiler(rDocument, aPosition, formula::FormulaGrammar::GRAM_ENGLISH_XL_A1, true, + false); + std::unique_ptr pCode = aCompiler.CompileString(rFormula); + CPPUNIT_ASSERT(pCode); + ScCompiler aRpnCompiler(rDocument, aPosition, *pCode, + formula::FormulaGrammar::GRAM_ENGLISH_XL_A1); + aRpnCompiler.CompileTokenArray(); + return printAsOoxml(rDocument, *pCode, aPosition); +} + // Compile a formula in native grammar and print it as OOXML. OUString compileAndPrintAsOoxml(ScDocument& rDocument, const OUString& rFormula) { @@ -7079,17 +7105,55 @@ CPPUNIT_TEST_FIXTURE(TestFormula2, testSpilledRangeBindsTighterThanSignAndMarker m_pDoc->DeleteTab(0); } -CPPUNIT_TEST_FIXTURE(TestFormula2, testSpilledRangeTakesTheWrapperThePercentSignClosed) +CPPUNIT_TEST_FIXTURE(TestFormula2, testIntersectionNextToASpilledRangeSurvives) { - // The percent sign ends an @ operand, so a # behind it takes the @ wrapper too. + // A # on either operand leaves the intersection operator between them alone, because the + // spill range the # yields is a reference like any other operand. m_pDoc->InsertTab(0, u"Sheet1"_ustr); - CPPUNIT_ASSERT_EQUAL(u"_xlfn.ANCHORARRAY(_xlfn.SINGLE(A1)%)"_ustr, - compileAndPrintAsOoxml(*m_pDoc, u"@A1%#"_ustr)); - CPPUNIT_ASSERT_EQUAL(u"_xlfn.ANCHORARRAY(_xlfn.SINGLE(-A1)%)"_ustr, + CPPUNIT_ASSERT_EQUAL(u"A1:A5 _xlfn.ANCHORARRAY(C1)"_ustr, + compileInXlA1SyntaxAndPrintAsOoxml(*m_pDoc, u"=A1:A5 C1#"_ustr)); + + // Same with the # on the operand before the blank. + CPPUNIT_ASSERT_EQUAL(u"_xlfn.ANCHORARRAY(A1) C1"_ustr, + compileInXlA1SyntaxAndPrintAsOoxml(*m_pDoc, u"=A1# C1"_ustr)); + + m_pDoc->DeleteTab(0); +} + +CPPUNIT_TEST_FIXTURE(TestFormula2, testPostfixOperatorWithoutAFactorIsNoFormula) +{ + // The # binds to the factor before it and a percent sign ends that factor, so a # or a + // union operator after one has nothing to bind to and the formula doesn't parse. OOXML + // has no spelling for either shape. + m_pDoc->InsertTab(0, u"Sheet1"_ustr); + + CPPUNIT_ASSERT_EQUAL(FormulaError::OperatorExpected, + parseErrorInNativeGrammar(*m_pDoc, u"=A1%#"_ustr)); + CPPUNIT_ASSERT_EQUAL(FormulaError::OperatorExpected, + parseErrorInNativeGrammar(*m_pDoc, u"=A1%~B2%"_ustr)); + + // The same operators parse fine when there is a factor in front of them. + CPPUNIT_ASSERT_EQUAL(FormulaError::NONE, parseErrorInNativeGrammar(*m_pDoc, u"=A1#"_ustr)); + CPPUNIT_ASSERT_EQUAL(FormulaError::NONE, parseErrorInNativeGrammar(*m_pDoc, u"=A1#%"_ustr)); + CPPUNIT_ASSERT_EQUAL(FormulaError::NONE, parseErrorInNativeGrammar(*m_pDoc, u"=A1~B2%"_ustr)); + + m_pDoc->DeleteTab(0); +} + +CPPUNIT_TEST_FIXTURE(TestFormula2, testSpilledRangeDroppedWithNoFactorInFrontOfIt) +{ + // A percent sign ends the factor before it, so a # after one has nothing to enclose and + // is dropped. OOXML has no spelling for that shape either, and no parse produces one. + m_pDoc->InsertTab(0, u"Sheet1"_ustr); + + CPPUNIT_ASSERT_EQUAL(u"A1%"_ustr, compileAndPrintAsOoxml(*m_pDoc, u"A1%#"_ustr)); + CPPUNIT_ASSERT_EQUAL(u"_xlfn.SINGLE(A1)%"_ustr, compileAndPrintAsOoxml(*m_pDoc, u"@A1%#"_ustr)); + CPPUNIT_ASSERT_EQUAL(u"_xlfn.SINGLE(-A1)%"_ustr, compileAndPrintAsOoxml(*m_pDoc, u"@-A1%#"_ustr)); - // A union list that follows such a wrapper begins in front of it as well. + // A union list that comes after an @ wrapper closed by the percent sign starts before + // that wrapper. CPPUNIT_ASSERT_EQUAL(u"(_xlfn.SINGLE(-A1)%,B2)"_ustr, compileAndPrintAsOoxml(*m_pDoc, u"@-A1%~B2"_ustr)); @@ -7154,8 +7218,8 @@ CPPUNIT_TEST_FIXTURE(TestFormula2, testUnionStaysInsideImplicitIntersectionWrapp CPPUNIT_ASSERT_EQUAL(u"(_xlfn.SINGLE( (A1,B2)))"_ustr, compileAndPrintAsOoxml(*m_pDoc, u"(@ A1~B2)"_ustr)); - // A # takes the whole list, so an @ on one part of it wraps the list's wrapper. - CPPUNIT_ASSERT_EQUAL(u"_xlfn.SINGLE(_xlfn.ANCHORARRAY((A1,B2)))"_ustr, + // A # takes the list part before it, and an @ in front of that part wraps around the #. + CPPUNIT_ASSERT_EQUAL(u"(A1,_xlfn.SINGLE(_xlfn.ANCHORARRAY(B2)))"_ustr, compileAndPrintAsOoxml(*m_pDoc, u"A1~@B2#"_ustr)); // An already parenthesised union list fills the wrapper on its own, for either operator. @@ -7171,30 +7235,40 @@ CPPUNIT_TEST_FIXTURE(TestFormula2, testUnionStaysInsideImplicitIntersectionWrapp m_pDoc->DeleteTab(0); } -CPPUNIT_TEST_FIXTURE(TestFormula2, testPostfixOperatorsApplyToTheWholeExpression) +CPPUNIT_TEST_FIXTURE(TestFormula2, testPostfixOperatorsBindWhereTheGrammarPutsThem) { - // The # and the % follow every operator that binds tighter than they do, so - // each takes the whole expression in front of it as its operand rather than - // the last part of it. + // The # takes the factor before it and nothing wider. The % is above every reference + // operator, so it takes the whole expression in front of it instead. A document from + // another application draws the same line. m_pDoc->InsertTab(0, u"Sheet1"_ustr); - CPPUNIT_ASSERT_EQUAL(u"_xlfn.ANCHORARRAY((A1,B2))"_ustr, + // One part of a union list is a factor, so the # takes that part and the % the list. + CPPUNIT_ASSERT_EQUAL(u"(A1,_xlfn.ANCHORARRAY(B2))"_ustr, compileAndPrintAsOoxml(*m_pDoc, u"A1~B2#"_ustr)); CPPUNIT_ASSERT_EQUAL(u"(A1,B2)%"_ustr, compileAndPrintAsOoxml(*m_pDoc, u"A1~B2%"_ustr)); - // The range operator binds tighter still, and a range built with it goes - // inside the wrapper whole. + // The first part of the list is a factor too, so a # there takes only that part and the + // list's own parentheses end up outside the wrapper. + CPPUNIT_ASSERT_EQUAL(u"(_xlfn.ANCHORARRAY(A1),B2)"_ustr, + compileAndPrintAsOoxml(*m_pDoc, u"A1#~B2"_ustr)); + + // A range reference is a single factor and goes inside the wrapper as a whole. A range + // built with the range operator has a factor on either side, and the # takes only the + // one before it. CPPUNIT_ASSERT_EQUAL(u"_xlfn.ANCHORARRAY(A1:A3)"_ustr, compileAndPrintAsOoxml(*m_pDoc, u"A1:A3#"_ustr)); - CPPUNIT_ASSERT_EQUAL(u"_xlfn.ANCHORARRAY(A1:INDEX(B1:B10,5))"_ustr, + CPPUNIT_ASSERT_EQUAL(u"A1:_xlfn.ANCHORARRAY(INDEX(B1:B10,5))"_ustr, compileAndPrintAsOoxml(*m_pDoc, u"A1:INDEX(B1:B10;5)#"_ustr)); + CPPUNIT_ASSERT_EQUAL(u"_xlfn.ANCHORARRAY(A1):C1"_ustr, + compileAndPrintAsOoxml(*m_pDoc, u"A1#:C1"_ustr)); - // A percent sign on one part of a union list keeps that part inside the list, and one - // after the list goes after the list's own parentheses. Neither shape comes out of a - // parse, which treats a percent sign as being above the union operator. + // A percent sign is above the union operator, so the one after the list goes after the + // list's own parentheses. The producing application only takes a percent sign in that + // trailing place, never on a part of the list, and no parse puts one there either, so + // the one on the first part stays where it is. CPPUNIT_ASSERT_EQUAL(u"(A1%,B2)%"_ustr, compileAndPrintAsOoxml(*m_pDoc, u"A1%~B2%"_ustr)); - // A prefix sign on one part of a union list stays outside the wrapper too. + // A prefix sign is above the # too, so it stays outside the wrapper. CPPUNIT_ASSERT_EQUAL(u"(A1,-_xlfn.ANCHORARRAY(B2))"_ustr, compileAndPrintAsOoxml(*m_pDoc, u"A1~-B2#"_ustr)); CPPUNIT_ASSERT_EQUAL(u"SUM((A1,-_xlfn.ANCHORARRAY(B2)))"_ustr, @@ -7224,8 +7298,10 @@ CPPUNIT_TEST_FIXTURE(TestFormula2, testUnionParenthesesEncloseExactlyTheirList) CPPUNIT_ASSERT_EQUAL(u"($A$1,_xlfn.SINGLE($B$2))"_ustr, printAsOoxml(*m_pDoc, aMarkerInList, ScAddress(1, 0, 0))); - // Same for a percent sign before the union operator. The sign comes after the first part - // of the list, so the list still starts before that part. + // Same for a percent sign before the union operator. The sign comes after the first + // part of the list, so the list still starts before that part. OOXML has no spelling + // for a percent sign there either, which is another reason no parse produces this + // shape. ScTokenArray aPercentInList(*m_pDoc); aPercentInList.AddSingleReference(aFirst); aPercentInList.AddOpCode(ocPercentSign); diff --git a/sc/qa/unit/ucalc_spilled_range.cxx b/sc/qa/unit/ucalc_spilled_range.cxx index 58ce7318a271..34c55db251e4 100644 --- a/sc/qa/unit/ucalc_spilled_range.cxx +++ b/sc/qa/unit/ucalc_spilled_range.cxx @@ -383,6 +383,59 @@ CPPUNIT_TEST_FIXTURE(TestSpilledRange, testSpilledRangeOperatorInExpression) m_pDoc->DeleteTab(0); } +CPPUNIT_TEST_FIXTURE(TestSpilledRange, testSpilledRangeAsReferenceOperatorOperand) +{ + // The reference operators take a # spilled range as the range it stands for. They pop it + // as a matrix that carries the range, and since they work on ranges they use the range + // and leave the values alone. + sc::AutoCalcSwitch aACSwitch(*m_pDoc, true); + m_pDoc->InsertTab(0, u"Sheet1"_ustr); + + m_pDoc->SetValue(ScAddress(4, 0, 0), 10.0); + m_pDoc->SetValue(ScAddress(4, 1, 0), 20.0); + m_pDoc->SetValue(ScAddress(4, 2, 0), 30.0); + + // Master at A1 spills =E1:E3 into A1:A3. + ScMarkData aMark(m_pDoc->GetSheetLimits()); + aMark.SelectOneTable(0); + m_pDoc->InsertMatrixFormula(0, 0, 0, 2, aMark, u"=E1:E3"_ustr); + + // B2 and C1 sit outside the spill range, so each operator's result is a distinct sum. + m_pDoc->SetValue(ScAddress(1, 1, 0), 7.0); + m_pDoc->SetValue(ScAddress(2, 0, 0), 5.0); + + // The intersection of A1:C2 with the spill range A1:A3 is A1:A2, whichever side the + // spill range is on. + m_pDoc->SetString(ScAddress(6, 0, 0), u"=SUM(A1:C2!A1#)"_ustr); + CPPUNIT_ASSERT_EQUAL(30.0, m_pDoc->GetValue(ScAddress(6, 0, 0))); + // The parentheses keep the # away from the intersection operator, because a # directly + // before an exclamation mark lexes as the start of an error constant. + m_pDoc->SetString(ScAddress(7, 0, 0), u"=SUM((A1#)!A1:C2)"_ustr); + CPPUNIT_ASSERT_EQUAL(30.0, m_pDoc->GetValue(ScAddress(7, 0, 0))); + + // The range operator spans the spill range and C1, which is A1:C3, so B2 comes in. + m_pDoc->SetString(ScAddress(6, 1, 0), u"=SUM(A1#:C1)"_ustr); + CPPUNIT_ASSERT_EQUAL(72.0, m_pDoc->GetValue(ScAddress(6, 1, 0))); + // The parentheses keep the left operand apart from the spill range, because C1:A1 on its + // own lexes as a single reference instead of two operands of the range operator. + m_pDoc->SetString(ScAddress(7, 1, 0), u"=SUM((C1):A1#)"_ustr); + CPPUNIT_ASSERT_EQUAL(FormulaError::NONE, m_pDoc->GetErrCode(ScAddress(7, 1, 0))); + CPPUNIT_ASSERT_EQUAL(72.0, m_pDoc->GetValue(ScAddress(7, 1, 0))); + + // The union operator keeps the parts apart, so B2 stays out. + m_pDoc->SetString(ScAddress(6, 2, 0), u"=SUM(A1#~C1)"_ustr); + CPPUNIT_ASSERT_EQUAL(65.0, m_pDoc->GetValue(ScAddress(6, 2, 0))); + m_pDoc->SetString(ScAddress(7, 2, 0), u"=SUM(C1~A1#)"_ustr); + CPPUNIT_ASSERT_EQUAL(65.0, m_pDoc->GetValue(ScAddress(7, 2, 0))); + + // With a third part the union operator extends a list it already built, which is a + // separate code path, so check that the spill range gets there too. + m_pDoc->SetString(ScAddress(6, 3, 0), u"=SUM(C1~B2~A1#)"_ustr); + CPPUNIT_ASSERT_EQUAL(72.0, m_pDoc->GetValue(ScAddress(6, 3, 0))); + + m_pDoc->DeleteTab(0); +} + CPPUNIT_TEST_FIXTURE(TestSpilledRange, testSpilledRangeOperatorFormulaStringRoundtrip) { // The # operator round-trips through GetFormula across several diff --git a/sc/source/core/inc/interpre.hxx b/sc/source/core/inc/interpre.hxx index b11eddae3155..a700da1774ec 100644 --- a/sc/source/core/inc/interpre.hxx +++ b/sc/source/core/inc/interpre.hxx @@ -397,6 +397,13 @@ private: assert(!p || dynamic_cast(p.get())); return static_cast(p.get()); } + /** Pop the operand of a reference operator. + + A spilled range arrives as a matrix that carries the range it was read from. These + operators work on ranges, so such a matrix comes back as the matching double + reference. Everything else comes back unchanged. + */ + formula::FormulaConstTokenRef PopReferenceOperand(); void Pop(); void PopError(); double PopDouble(); diff --git a/sc/source/core/tool/interpr2.cxx b/sc/source/core/tool/interpr2.cxx index 2868fec1e7b2..efb0683b4759 100644 --- a/sc/source/core/tool/interpr2.cxx +++ b/sc/source/core/tool/interpr2.cxx @@ -2408,8 +2408,8 @@ void ScInterpreter::ScMod() void ScInterpreter::ScIntersect() { - formula::FormulaConstTokenRef p2nd = PopToken(); - formula::FormulaConstTokenRef p1st = PopToken(); + formula::FormulaConstTokenRef p2nd = PopReferenceOperand(); + formula::FormulaConstTokenRef p1st = PopReferenceOperand(); if (nGlobalError != FormulaError::NONE || !p2nd || !p1st) { @@ -2551,8 +2551,8 @@ void ScInterpreter::ScIntersect() void ScInterpreter::ScRangeFunc() { - formula::FormulaConstTokenRef x2 = PopToken(); - formula::FormulaConstTokenRef x1 = PopToken(); + formula::FormulaConstTokenRef x2 = PopReferenceOperand(); + formula::FormulaConstTokenRef x1 = PopReferenceOperand(); if (nGlobalError != FormulaError::NONE || !x2 || !x1) { @@ -2571,8 +2571,8 @@ void ScInterpreter::ScRangeFunc() void ScInterpreter::ScUnionFunc() { - formula::FormulaConstTokenRef p2nd = PopToken(); - formula::FormulaConstTokenRef p1st = PopToken(); + formula::FormulaConstTokenRef p2nd = PopReferenceOperand(); + formula::FormulaConstTokenRef p1st = PopReferenceOperand(); if (nGlobalError != FormulaError::NONE || !p2nd || !p1st) { diff --git a/sc/source/core/tool/interpr4.cxx b/sc/source/core/tool/interpr4.cxx index b2527f5baf6d..a19c1c1d04cc 100644 --- a/sc/source/core/tool/interpr4.cxx +++ b/sc/source/core/tool/interpr4.cxx @@ -846,6 +846,20 @@ FormulaConstTokenRef ScInterpreter::PopTokenImpl() return nullptr; } +FormulaConstTokenRef ScInterpreter::PopReferenceOperand() +{ + FormulaConstTokenRef xToken = PopToken(); + if (!xToken || xToken->GetType() != svMatrix + || !static_cast(xToken.get())->IsMatrixRangeToken()) + { + return xToken; + } + + const ScComplexRefData& rReference + = static_cast(xToken.get())->GetDoubleRef(); + return FormulaConstTokenRef(new ScDoubleRefToken(mrDoc.GetSheetLimits(), rReference)); +} + double ScInterpreter::PopDouble() { nCurFmtType = SvNumFormatType::NUMBER;