From 627cc6481f8a0b3df6ee550f16140b40340e86b2 Mon Sep 17 00:00:00 2001 From: Andreas Heinisch Date: Fri, 29 Oct 2021 08:40:14 +0200 Subject: tdf#145371 - Delete array variable only before ReDim Otherwise, global array variables don't maintain their state. Change-Id: I10dafd9e2946630c5476c9858f765d67ef2f6d6c Reviewed-on: https://gerrit.libreoffice.org/c/core/+/124368 Tested-by: Jenkins Reviewed-by: Andreas Heinisch (cherry picked from commit 27d96bfaea5f7967a11dc3a7e2bc9d88dd6f5666) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/124475 Reviewed-by: Xisco Fauli --- basic/CppunitTest_basic_macros.mk | 1 + basic/qa/cppunit/test_global_array.cxx | 89 ++++++++++++++++++++++++++++++++++ basic/source/comp/dim.cxx | 11 +++-- 3 files changed, 97 insertions(+), 4 deletions(-) create mode 100644 basic/qa/cppunit/test_global_array.cxx (limited to 'basic') diff --git a/basic/CppunitTest_basic_macros.mk b/basic/CppunitTest_basic_macros.mk index c70bfff42c68..83a9221369f0 100644 --- a/basic/CppunitTest_basic_macros.mk +++ b/basic/CppunitTest_basic_macros.mk @@ -20,6 +20,7 @@ $(eval $(call gb_CppunitTest_add_exception_objects,basic_macros, \ basic/qa/cppunit/test_nested_struct \ basic/qa/cppunit/test_vba \ basic/qa/cppunit/test_global_as_new \ + basic/qa/cppunit/test_global_array \ )) $(eval $(call gb_CppunitTest_use_libraries,basic_macros, \ diff --git a/basic/qa/cppunit/test_global_array.cxx b/basic/qa/cppunit/test_global_array.cxx new file mode 100644 index 000000000000..0b83d5efddee --- /dev/null +++ b/basic/qa/cppunit/test_global_array.cxx @@ -0,0 +1,89 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */ +/* + * This file is part of the LibreOffice project. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +#include +#include +#include +#include +#include +#include +#include + +namespace +{ +class GlobalArrayTest : public CppUnit::TestFixture +{ + void testMaintainsValueAcrossCalls(); + + CPPUNIT_TEST_SUITE(GlobalArrayTest); + CPPUNIT_TEST(testMaintainsValueAcrossCalls); + CPPUNIT_TEST_SUITE_END(); + + BasicDLL lib; + StarBASICRef interpreter; + + SbModuleRef Module() + { + interpreter = new StarBASIC(); + auto mod = interpreter->MakeModule("GlobalArray", R"BAS( + +Type testType + iNr As Integer + sType As String +End Type + +Global aTestTypes(2) As New testType + +Function Macro1 As String + aTestTypes(0).iNr = 1 + aTestTypes(0).sType = "A" + Macro1 = aTestTypes(0).iNr & aTestTypes(0).sType +End Function + +Function Macro2 As String + aTestTypes(1).iNr = 2 + aTestTypes(1).sType = "B" + Macro2 = aTestTypes(0).iNr & aTestTypes(0).sType & aTestTypes(1).iNr & aTestTypes(1).sType +End Function + + )BAS"); + CPPUNIT_ASSERT(mod->Compile()); + CPPUNIT_ASSERT_EQUAL(StarBASIC::GetErrBasic(), ERRCODE_NONE); + CPPUNIT_ASSERT_EQUAL(SbxBase::GetError(), ERRCODE_NONE); + CPPUNIT_ASSERT(mod->IsCompiled()); + return mod; + } +}; + +void GlobalArrayTest::testMaintainsValueAcrossCalls() +{ + auto m = Module(); + auto Macro1 = m->FindMethod("Macro1", SbxClassType::Method); + CPPUNIT_ASSERT_MESSAGE("Could not Find Macro1 in module", Macro1 != nullptr); + + // There is no SbxMethod::call(), the basic code is exercised here in the copy ctor + SbxVariableRef returned = new SbxMethod{ *Macro1 }; + CPPUNIT_ASSERT(returned->IsString()); + CPPUNIT_ASSERT_EQUAL(OUString{ "1A" }, returned->GetOUString()); + + auto Macro2 = m->FindMethod("Macro2", SbxClassType::Method); + CPPUNIT_ASSERT_MESSAGE("Could not Find Macro2 in module", Macro2 != nullptr); + returned = new SbxMethod{ *Macro2 }; + CPPUNIT_ASSERT(returned->IsString()); + // tdf#145371 - check if the global array has maintained its state + // Without the fix in place, this test would have failed with: + // - Expected: 1A2B + // - Actual : 02B + CPPUNIT_ASSERT_EQUAL(OUString("1A2B"), returned->GetOUString()); +} + +// Put the test suite in the registry +CPPUNIT_TEST_SUITE_REGISTRATION(GlobalArrayTest); + +} // namespace diff --git a/basic/source/comp/dim.cxx b/basic/source/comp/dim.cxx index 56bbab29587a..cbc25b0152b4 100644 --- a/basic/source/comp/dim.cxx +++ b/basic/source/comp/dim.cxx @@ -431,10 +431,13 @@ void SbiParser::DefVar( SbiOpcode eOp, bool bStatic ) } else { - // tdf#136755 - delete the variable beforehand REDIM - SbiExpression aExpr(this, *pDef, nullptr); - aExpr.Gen(); - aGen.Gen(bVBASupportOn ? SbiOpcode::ERASE_CLEAR_ : SbiOpcode::ERASE_); + // tdf#145371, tdf#136755 - only delete the variable beforehand REDIM + if (eOp == SbiOpcode::REDIM_) + { + SbiExpression aExpr(this, *pDef, nullptr); + aExpr.Gen(); + aGen.Gen(bVBASupportOn ? SbiOpcode::ERASE_CLEAR_ : SbiOpcode::ERASE_); + } pDef->SetDims( pDim->GetDims() ); SbiExpression aExpr2( this, *pDef, std::move(pDim) ); -- cgit