From 1ff770e5bf599fdb674e844202bbfd6927a136c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Toma=C5=BE=20Vajngerl?= Date: Fri, 21 Aug 2026 23:41:01 +0900 Subject: [PATCH] sc: bind the # operator to the reference it follows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The # postfix operator was parsed at the same level as the percent sign, which is looser than the prefix operators. Because of this =@A1# compiled to the spill range of an implicit intersection and =-A1# to the spill range of a negated value. Neither of those means anything, and OOXML writes the # around the reference alone in both cases. The # is now parsed right after the factor it follows, tighter than any operator that can come before it. A whole range reference goes inside, because the lexer reads A1:A3 as a single reference. The percent sign stays above the reference operators, so =A1%# leaves the # with nothing to bind to and doesn't parse, and the export drops such a # (OOXML has no spelling for it either). A blank between two operands is only the intersection operator if both sides yield a reference. The # does, so the checks that keep the blank alive and that keep the operator once the operands are known had to be told about it. The interpreter passes the spill range on as a matrix that carries the range it was read from. The intersection, range and union operators only took references and refused that, so PopReferenceOperand now hands them the carried range as a double reference. Signed-off-by: Tomaž Vajngerl Change-Id: I1a587bf291ff639b326a366b8efc332106eb5fd3 Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/10156 Tested-by: Tomaž Vajngerl Reviewed-by: Miklos Vajna (cherry picked from commit c589c34aaec00d60fc47f1933762b105f9d22de3) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/210293 Tested-by: Xisco Fauli Reviewed-by: Xisco Fauli --- formula/qa/unit/OOXMLRewriterTest.cxx | 17 +++ formula/source/core/api/FormulaCompiler.cxx | 30 ++++- formula/source/core/api/OOXMLRewriter.cxx | 39 ++++--- include/formula/FormulaCompiler.hxx | 1 + sc/qa/unit/subsequent_export_test4.cxx | 24 ++++ sc/qa/unit/ucalc_formula2.cxx | 120 ++++++++++++++++---- sc/qa/unit/ucalc_spilled_range.cxx | 53 +++++++++ sc/source/core/inc/interpre.hxx | 7 ++ sc/source/core/tool/interpr2.cxx | 12 +- sc/source/core/tool/interpr4.cxx | 14 +++ 10 files changed, 270 insertions(+), 47 deletions(-) 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;