basic/source/classes/sb.cxx                          |    6 +++---
 basic/source/classes/sbxmod.cxx                      |   12 ++++++++----
 basic/source/comp/codegen.cxx                        |    4 +---
 include/basic/sbmod.hxx                              |    7 ++++---
 sc/qa/extras/macros-test.cxx                         |   13 +++++++++++++
 sc/qa/extras/testdocuments/udf_plus_class_module.ods |binary
 6 files changed, 29 insertions(+), 13 deletions(-)

New commits:
commit 5219ea7b98bb35fc9f4ec39f25ebfafb7346bc25
Author:     Mike Kaganski <[email protected]>
AuthorDate: Wed Oct 8 19:56:15 2025 +0500
Commit:     Xisco Fauli <[email protected]>
CommitDate: Mon Oct 20 10:57:36 2025 +0200

    tdf#168750: Make sure that class module is recognized on load
    
    The problem appeared, when a macro loaded a library with a class module,
    that had a public element (e.g., a function) with the same name, as used
    for a local variable in the macro without a prior DIM.
    
    Normally, there is a precaution in SbModule::Find, which hides the names
    in class modules. However, this precaution failed during load.
    
    The reason was, that the flag identifying the module as a class module
    was set only when the module got compiled. It turned out, that first, it
    is parsed in SbModule::SetSource32 only partially, with minimal work to
    recognize the names (and a few other small things). At this stage, it is
    not marked as class module yet; and when the clashing name is looked up,
    it gets found in the module. Then its value is requested; at this point,
    the module gets compiled, and the "class module" flag is set; then, if
    the class module element tries to use another name from the same module,
    it fails, which produces visible error message.
    
    This change makes sure, that the initial fast parse stage also checks
    Option ClassModule, to set it properly. Also it drops redundant variable
    bIsProxyModule from SbModule, because mnType already contains the needed
    type information. And lastly, it reorders SbModule::Find a bit, to avoid
    searching a module, only to discard the result immediately, because it's
    a class module.
    
    Change-Id: I8add9873bf31fb767b64d27a36aef8e3a89a0de0
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/192075
    Reviewed-by: Mike Kaganski <[email protected]>
    Tested-by: Jenkins
    Signed-off-by: Xisco Fauli <[email protected]>
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/192689

diff --git a/basic/source/classes/sb.cxx b/basic/source/classes/sb.cxx
index ae9f4d0adeed..a27811bde425 100644
--- a/basic/source/classes/sb.cxx
+++ b/basic/source/classes/sb.cxx
@@ -1175,7 +1175,7 @@ void StarBASIC::InitAllModules( StarBASIC const * 
pBasicNotToInit )
     for (const auto& pModule: pModules)
     {
         OUString aModuleName = pModule->GetName();
-        if( pModule->isProxyModule() )
+        if (pModule->isClassModule())
         {
             aMIDMap[aModuleName] = ClassModuleRunInitItem( pModule.get() );
         }
@@ -1190,7 +1190,7 @@ void StarBASIC::InitAllModules( StarBASIC const * 
pBasicNotToInit )
     // Call RunInit on standard modules
     for (const auto& pModule: pModules)
     {
-        if( !pModule->isProxyModule() )
+        if (!pModule->isClassModule())
         {
             pModule->RunInit();
         }
@@ -1216,7 +1216,7 @@ void StarBASIC::DeInitAllModules()
     // Deinit own modules
     for (const auto& pModule: pModules)
     {
-        if( pModule->pImage && !pModule->isProxyModule() && 
dynamic_cast<SbObjModule*>( pModule.get()) == nullptr )
+        if( pModule->pImage && !pModule->isClassModule() && 
dynamic_cast<SbObjModule*>( pModule.get()) == nullptr )
         {
             pModule->pImage->bInit = false;
         }
diff --git a/basic/source/classes/sbxmod.cxx b/basic/source/classes/sbxmod.cxx
index 6558050e1325..a838a1efc42f 100644
--- a/basic/source/classes/sbxmod.cxx
+++ b/basic/source/classes/sbxmod.cxx
@@ -418,7 +418,7 @@ static bool getDefaultVBAMode( StarBASIC* pb )
 
 SbModule::SbModule( const OUString& rName, bool bVBASupport )
     : SbxObject( u"StarBASICModule"_ustr )
-    , mbVBASupport(bVBASupport), mbCompat(bVBASupport), bIsProxyModule(false)
+    , mbVBASupport(bVBASupport), mbCompat(bVBASupport)
 {
     SetName( rName );
     SetFlag( SbxFlagBits::ExtSearch | SbxFlagBits::GlobalSearch );
@@ -626,11 +626,11 @@ void SbModule::Clear()
 SbxVariable* SbModule::Find( const OUString& rName, SbxClassType t )
 {
     // make sure a search in an uninstantiated class module will fail
-    SbxVariable* pRes = SbxObject::Find( rName, t );
-    if ( bIsProxyModule && !GetSbData()->bRunInit )
+    if (isClassModule() && !GetSbData()->bRunInit)
     {
         return nullptr;
     }
+    SbxVariable* pRes = SbxObject::Find( rName, t );
     if( !pRes && pImage )
     {
         SbiInstance* pInst = GetSbData()->pInst;
@@ -846,6 +846,10 @@ void SbModule::SetSource32( const OUString& r )
                         SetVBASupport( bIsVBA );
                         aTok.SetCompatible( bIsVBA );
                     }
+                    else if (eCurTok == CLASSMODULE)
+                    {
+                        SetModuleType(css::script::ModuleType::CLASS);
+                    }
                 }
             }
             eLastTok = eCurTok;
@@ -1376,7 +1380,7 @@ void StarBASIC::ClearAllModuleVars()
     for (const auto& rModule: pModules)
     {
         // Initialise only, if the startcode was already executed
-        if( rModule->pImage && rModule->pImage->bInit && 
!rModule->isProxyModule() && dynamic_cast<const SbObjModule*>( rModule.get()) 
== nullptr )
+        if( rModule->pImage && rModule->pImage->bInit && 
!rModule->isClassModule() && dynamic_cast<const SbObjModule*>( rModule.get()) 
== nullptr )
             rModule->ClearPrivateVars();
     }
 
diff --git a/basic/source/comp/codegen.cxx b/basic/source/comp/codegen.cxx
index ea3856269cf3..40b6a6fc00f4 100644
--- a/basic/source/comp/codegen.cxx
+++ b/basic/source/comp/codegen.cxx
@@ -148,9 +148,8 @@ void SbiCodeGen::Save()
         p->SetFlag( SbiImageFlags::EXPLICIT );
 
     int nIfaceCount = 0;
-    if( rMod.mnType == css::script::ModuleType::CLASS )
+    if (rMod.isClassModule())
     {
-        rMod.bIsProxyModule = true;
         p->SetFlag( SbiImageFlags::CLASSMODULE );
         GetSbData()->pClassFac->AddClassModule( &rMod );
 
@@ -179,7 +178,6 @@ void SbiCodeGen::Save()
         {
             rMod.mnType = css::script::ModuleType::NORMAL;
         }
-        rMod.bIsProxyModule = false;
     }
 
     // GlobalCode-Flag
diff --git a/include/basic/sbmod.hxx b/include/basic/sbmod.hxx
index 5c44d28f0f5f..e6a47632d022 100644
--- a/include/basic/sbmod.hxx
+++ b/include/basic/sbmod.hxx
@@ -30,6 +30,8 @@
 #include <basic/basicdllapi.h>
 #include <com/sun/star/uno/Reference.hxx>
 
+#include <com/sun/star/script/ModuleType.hpp>
+
 namespace com::sun::star::script { class XInvocation; }
 
 class SbMethod;
@@ -67,9 +69,8 @@ protected:
     std::unique_ptr<SbClassData> pClassData;
     bool mbVBASupport; // Option VBASupport
     bool mbCompat; // Option Compatible
-    sal_Int32 mnType;
+    sal_Int32 mnType; // css::script::ModuleType
     tools::SvRef<SbUnoObject> pDocObject; // an impl object ( used by Document 
Modules )
-    bool    bIsProxyModule;
 
     SAL_DLLPRIVATE static void implProcessModuleRunInit( 
ModuleInitDependencyMap& rMap, ClassModuleRunInitItem& rItem );
     SAL_DLLPRIVATE void StartDefinitions();
@@ -126,7 +127,7 @@ public:
     SAL_DLLPRIVATE void SetVBASupport( bool bSupport );
     sal_Int32 GetModuleType() const { return mnType; }
     void     SetModuleType( sal_Int32 nType ) { mnType = nType; }
-    bool     isProxyModule() const { return bIsProxyModule; }
+    bool isClassModule() const { return GetModuleType() == 
css::script::ModuleType::CLASS; }
     SAL_DLLPRIVATE void AddVarName( const OUString& aName );
     SAL_DLLPRIVATE void RemoveVars();
     css::uno::Reference< css::script::XInvocation > const & GetUnoModule();
diff --git a/sc/qa/extras/macros-test.cxx b/sc/qa/extras/macros-test.cxx
index 18956e8c5fea..00d85cea6989 100644
--- a/sc/qa/extras/macros-test.cxx
+++ b/sc/qa/extras/macros-test.cxx
@@ -1012,6 +1012,19 @@ CPPUNIT_TEST_FIXTURE(ScMacrosTest, 
testTdf81003_DateCellToVbaUDF)
     CPPUNIT_ASSERT_EQUAL(45898.0, pDoc->GetValue(1, 0, 0));
 }
 
+CPPUNIT_TEST_FIXTURE(ScMacrosTest, testTdf168750)
+{
+    // A class module's global names must not be visible without instantiated 
object
+    createScDoc("udf_plus_class_module.ods");
+    ScDocument* pDoc = getScDoc();
+
+    // B1 must have "FALSE", i.e. no errors happened during the load time.
+    // Without the fix, this would fail with
+    // - Unexpected dialog:  Error: BASIC runtime error. Argument is not 
optional.
+    // Which indicates, that the class module function was unexpectedly called.
+    CPPUNIT_ASSERT_EQUAL(u"FALSE"_ustr, pDoc->GetString(ScAddress(1, 0, 0)));
+}
+
 ScMacrosTest::ScMacrosTest()
       : ScModelTestBase(u"/sc/qa/extras/testdocuments"_ustr)
 {
diff --git a/sc/qa/extras/testdocuments/udf_plus_class_module.ods 
b/sc/qa/extras/testdocuments/udf_plus_class_module.ods
new file mode 100644
index 000000000000..0e0ff1db20b1
Binary files /dev/null and 
b/sc/qa/extras/testdocuments/udf_plus_class_module.ods differ

Reply via email to