basic/qa/basic_coverage/test_With.bas | 51 ++++++++++++++++++++++++++++++++++ basic/source/comp/loops.cxx | 39 +++++++++++++++++++++++++- 2 files changed, 89 insertions(+), 1 deletion(-)
New commits: commit f3f46b5fe729876d128f63f7ab158954ab6657d7 Author: Mike Kaganski <[email protected]> AuthorDate: Sat Aug 17 22:10:07 2024 +0500 Commit: Mike Kaganski <[email protected]> CommitDate: Sat Aug 17 22:18:18 2024 +0200 tdf#132064: make With statement only evaluate its argument once This is implemented by creating a unique variable at entry of the block, using it as the "with parent", and clearing afterwards. It uses an invalid name, so can't be used by a user. The generation of the bytecode is done in compatible way; only existing opcodes were used (which was the reason to choose the implementation with a variable), so compiled binaries in password-protected libraries are expected to work with older versions. The functional changes are: 1. The argument of With statement is evaluated immediately at the start of the block; previously, it evaluated first time, when the leading dot operator was used, and could even be never evaluated, if the operator wasn't used. This is consistent with VBA, and has a benefit that the lifetime of the object is guaranteed to cover the whole block (might be useful if other block statements depend on the object, similar to Python's 'with' statement contexts). 2. The primary goal of the change: it is only ever evaluated once, no matter how many times was the dot operator used in the block. Change-Id: I4dbd520538a57e3668ab706e8f45740db5d33393 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171980 Reviewed-by: Mike Kaganski <[email protected]> Tested-by: Jenkins diff --git a/basic/qa/basic_coverage/test_With.bas b/basic/qa/basic_coverage/test_With.bas new file mode 100644 index 000000000000..389b89090d9b --- /dev/null +++ b/basic/qa/basic_coverage/test_With.bas @@ -0,0 +1,51 @@ +' +' 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/. +' + +Option Explicit + +Function doUnitTest as String + TestUtil.TestInit + test_with + doUnitTest = TestUtil.GetResult() +End Function + +Sub DEV_TST : MsgBox doUnitTesT : End Sub + +Type foo + n As Integer + s As String +End Type + +Dim get_foo_count As Integer + +Function get_foo As foo + get_foo_count = get_foo_count + 1 + get_foo = New foo +End Function + +Sub test_with + On Error GoTo errorHandler + + Dim fields As String + With get_foo() + .n = 5 + .s = "bar" + fields = "n = " & .n & " s = " & .s + End With + ' get_foo must be called only once; before the fix, it failed with + ' Number of calls to get_foo returned 4, expected 1 + TestUtil.AssertEqual(get_foo_count, 1, "Number of calls to get_foo") + ' Before the fix, each use of . resulted in creation of a new 'foo' object, + ' and previous assignments didn't reflect in the result; it failed with + ' Field values returned n = 0 s = , expected n = 5 s = bar + TestUtil.AssertEqual(fields, "n = 5 s = bar", "Field values") + + Exit Sub +errorHandler: + TestUtil.ReportErrorHandler("test_with", Err, Error$, Erl) +End Sub diff --git a/basic/source/comp/loops.cxx b/basic/source/comp/loops.cxx index 12053291fb8b..0b15c55f3c33 100644 --- a/basic/source/comp/loops.cxx +++ b/basic/source/comp/loops.cxx @@ -22,6 +22,9 @@ #include <memory> #include <basic/sberrors.hxx> +#include <basic/sbmod.hxx> + +#include <rtl/ustrbuf.hxx> static bool EndsIfBranch(SbiToken eTok) { @@ -290,9 +293,43 @@ void SbiParser::With() pNode->SetType( SbxOBJECT ); - OpenBlock( NIL, aVar.GetExprNode() ); + // Generate a '{_with_library.module_offset} = aVar.GetExprNode()' + // Use the {_with_library.module_offset} in OpenBlock + // The name of the variable can't be used by user: a name like [{_with_library.module_offset}] + // is valid, but not without the square brackets + + // Create the unique name + OUStringBuffer moduleName(aGen.GetModule().GetName()); + for (auto parent = aGen.GetModule().GetParent(); parent; parent = parent->GetParent()) + moduleName.insert(0, parent->GetName() + "."); + + OUString uniqueName = "{_with_" + moduleName + "_" + OUString::number(aGen.GetOffset()) + "}"; + while (pPool->Find(uniqueName) != nullptr) + { + static sal_Int64 unique_suffix; + uniqueName = "{_with_" + moduleName + "_" + OUString::number(aGen.GetOffset()) + "_" + + OUString::number(unique_suffix++) + "}"; + } + SbiSymDef* pWithParentDef = new SbiSymDef(uniqueName); + pWithParentDef->SetType(SbxOBJECT); + pPool->Add(pWithParentDef); + + // DIM local variable: work with Option Explicit + aGen.Gen(SbiOpcode::LOCAL_, pWithParentDef->GetId(), pWithParentDef->GetType()); + + // Assignment + SbiExpression aWithParent(this, *pWithParentDef); + aWithParent.Gen(); + aVar.Gen(); + aGen.Gen(SbiOpcode::SET_); + + OpenBlock(NIL, aWithParent.GetExprNode()); StmntBlock( ENDWITH ); CloseBlock(); + + // Erase {_with_library.module_offset} + aWithParent.Gen(); + aGen.Gen(SbiOpcode::ERASE_); } // LOOP/NEXT/WEND without construct
