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@collabora.co.uk>

4 files changed, 33 insertions, 22 deletions

diff --git a/basic/source/classes/sb.cxx b/basic/source/classes/sb.cxx
index 00454a759ef0..dac4197c18f6 100644
--- a/basic/source/classes/sb.cxx
+++ b/basic/source/classes/sb.cxx
@@ -632,8 +632,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() );
 
@@ -776,8 +777,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 f288f0ec99ae..ee0123710e3c 100644
--- a/basic/source/classes/sbxmod.cxx
+++ b/basic/source/classes/sbxmod.cxx
@@ -416,7 +416,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 );
@@ -433,8 +433,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;
 }
@@ -464,7 +464,7 @@ const SbxObject* SbModule::FindType( const OUString& aTypeName ) const
 
 void SbModule::StartDefinitions()
 {
-    pImage.reset();
+    delete pImage; pImage = nullptr;
     if( pClassData )
         pClassData->clear();
 
@@ -614,7 +614,7 @@ void SbModule::EndDefinitions( bool bNewState )
 
 void SbModule::Clear()
 {
-    pImage.reset();
+    delete pImage; pImage = nullptr;
     if( pClassData )
         pClassData->clear();
     SbxObject::Clear();
@@ -1531,7 +1531,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)
@@ -1559,7 +1559,8 @@ bool SbModule::ClearBP( sal_uInt16 nLine )
         }
         if( pBreaks->empty() )
         {
-            pBreaks.reset();
+            delete pBreaks;
+            pBreaks = nullptr;
         }
     }
     return bRes;
@@ -1567,14 +1568,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) )  );
@@ -1601,17 +1603,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;
@@ -1623,13 +1626,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 569824648fc1..f36ab61f7cf1 100644
--- a/basic/source/comp/codegen.cxx
+++ b/basic/source/comp/codegen.cxx
@@ -139,7 +139,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;
@@ -156,7 +156,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++ )
@@ -381,7 +381,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 8451455fbe89..993493a655a7 100644
--- a/basic/source/runtime/runtime.cxx
+++ b/basic/source/runtime/runtime.cxx
@@ -570,7 +570,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();
@@ -3163,9 +3163,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);
             }
         }