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 79a2e1d861c9e6b8cc7e9ff3fbb298475176bcf1 Author: Mike Kaganski <[email protected]> AuthorDate: Wed Oct 8 19:56:15 2025 +0500 Commit: Mike Kaganski <[email protected]> CommitDate: Thu Oct 9 05:57:56 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 diff --git a/basic/source/classes/sb.cxx b/basic/source/classes/sb.cxx index 4babf05641c8..301cd724ed2f 100644 --- a/basic/source/classes/sb.cxx +++ b/basic/source/classes/sb.cxx @@ -1174,7 +1174,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() ); } @@ -1189,7 +1189,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(); } @@ -1215,7 +1215,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 c3c3963f78df..0d3338f88252 100644 --- a/basic/source/classes/sbxmod.cxx +++ b/basic/source/classes/sbxmod.cxx @@ -419,7 +419,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 ); @@ -627,11 +627,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; @@ -847,6 +847,10 @@ void SbModule::SetSource32( const OUString& r ) SetVBASupport( bIsVBA ); aTok.SetCompatible( bIsVBA ); } + else if (eCurTok == CLASSMODULE) + { + SetModuleType(css::script::ModuleType::CLASS); + } } } eLastTok = eCurTok; @@ -1377,7 +1381,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 50d98027d4dd..cb69053e187b 100644 --- a/sc/qa/extras/macros-test.cxx +++ b/sc/qa/extras/macros-test.cxx @@ -1069,6 +1069,19 @@ CPPUNIT_TEST_FIXTURE(ScMacrosTest, testTdf161948NaturalSortDispatcher) } } +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
