This is an automated email from the ASF dual-hosted git repository. jimjag pushed a commit to branch openformula in repository https://gitbox.apache.org/repos/asf/openoffice.git
commit 4a7f62e935a6f0660630bc262242bbcda36c50a9 Author: Jim Jagielski <[email protected]> AuthorDate: Fri Nov 13 08:45:51 2026 -0500 sc: make IFNA a jump opcode and fix its function list entry IFNA compiled to just a plain function token, so the compiler wrote its jumps through a null pointer as soon as a formula used it. Ugg. Not good. --- main/formula/source/core/api/FormulaCompiler.cxx | 47 ++--------- main/formula/source/core/api/token.cxx | 7 +- main/sc/inc/helpids.h | 2 + main/sc/source/core/tool/interpr1.cxx | 99 ++++++++---------------- main/sc/source/core/tool/interpr4.cxx | 5 +- main/sc/source/core/tool/token.cxx | 4 + main/sc/source/ui/src/scfuncs.src | 4 +- main/sc/util/hidother.src | 2 + 8 files changed, 58 insertions(+), 112 deletions(-) diff --git a/main/formula/source/core/api/FormulaCompiler.cxx b/main/formula/source/core/api/FormulaCompiler.cxx index 79c1df2486..df8ac404e5 100644 --- a/main/formula/source/core/api/FormulaCompiler.cxx +++ b/main/formula/source/core/api/FormulaCompiler.cxx @@ -1011,43 +1011,8 @@ void FormulaCompiler::Factor() PutCode( pFacToken ); eOp = NextToken(); } - else if (eOp == ocIfna) - { - // IFNA(value; value_if_na): lazy evaluation of the second argument. - // jump[0] = 2 (value path, fallback path, behind) - pFacToken = pToken; - pFacToken->GetJump()[ 0 ] = 2; - eOp = NextToken(); - if (eOp == ocOpen) - { - NextToken(); - eOp = Expression(); - } - else - SetError(errPairExpected); - short nJumpCount = 0; - PutCode( pFacToken ); - if ( ++nJumpCount <= 2 ) - pFacToken->GetJump()[nJumpCount] = pc-1; // fallback starts here - NextToken(); - if ( pToken->GetOpCode() == ocSep ) - { - NextToken(); - eOp = Expression(); - } - else - SetError(errPairExpected); - if (eOp != ocClose) - SetError(errPairExpected); - else - { - eOp = NextToken(); - if ( ++nJumpCount <= 2 ) - pFacToken->GetJump()[ nJumpCount ] = pc-1; // behind - pFacToken->GetJump()[ 0 ] = nJumpCount; - } - } - else if ((SC_OPCODE_START_2_PAR <= eOp && eOp < SC_OPCODE_STOP_2_PAR) + else if ((SC_OPCODE_START_2_PAR <= eOp && eOp < SC_OPCODE_STOP_2_PAR + && (eOp != ocIfna || bCompileForFAP)) || eOp == ocExternal || eOp == ocMacro || eOp == ocAnd @@ -1106,12 +1071,14 @@ void FormulaCompiler::Factor() pArr->SetRecalcModeAlways(); PutCode( pFacToken ); } - else if (eOp == ocIf || eOp == ocChose) + else if (eOp == ocIf || eOp == ocChose || eOp == ocIfna) { // the PC counters are -1 pFacToken = pToken; if ( eOp == ocIf ) pFacToken->GetJump()[ 0 ] = 3; // if, else, behind + else if ( eOp == ocIfna ) + pFacToken->GetJump()[ 0 ] = 2; // fallback, behind else pFacToken->GetJump()[ 0 ] = MAXJUMPCOUNT+1; eOp = NextToken(); @@ -1129,7 +1096,8 @@ void FormulaCompiler::Factor() // ScRawToken::Clone() allocates the JumpBuffer according to // nJump[0]*2+2, which is 3*2+2 on ocIf. const short nJumpMax = - (pFacToken->GetOpCode() == ocIf ? 3 : MAXJUMPCOUNT); + (pFacToken->GetOpCode() == ocIf ? 3 : + (pFacToken->GetOpCode() == ocIfna ? 2 : MAXJUMPCOUNT)); while ( (nJumpCount < (MAXJUMPCOUNT - 1)) && (eOp == ocSep) && (!pArr->GetCodeError() || bIgnoreErrors) ) { @@ -1156,6 +1124,7 @@ void FormulaCompiler::Factor() if ( ++nJumpCount <= nJumpMax ) pFacToken->GetJump()[ nJumpCount ] = pc-1; if ((pFacToken->GetOpCode() == ocIf && (nJumpCount > 3)) || + (pFacToken->GetOpCode() == ocIfna && (nJumpCount != 2)) || (nJumpCount >= MAXJUMPCOUNT)) SetError(errIllegalParameter); else diff --git a/main/formula/source/core/api/token.cxx b/main/formula/source/core/api/token.cxx index 2be3942015..fee11f9f25 100644 --- a/main/formula/source/core/api/token.cxx +++ b/main/formula/source/core/api/token.cxx @@ -124,7 +124,7 @@ sal_uInt8 FormulaToken::GetParamCount() const return 0; // no parameter else if (SC_OPCODE_START_1_PAR <= eOp && eOp < SC_OPCODE_STOP_1_PAR) return 1; // one parameter - else if ( eOp == ocIf || eOp == ocChose ) + else if ( eOp == ocIf || eOp == ocChose || eOp == ocIfna ) return 1; // only the condition counts as parameter else return 0; // all the rest, no Parameter, or @@ -842,7 +842,7 @@ sal_Bool FormulaTokenArray::HasMatrixDoubleRefOps() } if ( eOp == ocPush || lcl_IsReference( eOp, t->GetType() ) ) pStack[sp++] = t; - else if ( eOp == ocIf || eOp == ocChose ) + else if ( eOp == ocIf || eOp == ocChose || eOp == ocIfna ) { // Jumps ignorieren, vorheriges Result (Condition) poppen if ( sp ) --sp; @@ -1194,9 +1194,10 @@ FormulaToken* FormulaTokenArray::AddOpCode( OpCode eOp ) break; case ocIf: case ocChose: + case ocIfna: { short nJump[MAXJUMPCOUNT + 1]; - nJump[ 0 ] = ocIf == eOp ? 3 : MAXJUMPCOUNT+1; + nJump[ 0 ] = ocIf == eOp ? 3 : (ocIfna == eOp ? 2 : MAXJUMPCOUNT+1); pRet = new FormulaJumpToken( eOp, (short*)nJump ); } break; diff --git a/main/sc/inc/helpids.h b/main/sc/inc/helpids.h index b204e3f7c2..db6713b8ac 100644 --- a/main/sc/inc/helpids.h +++ b/main/sc/inc/helpids.h @@ -779,3 +779,5 @@ #define HID_FUNC_BITXOR "SC_HID_FUNC_BITXOR" #define HID_FUNC_BITLSHIFT "SC_HID_FUNC_BITLSHIFT" #define HID_FUNC_BITRSHIFT "SC_HID_FUNC_BITRSHIFT" +#define HID_FUNC_IFNA "SC_HID_FUNC_IFNA" +#define HID_FUNC_SKEWP "SC_HID_FUNC_SKEWP" diff --git a/main/sc/source/core/tool/interpr1.cxx b/main/sc/source/core/tool/interpr1.cxx index de0fb4508d..64eb45895f 100644 --- a/main/sc/source/core/tool/interpr1.cxx +++ b/main/sc/source/core/tool/interpr1.cxx @@ -325,84 +325,51 @@ void ScInterpreter::ScIfna() { RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "sc", "er", "ScInterpreter::ScIfna" ); const short* pJump = pCur->GetJump(); + bool bIsNA = false; + MatrixDoubleRefToMatrix(); + // the value path keeps the format of the value + nFuncFmtType = nCurFmtType; + nFuncFmtIndex = nCurFmtIndex; switch ( GetStackType() ) { - case formula::svError: - { - // Direct error value on stack (e.g. NA() or 1/0). - // Peek without popping: if #N/A, discard and jump to fallback; - // otherwise leave on stack and jump to end (propagate error). - if ( pStack[sp-1]->GetError() == NOTAVAILABLE ) - { - --sp; // discard #N/A without touching nGlobalError - aCode.Jump( pJump[ 1 ], pJump[ 2 ] ); - } - else - { - aCode.Jump( pJump[ 2 ], pJump[ 2 ] ); - } - } - break; - - case formula::svSingleRef : - case formula::svDoubleRef : + case svSingleRef : + case svDoubleRef : { - // Reference: resolve to a single cell (implicit intersection for - // ranges) and check whether the cell contains #N/A. ScAddress aAdr; - if ( !PopDoubleRefOrSingleRef( aAdr ) ) - { - // Reference resolution failed: propagate error, jump to end. - aCode.Jump( pJump[ 2 ], pJump[ 2 ] ); - } + PopDoubleRefOrSingleRef( aAdr ); + if ( nGlobalError == NOTAVAILABLE ) + bIsNA = true; else if ( nGlobalError ) - { - // Error resolving reference: propagate, jump to end. - aCode.Jump( pJump[ 2 ], pJump[ 2 ] ); - } + PushError( nGlobalError ); + else if ( GetCellErrCode( GetCell( aAdr ) ) == NOTAVAILABLE ) + bIsNA = true; else - { - ScBaseCell* pCell = GetCell( aAdr ); - sal_uInt16 nErr = GetCellErrCode( pCell ); - if ( nErr == NOTAVAILABLE ) - { - // Cell contains #N/A: jump to fallback. - aCode.Jump( pJump[ 1 ], pJump[ 2 ] ); - } - else if ( nErr ) - { - // Cell contains another error: push it, jump to end. - PushError( nErr ); - aCode.Jump( pJump[ 2 ], pJump[ 2 ] ); - } - else - { - // Cell has a value: push it, jump to end. - PushCellResultToken( true, aAdr, NULL, NULL ); - aCode.Jump( pJump[ 2 ], pJump[ 2 ] ); - } - } + PushCellResultToken( false, aAdr, &nFuncFmtType, &nFuncFmtIndex ); } break; - - case formula::svRefList : - case formula::svMatrix: - { - // Reference list or matrix input: IFNA is not an array-forcing - // function. Return the value as-is; per-element #N/A - // substitution is not implemented. This is a documented - // limitation (see openformula-milestone-b-report.md). - aCode.Jump( pJump[ 2 ], pJump[ 2 ] ); - } + case svRefList : + case svMatrix : + // No per-element #N/A substitution, the value passes through. break; - default: - { - // Regular value (double, string, logical, etc.): not #N/A. - aCode.Jump( pJump[ 2 ], pJump[ 2 ] ); - } + if ( nGlobalError == NOTAVAILABLE || (GetStackType() == svError && + pStack[ sp - 1 ]->GetError() == NOTAVAILABLE) ) + { + Pop(); + bIsNA = true; + } } + + if ( bIsNA ) + { + nGlobalError = 0; + nFuncFmtType = NUMBERFORMAT_NUMBER; + nFuncFmtIndex = 0; + aCode.Jump( pJump[ 1 ], pJump[ 2 ] ); + } + else + aCode.Jump( pJump[ 2 ], pJump[ 2 ] ); } void lcl_AdjustJumpMatrix( ScJumpMatrix* pJumpM, ScMatrixRef& pResMat, SCSIZE nParmCols, SCSIZE nParmRows ) diff --git a/main/sc/source/core/tool/interpr4.cxx b/main/sc/source/core/tool/interpr4.cxx index c988b71d9f..bf5a6ddc77 100644 --- a/main/sc/source/core/tool/interpr4.cxx +++ b/main/sc/source/core/tool/interpr4.cxx @@ -3398,7 +3398,7 @@ StackVar ScInterpreter::Interpret() // RPN code push without error PushWithoutError( (FormulaToken&) *pCur ); } - else if (pTokenMatrixMap && !(eOp == ocIf || eOp == ocChose) && + else if (pTokenMatrixMap && !(eOp == ocIf || eOp == ocChose || eOp == ocIfna) && ((aTokenMatrixMapIter = pTokenMatrixMap->find( pCur)) != pTokenMatrixMap->end()) && (*aTokenMatrixMapIter).second->GetType() != svJumpMatrix) @@ -3419,7 +3419,7 @@ StackVar ScInterpreter::Interpret() nFuncFmtType = NUMBERFORMAT_NUMBER; nFuncFmtIndex = 0; - if ( eOp == ocIf || eOp == ocChose ) + if ( eOp == ocIf || eOp == ocChose || eOp == ocIfna ) nStackBase = sp; // don't mess around with the jumps else { @@ -3854,6 +3854,7 @@ StackVar ScInterpreter::Interpret() case ocIsEmpty : \ case ocIsErr : \ case ocIsError : \ + case ocIfna : \ case ocIsFormula : \ case ocIsLogical : \ case ocIsNA : \ diff --git a/main/sc/source/core/tool/token.cxx b/main/sc/source/core/tool/token.cxx index 40073e5794..9dda7fd408 100644 --- a/main/sc/source/core/tool/token.cxx +++ b/main/sc/source/core/tool/token.cxx @@ -160,6 +160,10 @@ void ScRawToken::SetOpCode( OpCode e ) eType = svJump; nJump[ 0 ] = MAXJUMPCOUNT+1; break; + case ocIfna: + eType = svJump; + nJump[ 0 ] = 2; // Fallback, Behind + break; case ocMissing: eType = svMissing; break; diff --git a/main/sc/source/ui/src/scfuncs.src b/main/sc/source/ui/src/scfuncs.src index a45dfba861..ec6ff81ad7 100644 --- a/main/sc/source/ui/src/scfuncs.src +++ b/main/sc/source/ui/src/scfuncs.src @@ -1034,9 +1034,9 @@ Resource RID_SC_FUNCTION_DESCRIPTIONS1 ExtraData = { 0; - ID_FUNCTION_GRP_LOGICAL; + ID_FUNCTION_GRP_LOGIC; U2S( HID_FUNC_IFNA ); - 2; 2; 0; + 2; 0; 0; 0; }; String 2 // Name of Parameter 1 diff --git a/main/sc/util/hidother.src b/main/sc/util/hidother.src index 866acc9b0b..86014604a7 100644 --- a/main/sc/util/hidother.src +++ b/main/sc/util/hidother.src @@ -491,3 +491,5 @@ hidspecial HID_FUNC_BITOR { HelpID = HID_FUNC_BITOR; }; hidspecial HID_FUNC_BITXOR { HelpID = HID_FUNC_BITXOR; }; hidspecial HID_FUNC_BITLSHIFT { HelpID = HID_FUNC_BITLSHIFT; }; hidspecial HID_FUNC_BITRSHIFT { HelpID = HID_FUNC_BITRSHIFT; }; +hidspecial HID_FUNC_IFNA { HelpID = HID_FUNC_IFNA; }; +hidspecial HID_FUNC_SKEWP { HelpID = HID_FUNC_SKEWP; };
