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 <andreas.heinisch@yahoo.de>
(cherry picked from commit 27d96bfaea5f7967a11dc3a7e2bc9d88dd6f5666)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/124475
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>

3 files changed, 97 insertions, 4 deletions

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 <basic/sbstar.hxx>
+#include <basic/sbmeth.hxx>
+#include <basic/basrdll.hxx>
+#include <cppunit/TestSuite.h>
+#include <cppunit/plugin/TestPlugIn.h>
+#include <cppunit/extensions/HelperMacros.h>
+#include <iostream>
+
+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) );