From 6079bf6879c4baf144b68ef459d825412ecea697 Mon Sep 17 00:00:00 2001 From: Eike Rathke Date: Fri, 3 Jun 2022 15:57:12 +0200 Subject: [PATCH] Add binary operators to ForceArrayReturn handling, tdf#149378 follow-up So ={1;2} or =-{1;2} or ={1;2}+3 or ={1;2}+{3;4} or ={1;2}+A1 are propagated. But ={1;2}+A1:A2 is not because the range reference should be implicit intersection not to be forced to array mode unless user says so. This also adds low level ocPush with svMatrix returning always ParamClass::ForceArrayReturn in FormulaToken::GetInForceArray() so any derived like ScMatrixToken inherit that. Change-Id: Ida24414a795d6609bf01e361f96438f9e7f7f66c Reviewed-on: https://gerrit.libreoffice.org/c/core/+/135360 Reviewed-by: Eike Rathke Tested-by: Jenkins --- formula/source/core/api/FormulaCompiler.cxx | 66 +++++++++++++++++---- formula/source/core/api/token.cxx | 2 +- 2 files changed, 55 insertions(+), 13 deletions(-) diff --git a/formula/source/core/api/FormulaCompiler.cxx b/formula/source/core/api/FormulaCompiler.cxx index e74284ae89f2..4b6495826d48 100644 --- a/formula/source/core/api/FormulaCompiler.cxx +++ b/formula/source/core/api/FormulaCompiler.cxx @@ -2783,19 +2783,25 @@ formula::ParamClass FormulaCompiler::GetForceArrayParameter( const FormulaToken* void FormulaCompiler::ForceArrayOperator( FormulaTokenRef const & rCurr ) { - if (rCurr->GetInForceArray() != ParamClass::Unknown) - // Already set, unnecessary to evaluate again. This happens by calls to - // CurrentFactor::operator=() while descending through Factor() and - // then ascending back (and down and up, ...), - // CheckSetForceArrayParameter() and later PutCode(). + if (pCurrentFactorToken.get() == rCurr.get()) return; const OpCode eOp = rCurr->GetOpCode(); const StackVar eType = rCurr->GetType(); - bool bInlineArray = false; - if (!(eOp != ocPush && (eType == svByte || eType == svJump)) - && !(bInlineArray = (eOp == ocPush && eType == svMatrix))) - return; + const bool bInlineArray = (eOp == ocPush && eType == svMatrix); + + if (!bInlineArray) + { + if (rCurr->GetInForceArray() != ParamClass::Unknown) + // Already set, unnecessary to evaluate again. This happens by calls to + // CurrentFactor::operator=() while descending through Factor() and + // then ascending back (and down and up, ...), + // CheckSetForceArrayParameter() and later PutCode(). + return; + + if (!(eOp != ocPush && (eType == svByte || eType == svJump))) + return; + } // Return class for inline arrays and functions returning array/matrix. // It's somewhat unclear what Excel actually does there and in @@ -2815,7 +2821,8 @@ void FormulaCompiler::ForceArrayOperator( FormulaTokenRef const & rCurr ) if (bInlineArray) { - // rCurr->SetInForceArray() can not be used with ocPush. + // rCurr->SetInForceArray() can not be used with ocPush, but ocPush + // with svMatrix has an implicit ParamClass::ForceArrayReturn. if (nCurrentFactorParam > 0 && pCurrentFactorToken && pCurrentFactorToken->GetInForceArray() == ParamClass::Unknown && GetForceArrayParameter( pCurrentFactorToken.get(), static_cast(nCurrentFactorParam - 1)) @@ -2828,15 +2835,50 @@ void FormulaCompiler::ForceArrayOperator( FormulaTokenRef const & rCurr ) return; } - if (!pCurrentFactorToken || (pCurrentFactorToken.get() == rCurr.get())) + if (!pCurrentFactorToken) { - if (!pCurrentFactorToken && mbMatrixFlag) + if (mbMatrixFlag) { // An array/matrix formula acts as ForceArray on all top level // operators and function calls, so that can be inherited properly // below. rCurr->SetInForceArray( ParamClass::ForceArray); } + else if (pc >= 2 && SC_OPCODE_START_BIN_OP <= eOp && eOp < SC_OPCODE_STOP_BIN_OP) + { + // Binary operators are not functions followed by arguments + // and need some peeking into RPN to inspect their operands. + // Note that array context is not forced if only one + // of the operands is an array like "={1;2}+A1:A2" returns #VALUE! + // if entered in column A and not input in array mode, because it + // involves a range reference with an implicit intersection. Check + // both arguments are arrays, or the other is ocPush without ranges + // for "={1;2}+3" or "={1;2}+A1". + // Note this does not catch "={1;2}+ABS(A1)" that could be forced + // to array, user still has to close in array mode. + // The IsMatrixFunction() is only necessary because not all + // functions returning matrix have ForceArrayReturn (yet?), see + // OOXML comment above. + + const OpCode eOp1 = pCode[-1]->GetOpCode(); + const OpCode eOp2 = pCode[-2]->GetOpCode(); + const bool b1 = (pCode[-1]->GetInForceArray() != ParamClass::Unknown || IsMatrixFunction(eOp1)); + const bool b2 = (pCode[-2]->GetInForceArray() != ParamClass::Unknown || IsMatrixFunction(eOp2)); + if ((b1 && b2) + || (b1 && eOp2 == ocPush && pCode[-2]->GetType() != svDoubleRef) + || (b2 && eOp1 == ocPush && pCode[-1]->GetType() != svDoubleRef)) + { + rCurr->SetInForceArray( eArrayReturn); + } + } + else if (pc >= 1 && SC_OPCODE_START_UN_OP <= eOp && eOp < SC_OPCODE_STOP_UN_OP) + { + // Similar for unary operators. + if (pCode[-1]->GetInForceArray() != ParamClass::Unknown || IsMatrixFunction(pCode[-1]->GetOpCode())) + { + rCurr->SetInForceArray( eArrayReturn); + } + } return; } diff --git a/formula/source/core/api/token.cxx b/formula/source/core/api/token.cxx index 87b0fb67ca98..2ca7af78f963 100644 --- a/formula/source/core/api/token.cxx +++ b/formula/source/core/api/token.cxx @@ -175,7 +175,7 @@ void FormulaToken::SetByte( sal_uInt8 ) ParamClass FormulaToken::GetInForceArray() const { // ok to be called for any derived class - return ParamClass::Unknown; + return (eOp == ocPush && eType == svMatrix) ? ParamClass::ForceArrayReturn : ParamClass::Unknown; } void FormulaToken::SetInForceArray( ParamClass )