[Libreoffice-commits] core.git: basic/source

[Mike Kaganski (via logerrit)]

 basic/source/runtime/methods.cxx |  127 +++++++++++++++++++--------------------
 1 file changed, 65 insertions(+), 62 deletions(-)

New commits:
commit dd06a8a2caf93707fbf29c7264cf41f8ec9d4c6d
Author:     Mike Kaganski <mike.kaganski at collabora.com>
AuthorDate: Sat Apr 25 00:49:07 2020 +0300
Commit:     Mike Kaganski <mike.kaganski at collabora.com>
CommitDate: Sat Apr 25 08:00:41 2020 +0200

    tdf#132388: reimplement SbRtl_Replace
    
    It should not convert strings to uppercase each loop; it should use
    OUStringBuffer to avoid extra allocations.
    
    This reduces tick count for the code in the bug from ~6000 to ~30.
    
    Change-Id: I89ea062fc6d012464bb461b6a8ef321f8cc62fe6
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/92884
    Tested-by: Jenkins
    Reviewed-by: Mike Kaganski <mike.kaganski at collabora.com>

diff --git a/basic/source/runtime/methods.cxx b/basic/source/runtime/methods.cxx
index df177d334166..75b7b9177b42 100644
--- a/basic/source/runtime/methods.cxx
+++ b/basic/source/runtime/methods.cxx
@@ -1219,88 +1219,91 @@ void SbRtl_Replace(StarBASIC *, SbxArray & rPar, bool)
     if ( nArgCount < 3 || nArgCount > 6 )
     {
         StarBASIC::Error( ERRCODE_BASIC_BAD_ARGUMENT );
+        return;
     }
-    else
-    {
-        OUString aExpStr = rPar.Get32(1)->GetOUString();
-        OUString aFindStr = rPar.Get32(2)->GetOUString();
-        OUString aReplaceStr = rPar.Get32(3)->GetOUString();
 
-        sal_Int32 lStartPos = 1;
-        if ( nArgCount >= 4 )
+    sal_Int32 lStartPos = 1;
+    if (nArgCount >= 4)
+    {
+        if (rPar.Get32(4)->GetType() != SbxEMPTY)
         {
-            if( rPar.Get32(4)->GetType() != SbxEMPTY )
-            {
-                lStartPos = rPar.Get32(4)->GetLong();
-            }
-            if( lStartPos < 1)
-            {
-                StarBASIC::Error( ERRCODE_BASIC_BAD_ARGUMENT );
-                lStartPos = 1;
-            }
+            lStartPos = rPar.Get32(4)->GetLong();
         }
+        if (lStartPos < 1)
+        {
+            StarBASIC::Error(ERRCODE_BASIC_BAD_ARGUMENT);
+            return;
+        }
+    }
 
-        sal_Int32 lCount = -1;
-        if( nArgCount >=5 )
+    sal_Int32 lCount = -1;
+    if (nArgCount >= 5)
+    {
+        if (rPar.Get32(5)->GetType() != SbxEMPTY)
         {
-            if( rPar.Get32(5)->GetType() != SbxEMPTY )
-            {
-                lCount = rPar.Get32(5)->GetLong();
-            }
-            if( lCount < -1)
-            {
-                StarBASIC::Error( ERRCODE_BASIC_BAD_ARGUMENT );
-                lCount = -1;
-            }
+            lCount = rPar.Get32(5)->GetLong();
         }
+        if (lCount < -1)
+        {
+            StarBASIC::Error(ERRCODE_BASIC_BAD_ARGUMENT);
+            return;
+        }
+    }
 
+    bool bCaseInsensitive;
+    if (nArgCount == 6)
+    {
+        bCaseInsensitive = rPar.Get32(6)->GetInteger();
+    }
+    else
+    {
         SbiInstance* pInst = GetSbData()->pInst;
-        bool bTextMode;
-        bool bCompatibility = ( pInst && pInst->IsCompatibility() );
-        if( bCompatibility )
+        if (pInst && pInst->IsCompatibility())
         {
             SbiRuntime* pRT = pInst->pRun;
-            bTextMode = pRT && pRT->IsImageFlag( SbiImageFlags::COMPARETEXT );
+            bCaseInsensitive = pRT && pRT->IsImageFlag(SbiImageFlags::COMPARETEXT);
         }
         else
         {
-            bTextMode = true;
+            bCaseInsensitive = true;
         }
-        if ( nArgCount == 6 )
+    }
+
+    const OUString aExpStr = rPar.Get32(1)->GetOUString();
+    OUString aFindStr = rPar.Get32(2)->GetOUString();
+    const OUString aReplaceStr = rPar.Get32(3)->GetOUString();
+    const sal_Int32 nExpStrLen = aExpStr.getLength();
+    const sal_Int32 nFindStrLen = aFindStr.getLength();
+
+    OUString aSrcStr(aExpStr);
+    if (bCaseInsensitive)
+    {
+        // FIXME: case insensitivity should not be ASCII-only
+        aSrcStr = aSrcStr.toAsciiUpperCase();
+        aFindStr = aFindStr.toAsciiUpperCase();
+    }
+
+    // Note: the result starts from lStartPos, removing everything to the left. See i#94895.
+    sal_Int32 nPrevPos = std::min(lStartPos - 1, nExpStrLen);
+    OUStringBuffer sResult(nExpStrLen - nPrevPos);
+    sal_Int32 nCounts = 0;
+    while (lCount == -1 || lCount > nCounts)
+    {
+        sal_Int32 nPos = aSrcStr.indexOf(aFindStr, nPrevPos);
+        if (nPos >= 0)
         {
-            bTextMode = rPar.Get32(6)->GetInteger();
+            sResult.append(aExpStr.getStr() + nPrevPos, nPos - nPrevPos);
+            sResult.append(aReplaceStr);
+            nPrevPos = nPos + nFindStrLen;
+            nCounts++;
         }
-        sal_Int32 nExpStrLen = aExpStr.getLength();
-        sal_Int32 nFindStrLen = aFindStr.getLength();
-        sal_Int32 nReplaceStrLen = aReplaceStr.getLength();
-
-        if( lStartPos <= nExpStrLen )
+        else
         {
-            sal_Int32 nPos = lStartPos - 1;
-            sal_Int32 nCounts = 0;
-            while( lCount == -1 || lCount > nCounts )
-            {
-                OUString aSrcStr( aExpStr );
-                if( bTextMode )
-                {
-                    aSrcStr = aSrcStr.toAsciiUpperCase();
-                    aFindStr = aFindStr.toAsciiUpperCase();
-                }
-                nPos = aSrcStr.indexOf( aFindStr, nPos );
-                if( nPos >= 0 )
-                {
-                    aExpStr = aExpStr.replaceAt( nPos, nFindStrLen, aReplaceStr );
-                    nPos = nPos + nReplaceStrLen;
-                    nCounts++;
-                }
-                else
-                {
-                    break;
-                }
-            }
+            break;
         }
-        rPar.Get32(0)->PutString( aExpStr.copy( lStartPos - 1 )  );
     }
+    sResult.append(aExpStr.getStr() + nPrevPos, nExpStrLen - nPrevPos);
+    rPar.Get32(0)->PutString(sResult.makeStringAndClear());
 }
 
 void SbRtl_Right(StarBASIC *, SbxArray & rPar, bool)