commit 5760c94b8847164f9a7a181f031c7c86643944af
author Mike Kaganski <mike.kaganski@collabora.com> Fri Jul 10 19:06:10 2020 +0300
committer Mike Kaganski <mike.kaganski@collabora.com> Fri Jul 10 20:05:25 2020 +0200
tree eec33567fa521f17a8431508791b2375063ccefe
parent 2490559b6cddb0815808055dc7f32c733a48c44b

tdf#134576: proper handling of For/For Each with On Error Resume Next

1. In SbiRuntime::PushForEach, always initialize SbiForStack::eForType
to avoid handling the loop as simple For loop in SbiRuntime::StepNEXT.

2. In SbiRuntime::PushForEach, just like in SbiRuntime::PushFor, don't
set error, but imitialize the loop properly. This allows to proceed to
SbiRuntime::StepTESTFOR, where the error will be handled gracefully.

3. In SbiRuntime::StepTESTFOR, check for the proper initialization of
the iteration, and set the error.

4. In SbiRuntime::StepTESTFOR, on error, modify the type of iteration
into Error - to make sure that loop ends on next iteration (so only a
single iteration happens inside the Resume Next).

Change-Id: Idd0bab79fb0099fa3ad295b7fc6ab2eb6173f7b8
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/98537
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>

basic/source/inc/runtime.hxx

diff --git a/basic/source/inc/runtime.hxx b/basic/source/inc/runtime.hxx
index 3b59a5b1..65be051 100644
--- a/basic/source/inc/runtime.hxx
+++ b/basic/source/inc/runtime.hxx
@@ -55,7 +55,8 @@ enum class ForType {
    To,
    EachArray,
    EachCollection,
    EachXEnumeration
    EachXEnumeration,
    Error,
};

struct SbiForStack {                // for/next stack:

basic/source/runtime/runtime.cxx

diff --git a/basic/source/runtime/runtime.cxx b/basic/source/runtime/runtime.cxx
index 0c9a1736..9ac23bd 100644
--- a/basic/source/runtime/runtime.cxx
+++ b/basic/source/runtime/runtime.cxx
@@ -1138,21 +1138,16 @@ void SbiRuntime::PushFor()
void SbiRuntime::PushForEach()
{
    SbiForStack* p = new SbiForStack;
    // Set default value in case of error which is ignored in Resume Next
    p->eForType = ForType::EachArray;
    p->pNext = pForStk;
    pForStk = p;

    SbxVariableRef xObjVar = PopVar();
    SbxBase* pObj = xObjVar.is() ? xObjVar->GetObject() : nullptr;
    if( pObj == nullptr )
    {
        Error( ERRCODE_BASIC_NO_OBJECT );
        return;
    }
    SbxBase* pObj = xObjVar.is() && xObjVar->IsObject() ? xObjVar->GetObject() : nullptr;

    bool bError_ = false;
    if (SbxDimArray* pArray = dynamic_cast<SbxDimArray*>(pObj))
    {
        p->eForType = ForType::EachArray;
        p->refEnd = reinterpret_cast<SbxVariable*>(pArray);

        sal_Int32 nDims = pArray->GetDims32();
@@ -1196,25 +1191,7 @@ void SbiRuntime::PushForEach()
                catch(const uno::Exception& )
                {}
            }
            if ( !p->xEnumeration.is() )
            {
                bError_ = true;
            }
        }
        else
        {
            bError_ = true;
        }
    }
    else
    {
        bError_ = true;
    }

    if( bError_ )
    {
        Error( ERRCODE_BASIC_CONVERSION );
        return;
    }

    // Container variable
@@ -3045,12 +3022,19 @@ void SbiRuntime::StepTESTFOR( sal_uInt32 nOp1 )
            SbxOperator eOp = ( pForStk->refInc->GetDouble() < 0 ) ? SbxLT : SbxGT;
            if( pForStk->refVar->Compare( eOp, *pForStk->refEnd ) )
                bEndLoop = true;
            if (SbxBase::IsError())
                pForStk->eForType = ForType::Error; // terminate loop at the next iteration
            break;
        }
        case ForType::EachArray:
        {
            SbiForStack* p = pForStk;
            if( p->pArrayCurIndices == nullptr )
            if (!p->refEnd)
            {
                SbxBase::SetError(ERRCODE_BASIC_CONVERSION);
                pForStk->eForType = ForType::Error; // terminate loop at the next iteration
            }
            else if (p->pArrayCurIndices == nullptr)
            {
                bEndLoop = true;
            }
@@ -3089,6 +3073,13 @@ void SbiRuntime::StepTESTFOR( sal_uInt32 nOp1 )
        }
        case ForType::EachCollection:
        {
            if (!pForStk->refEnd)
            {
                SbxBase::SetError(ERRCODE_BASIC_CONVERSION);
                pForStk->eForType = ForType::Error; // terminate loop at the next iteration
                break;
            }

            BasicCollection* pCollection = static_cast<BasicCollection*>(pForStk->refEnd.get());
            SbxArrayRef xItemArray = pCollection->xItemArray;
            sal_Int32 nCount = xItemArray->Count32();
@@ -3107,7 +3098,12 @@ void SbiRuntime::StepTESTFOR( sal_uInt32 nOp1 )
        case ForType::EachXEnumeration:
        {
            SbiForStack* p = pForStk;
            if( p->xEnumeration->hasMoreElements() )
            if (!p->xEnumeration)
            {
                SbxBase::SetError(ERRCODE_BASIC_CONVERSION);
                pForStk->eForType = ForType::Error; // terminate loop at the next iteration
            }
            else if (p->xEnumeration->hasMoreElements())
            {
                Any aElem = p->xEnumeration->nextElement();
                SbxVariableRef xVar = new SbxVariable( SbxVARIANT );
@@ -3120,6 +3116,12 @@ void SbiRuntime::StepTESTFOR( sal_uInt32 nOp1 )
            }
            break;
        }
        case ForType::Error:
        {
            // We are in Resume Next mode, and we already had one iteration
            bEndLoop = true;
            break;
        }
    }
    if( bEndLoop )
    {