[Libreoffice-commits] core.git: Branch 'libreoffice-6-3-4' - basic/source include/basic

Noel Grandin (via logerrit) logerrit at kemper.freedesktop.org
Thu Dec 5 09:51:27 UTC 2019


 basic/source/classes/sb.cxx      |    9 +++++----
 basic/source/classes/sbxmod.cxx  |   29 +++++++++++++++++------------
 basic/source/comp/codegen.cxx    |   10 +++++++---
 basic/source/runtime/runtime.cxx |    7 ++++---
 include/basic/sbmod.hxx          |    7 +++----
 5 files changed, 36 insertions(+), 26 deletions(-)

New commits:
commit eaf92eca088736d132bd9467423e8a251b8cfb54
Author:     Noel Grandin <noel.grandin at collabora.co.uk>
AuthorDate: Tue Dec 3 15:02:05 2019 +0200
Commit:     Caolán McNamara <caolanm at redhat.com>
CommitDate: Thu Dec 5 10:50:37 2019 +0100

    tdf#129107 objects in basic disappear
    
       Reverts part of "loplugin:useuniqueptr in SbModule"
       This reverts commit 263d7325691f4b0a1bda155f1c53bbcf712e9f09.
    
    because SbClassModuleObject is playing silly buggers with
    ownership by messing with fields in its SbModule superclass.
    
    Change-Id: I725332d080663e94b57f4bd4e1fb05aeeddf9038
    Reviewed-on: https://gerrit.libreoffice.org/84352
    Tested-by: Jenkins
    Reviewed-by: Noel Grandin <noel.grandin at collabora.co.uk>
    (cherry picked from commit 30c707666dbe810c577dc14bc995dc91c2293b17)
    Reviewed-on: https://gerrit.libreoffice.org/84430
    (cherry picked from commit 87d6a410e7676babc5475c2d7c05cde4d36d86c5)
    Reviewed-on: https://gerrit.libreoffice.org/84520
    Reviewed-by: Xisco Faulí <xiscofauli at libreoffice.org>
    Reviewed-by: Caolán McNamara <caolanm at redhat.com>
    Tested-by: Caolán McNamara <caolanm at redhat.com>

diff --git a/basic/source/classes/sb.cxx b/basic/source/classes/sb.cxx
index 8cce1979c6ec..9a43d0f62b7e 100644
--- a/basic/source/classes/sb.cxx
+++ b/basic/source/classes/sb.cxx
@@ -627,8 +627,9 @@ SbClassModuleObject::SbClassModuleObject( SbModule* pClassModule )
 {
     aOUSource = pClassModule->aOUSource;
     aComment = pClassModule->aComment;
-    pImage = std::move(pClassModule->pImage);
-    pBreaks = std::move(pClassModule->pBreaks);
+    // see comment in destructor about these two
+    pImage = pClassModule->pImage;
+    pBreaks = pClassModule->pBreaks;
 
     SetClassName( pClassModule->GetName() );
 
@@ -771,8 +772,8 @@ SbClassModuleObject::~SbClassModuleObject()
                 if( !pDocBasicItem->isDocClosed() )
                     triggerTerminateEvent();
 
-    // Must be deleted by base class dtor because this data
-    // is not owned by the SbClassModuleObject object
+    // prevent the base class destructor from deleting these because
+    // we do not actually own them
     pImage = nullptr;
     pBreaks = nullptr;
 }
diff --git a/basic/source/classes/sbxmod.cxx b/basic/source/classes/sbxmod.cxx
index 36b4afbb28f7..64e7d2ec7e65 100644
--- a/basic/source/classes/sbxmod.cxx
+++ b/basic/source/classes/sbxmod.cxx
@@ -415,7 +415,7 @@ static bool getDefaultVBAMode( StarBASIC* pb )
 
 SbModule::SbModule( const OUString& rName, bool bVBACompat )
          : SbxObject( "StarBASICModule" ),
-           mbVBACompat( bVBACompat ), bIsProxyModule( false )
+           pImage(nullptr), pBreaks(nullptr), mbVBACompat( bVBACompat ), bIsProxyModule( false )
 {
     SetName( rName );
     SetFlag( SbxFlagBits::ExtSearch | SbxFlagBits::GlobalSearch );
@@ -432,8 +432,8 @@ SbModule::SbModule( const OUString& rName, bool bVBACompat )
 SbModule::~SbModule()
 {
     SAL_INFO("basic","Module named " << GetName() << " is destructing");
-    pImage.reset();
-    pBreaks.reset();
+    delete pImage;
+    delete pBreaks;
     pClassData.reset();
     mxWrapper = nullptr;
 }
@@ -463,7 +463,7 @@ const SbxObject* SbModule::FindType( const OUString& aTypeName ) const
 
 void SbModule::StartDefinitions()
 {
-    pImage.reset();
+    delete pImage; pImage = nullptr;
     if( pClassData )
         pClassData->clear();
 
@@ -613,7 +613,7 @@ void SbModule::EndDefinitions( bool bNewState )
 
 void SbModule::Clear()
 {
-    pImage.reset();
+    delete pImage; pImage = nullptr;
     if( pClassData )
         pClassData->clear();
     SbxObject::Clear();
@@ -1494,7 +1494,7 @@ bool SbModule::SetBP( sal_uInt16 nLine )
     if( !IsBreakable( nLine ) )
         return false;
     if( !pBreaks )
-        pBreaks.reset( new SbiBreakpoints );
+        pBreaks = new SbiBreakpoints;
     auto it = std::find_if(pBreaks->begin(), pBreaks->end(),
         [&nLine](const sal_uInt16 b) { return b <= nLine; });
     if (it != pBreaks->end() && *it == nLine)
@@ -1522,7 +1522,8 @@ bool SbModule::ClearBP( sal_uInt16 nLine )
         }
         if( pBreaks->empty() )
         {
-            pBreaks.reset();
+            delete pBreaks;
+            pBreaks = nullptr;
         }
     }
     return bRes;
@@ -1530,14 +1531,15 @@ bool SbModule::ClearBP( sal_uInt16 nLine )
 
 void SbModule::ClearAllBP()
 {
-    pBreaks.reset();
+    delete pBreaks;
+    pBreaks = nullptr;
 }
 
 void
 SbModule::fixUpMethodStart( bool bCvtToLegacy, SbiImage* pImg ) const
 {
         if ( !pImg )
-            pImg = pImage.get();
+            pImg = pImage;
         for( sal_uInt32 i = 0; i < pMethods->Count(); i++ )
         {
             SbMethod* pMeth = dynamic_cast<SbMethod*>( pMethods->Get( static_cast<sal_uInt16>(i) )  );
@@ -1564,17 +1566,18 @@ bool SbModule::LoadData( SvStream& rStrm, sal_uInt16 nVer )
     rStrm.ReadUChar( bImage );
     if( bImage )
     {
-        std::unique_ptr<SbiImage> p( new SbiImage );
+        SbiImage* p = new SbiImage;
         sal_uInt32 nImgVer = 0;
 
         if( !p->Load( rStrm, nImgVer ) )
         {
+            delete p;
             return false;
         }
         // If the image is in old format, we fix up the method start offsets
         if ( nImgVer < B_EXT_IMG_VERSION )
         {
-            fixUpMethodStart( false, p.get() );
+            fixUpMethodStart( false, p );
             p->ReleaseLegacyBuffer();
         }
         aComment = p->aComment;
@@ -1586,13 +1589,15 @@ bool SbModule::LoadData( SvStream& rStrm, sal_uInt16 nVer )
             if( nVer == 1 )
             {
                 SetSource32( p->aOUSource );
+                delete p;
             }
             else
-                pImage = std::move(p);
+                pImage = p;
         }
         else
         {
             SetSource32( p->aOUSource );
+            delete p;
         }
     }
     return true;
diff --git a/basic/source/comp/codegen.cxx b/basic/source/comp/codegen.cxx
index dbed7a50ac0b..714418229192 100644
--- a/basic/source/comp/codegen.cxx
+++ b/basic/source/comp/codegen.cxx
@@ -135,7 +135,7 @@ void SbiCodeGen::Save()
     if( pParser->IsCodeCompleting() )
         return;
 
-    std::unique_ptr<SbiImage> p( new SbiImage );
+    SbiImage* p = new SbiImage;
     rMod.StartDefinitions();
     // OPTION BASE-Value:
     p->nDimBase = pParser->nBase;
@@ -152,7 +152,7 @@ void SbiCodeGen::Save()
 
         nIfaceCount = pParser->aIfaceVector.size();
         if( !rMod.pClassData )
-            rMod.pClassData.reset( new SbClassData );
+            rMod.pClassData.reset(new SbClassData);
         if( nIfaceCount )
         {
             for( int i = 0 ; i < nIfaceCount ; i++ )
@@ -377,7 +377,11 @@ void SbiCodeGen::Save()
     }
     if( !p->IsError() )
     {
-        rMod.pImage = std::move(p);
+        rMod.pImage = p;
+    }
+    else
+    {
+        delete p;
     }
     rMod.EndDefinitions();
 }
diff --git a/basic/source/runtime/runtime.cxx b/basic/source/runtime/runtime.cxx
index 3baba08b2f8a..e6b1dca97c62 100644
--- a/basic/source/runtime/runtime.cxx
+++ b/basic/source/runtime/runtime.cxx
@@ -572,7 +572,7 @@ SbMethod* SbiInstance::GetCaller( sal_uInt16 nLevel )
 
 SbiRuntime::SbiRuntime( SbModule* pm, SbMethod* pe, sal_uInt32 nStart )
          : rBasic( *static_cast<StarBASIC*>(pm->pParent) ), pInst( GetSbData()->pInst ),
-           pMod( pm ), pMeth( pe ), pImg( pMod->pImage.get() ), mpExtCaller(nullptr), m_nLastTime(0)
+           pMod( pm ), pMeth( pe ), pImg( pMod->pImage ), mpExtCaller(nullptr), m_nLastTime(0)
 {
     nFlags    = pe ? pe->GetDebugFlags() : BasicDebugFlags::NONE;
     pIosys    = pInst->GetIoSystem();
@@ -3158,9 +3158,10 @@ bool SbiRuntime::implIsClass( SbxObject const * pObj, const OUString& aClass )
         {
             const OUString& aObjClass = pObj->GetClassName();
             SbModule* pClassMod = GetSbData()->pClassFac->FindClass( aObjClass );
-            if( pClassMod && pClassMod->pClassData )
+            SbClassData* pClassData;
+            if( pClassMod && (pClassData=pClassMod->pClassData.get()) != nullptr )
             {
-                SbxVariable* pClassVar = pClassMod->pClassData->mxIfaces->Find( aClass, SbxClassType::DontCare );
+                SbxVariable* pClassVar = pClassData->mxIfaces->Find( aClass, SbxClassType::DontCare );
                 bRet = (pClassVar != nullptr);
             }
         }
diff --git a/include/basic/sbmod.hxx b/include/basic/sbmod.hxx
index 8b9b78e9a07f..608a763b06dd 100644
--- a/include/basic/sbmod.hxx
+++ b/include/basic/sbmod.hxx
@@ -27,7 +27,6 @@
 #include <rtl/ustring.hxx>
 #include <vector>
 #include <deque>
-#include <memory>
 #include <basic/basicdllapi.h>
 #include <com/sun/star/uno/Reference.hxx>
 
@@ -63,9 +62,9 @@ protected:
     css::uno::Reference< css::script::XInvocation > mxWrapper;
     OUString            aOUSource;
     OUString            aComment;
-    std::unique_ptr<SbiImage>        pImage;        // the Image
-    std::unique_ptr<SbiBreakpoints>  pBreaks;       // Breakpoints
-    std::unique_ptr<SbClassData>     pClassData;
+    SbiImage*           pImage;        // the Image
+    SbiBreakpoints*     pBreaks;       // Breakpoints
+    std::unique_ptr<SbClassData> pClassData;
     bool mbVBACompat;
     sal_Int32 mnType;
     SbxObjectRef pDocObject; // an impl object ( used by Document Modules )


More information about the Libreoffice-commits mailing list