formula/source/core/api/FormulaCompiler.cxx | 54 ++++++++++++++++++++++++++-- include/formula/FormulaCompiler.hxx | 29 ++++++++++----- sc/inc/compiler.hxx | 2 + sc/qa/unit/ucalc_formula.cxx | 8 ++++ sc/source/core/inc/interpre.hxx | 4 +- sc/source/core/tool/compiler.cxx | 8 ++++ sc/source/core/tool/interpr4.cxx | 2 - sc/source/core/tool/token.cxx | 2 - 8 files changed, 94 insertions(+), 15 deletions(-)
New commits: commit 311b5b69f032134457184255920ff1083c9afed2 Author: Eike Rathke <[email protected]> Date: Mon Nov 16 13:19:20 2015 +0100 unit test for tdf#95670 Change-Id: I5874a7fea97311b0e69dbeae8923517a08b63c9a diff --git a/sc/qa/unit/ucalc_formula.cxx b/sc/qa/unit/ucalc_formula.cxx index 7a0158e..ffcf834 100644 --- a/sc/qa/unit/ucalc_formula.cxx +++ b/sc/qa/unit/ucalc_formula.cxx @@ -3431,6 +3431,14 @@ void Test::testFuncSUMPRODUCT() sal_uInt16 nError = m_pDoc->GetErrCode(aPos); CPPUNIT_ASSERT_MESSAGE("Formula result should be a propagated error", nError); + // Test ForceArray propagation of SUMPRODUCT parameters to ABS and + operator. + // => ABS({-3,4})*({-3,4}+{-3,4}) => {3,4}*{-6,8} => {-18,32} => 14 + m_pDoc->SetValue(ScAddress(4,0,0), -3.0); // E1 + m_pDoc->SetValue(ScAddress(4,1,0), 4.0); // E2 + // Non-intersecting formula in F3. + m_pDoc->SetString(ScAddress(5,2,0), "=SUMPRODUCT(ABS(E1:E2);E1:E2+E1:E2)"); + CPPUNIT_ASSERT_EQUAL(14.0, m_pDoc->GetValue(ScAddress(5,2,0))); + m_pDoc->DeleteTab(0); } commit 49257e1da7e371fdea0fac080116b0511789cac7 Author: Eike Rathke <[email protected]> Date: Mon Nov 16 12:23:15 2015 +0100 Resolves: tdf#95670 propagate ForceArray per parameter Regression of b5cd11b4b02a85a83db77ba9d8d1763f0cd88cb1 It was always wrong to propagate ForceArray already if a function had a ForceArray parameter *somewhere*, we need to do this per parameter instead. Change-Id: If188d45366279d9a7bf641edc7e4dd7095d6d035 diff --git a/formula/source/core/api/FormulaCompiler.cxx b/formula/source/core/api/FormulaCompiler.cxx index e32a6f9..00988f65 100644 --- a/formula/source/core/api/FormulaCompiler.cxx +++ b/formula/source/core/api/FormulaCompiler.cxx @@ -532,6 +532,7 @@ void FormulaCompiler::OpCodeMap::putOpCode( const OUString & rStr, const OpCode FormulaCompiler::FormulaCompiler( FormulaTokenArray& rArr ) : + nCurrentFactorParam(0), pArr( &rArr ), pCode( nullptr ), pStack( nullptr ), @@ -550,6 +551,7 @@ FormulaCompiler::FormulaCompiler( FormulaTokenArray& rArr ) FormulaCompiler::FormulaCompiler() : + nCurrentFactorParam(0), pArr( nullptr ), pCode( nullptr ), pStack( nullptr ), @@ -1169,6 +1171,7 @@ void FormulaCompiler::Factor() { // range list (A1;A2) converted to (A1~A2) pFacToken = mpToken; NextToken(); + CheckSetForceArrayParameter( mpToken, 0); eOp = Expression(); // Do not ignore error here, regardless of bIgnoreErrors, otherwise // errors like =(1;) would also result in display of =(1~) @@ -1260,7 +1263,10 @@ void FormulaCompiler::Factor() if (eOp == ocClose) bNoParam = true; else + { + CheckSetForceArrayParameter( mpToken, 0); eOp = Expression(); + } } else SetError( errPairExpected); @@ -1270,8 +1276,9 @@ void FormulaCompiler::Factor() nSepCount++; while ((eOp == ocSep) && (!pArr->GetCodeError() || !mbStopOnError)) { - nSepCount++; NextToken(); + CheckSetForceArrayParameter( mpToken, nSepCount); + nSepCount++; eOp = Expression(); } } @@ -1296,6 +1303,7 @@ void FormulaCompiler::Factor() if (eOp == ocOpen) { NextToken(); + CheckSetForceArrayParameter( mpToken, 0); eOp = Expression(); } else @@ -1328,7 +1336,10 @@ void FormulaCompiler::Factor() if (eOp == ocClose) bNoParam = true; else + { + CheckSetForceArrayParameter( mpToken, 0); eOp = Expression(); + } } else if (eMyLastOp == ocBad) { @@ -1345,8 +1356,9 @@ void FormulaCompiler::Factor() nSepCount++; while ((eOp == ocSep) && (!pArr->GetCodeError() || !mbStopOnError)) { - nSepCount++; NextToken(); + CheckSetForceArrayParameter( mpToken, nSepCount); + nSepCount++; eOp = Expression(); } } @@ -1386,6 +1398,7 @@ void FormulaCompiler::Factor() if (eOp == ocOpen) { NextToken(); + CheckSetForceArrayParameter( mpToken, 0); eOp = Expression(); } else @@ -1420,6 +1433,7 @@ void FormulaCompiler::Factor() if ( ++nJumpCount <= nJumpMax ) pFacToken->GetJump()[nJumpCount] = pc-1; NextToken(); + CheckSetForceArrayParameter( mpToken, nJumpCount - 1); eOp = Expression(); // ocSep or ocClose terminate the subexpression PutCode( mpToken ); @@ -2179,7 +2193,7 @@ void FormulaCompiler::PutCode( FormulaTokenRef& p ) } if (pArr->GetCodeError() && mbJumpCommandReorder) return; - ForceArrayOperator( p, pCurrentFactorToken); + ForceArrayOperator( p); p->IncRef(); *pCode++ = p.get(); pc++; @@ -2235,6 +2249,40 @@ void FormulaCompiler::LocalizeString( OUString& /*rName*/ ) const { } +bool FormulaCompiler::IsForceArrayParameter( const FormulaToken* /*pToken*/, sal_uInt16 /*nParam*/ ) const +{ + return false; +} + +void FormulaCompiler::ForceArrayOperator( FormulaTokenRef& rCurr ) +{ + if (!pCurrentFactorToken) + return; + + if (!(rCurr->GetOpCode() != ocPush && (rCurr->GetType() == svByte || rCurr->GetType() == svJump))) + return; + + if (pCurrentFactorToken->HasForceArray()) + { + rCurr->SetForceArray( true); + return; + } + + if (nCurrentFactorParam && IsForceArrayParameter( pCurrentFactorToken.get(), + static_cast<sal_uInt8>(nCurrentFactorParam - 1))) + rCurr->SetForceArray( true); +} + +void FormulaCompiler::CheckSetForceArrayParameter( FormulaTokenRef& rCurr, sal_uInt8 nParam ) +{ + if (!pCurrentFactorToken) + return; + + nCurrentFactorParam = nParam + 1; + + ForceArrayOperator( rCurr); +} + void FormulaCompiler::PushTokenArray( FormulaTokenArray* pa, bool bTemp ) { if ( bAutoCorrect && !pStack ) diff --git a/include/formula/FormulaCompiler.hxx b/include/formula/FormulaCompiler.hxx index 8e5007e..00e76fa 100644 --- a/include/formula/FormulaCompiler.hxx +++ b/include/formula/FormulaCompiler.hxx @@ -293,6 +293,10 @@ protected: virtual void CreateStringFromIndex( OUStringBuffer& rBuffer, const FormulaToken* pToken ) const; virtual void LocalizeString( OUString& rName ) const; // modify rName - input: exact name + /** Whether parameter nParam (0-based) is forced to array for OpCode eOp. + Calc: ForceArray or ReferenceOrForceArray type. */ + virtual bool IsForceArrayParameter( const FormulaToken* pToken, sal_uInt16 nParam ) const; + void AppendErrorConstant( OUStringBuffer& rBuffer, sal_uInt16 nError ) const; bool GetToken(); @@ -323,6 +327,7 @@ protected: FormulaTokenRef mpToken; // current token FormulaTokenRef pCurrentFactorToken; // current factor token (of Factor() method) + sal_uInt16 nCurrentFactorParam; // current factor token's parameter, 1-based FormulaTokenArray* pArr; FormulaToken** pCode; @@ -353,32 +358,38 @@ private: void loadSymbols( sal_uInt16 nSymbols, FormulaGrammar::Grammar eGrammar, NonConstOpCodeMapPtr& rxMap, SeparatorType eSepType = SEMICOLON_BASE ) const; - static inline void ForceArrayOperator( FormulaTokenRef& rCurr, const FormulaTokenRef& rPrev ) - { - if ( rPrev && rPrev->HasForceArray() && rCurr->GetOpCode() != ocPush && - (rCurr->GetType() == svByte || rCurr->GetType() == svJump) && - !rCurr->HasForceArray() ) - rCurr->SetForceArray( true); - } + /** Check pCurrentFactorToken for nParam's (0-based) ForceArray types and + set ForceArray at rCurr if so. Set nParam+1 as 1-based + nCurrentFactorParam for subsequent ForceArrayOperator() calls. + */ + void CheckSetForceArrayParameter( FormulaTokenRef& rCurr, sal_uInt8 nParam ); + + void ForceArrayOperator( FormulaTokenRef& rCurr ); class CurrentFactor { FormulaTokenRef pPrevFac; + sal_uInt16 nPrevParam; FormulaCompiler* pCompiler; CurrentFactor( const CurrentFactor& ) = delete; CurrentFactor& operator=( const CurrentFactor& ) = delete; public: explicit CurrentFactor( FormulaCompiler* pComp ) : pPrevFac( pComp->pCurrentFactorToken ) + , nPrevParam( pComp->nCurrentFactorParam ) , pCompiler( pComp ) {} ~CurrentFactor() - { pCompiler->pCurrentFactorToken = pPrevFac; } + { + pCompiler->pCurrentFactorToken = pPrevFac; + pCompiler->nCurrentFactorParam = nPrevParam; + } // yes, this operator= may modify the RValue void operator=( FormulaTokenRef& r ) { - ForceArrayOperator( r, pPrevFac); + pCompiler->ForceArrayOperator( r ); pCompiler->pCurrentFactorToken = r; + pCompiler->nCurrentFactorParam = 0; } void operator=( FormulaToken* p ) { diff --git a/sc/inc/compiler.hxx b/sc/inc/compiler.hxx index ca33559..ffdd65a 100644 --- a/sc/inc/compiler.hxx +++ b/sc/inc/compiler.hxx @@ -475,6 +475,8 @@ private: virtual void CreateStringFromIndex( OUStringBuffer& rBuffer, const formula::FormulaToken* pToken ) const override; virtual void LocalizeString( OUString& rName ) const override; // modify rName - input: exact name + virtual bool IsForceArrayParameter( const formula::FormulaToken* pToken, sal_uInt16 nParam ) const; + /// Access the CharTable flags inline sal_uLong GetCharTableFlags( sal_Unicode c, sal_Unicode cLast ) { return c < 128 ? pConv->getCharTableFlags(c, cLast) : 0; } diff --git a/sc/source/core/inc/interpre.hxx b/sc/source/core/inc/interpre.hxx index 1343e75..c35248d 100644 --- a/sc/source/core/inc/interpre.hxx +++ b/sc/source/core/inc/interpre.hxx @@ -32,6 +32,7 @@ #include "calcconfig.hxx" #include "token.hxx" #include "math.hxx" +#include "parclass.hxx" #include <map> #include <memory> @@ -918,7 +919,8 @@ inline void ScInterpreter::MatrixDoubleRefToMatrix() inline bool ScInterpreter::MatrixParameterConversion() { - if ( (bMatrixFormula || pCur->HasForceArray()) && !pJumpMatrix && sp > 0 ) + if ( (bMatrixFormula || pCur->HasForceArray() || ScParameterClassification::HasForceArray( pCur->GetOpCode())) && + !pJumpMatrix && sp > 0 ) return ConvertMatrixParameters(); return false; } diff --git a/sc/source/core/tool/compiler.cxx b/sc/source/core/tool/compiler.cxx index 59c259e..05cebc1 100644 --- a/sc/source/core/tool/compiler.cxx +++ b/sc/source/core/tool/compiler.cxx @@ -5416,4 +5416,12 @@ bool ScCompiler::HandleTableRef() return true; } +bool ScCompiler::IsForceArrayParameter( const formula::FormulaToken* pToken, sal_uInt16 nParam ) const +{ + ScParameterClassification::Type eType = ScParameterClassification::GetParameterType( pToken, nParam); + return + eType == ScParameterClassification::ForceArray || + eType == ScParameterClassification::ReferenceOrForceArray; +} + /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sc/source/core/tool/interpr4.cxx b/sc/source/core/tool/interpr4.cxx index 0a8a745..a5f5982 100644 --- a/sc/source/core/tool/interpr4.cxx +++ b/sc/source/core/tool/interpr4.cxx @@ -1400,7 +1400,7 @@ bool ScInterpreter::ConvertMatrixParameters() // has ForceArray or ReferenceOrForceArray // parameter *somewhere else*) pick a normal // position dependent implicit intersection later. - (eType != ScParameterClassification::Value || bMatrixFormula)) + (eType != ScParameterClassification::Value || bMatrixFormula || pCur->HasForceArray())) { SCCOL nCol1, nCol2; SCROW nRow1, nRow2; diff --git a/sc/source/core/tool/token.cxx b/sc/source/core/tool/token.cxx index cfdd5a3..ea66456 100644 --- a/sc/source/core/tool/token.cxx +++ b/sc/source/core/tool/token.cxx @@ -252,7 +252,7 @@ void ScRawToken::SetOpCode( OpCode e ) default: eType = svByte; sbyte.cByte = 0; - sbyte.bHasForceArray = ScParameterClassification::HasForceArray( eOp); + sbyte.bHasForceArray = false; } } _______________________________________________ Libreoffice-commits mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
