basic/qa/basic_coverage/test_With.bas | 11 +++
basic/source/comp/loops.cxx | 97 +++++++++++++++++++++-------------
2 files changed, 72 insertions(+), 36 deletions(-)
New commits:
commit acd0556dc8d0ee48c4a0b338a093c9ece029a57c
Author: Mike Kaganski <mike.kagan...@collabora.com>
AuthorDate: Sun Oct 6 22:53:18 2024 +0500
Commit: Xisco Fauli <xiscofa...@libreoffice.org>
CommitDate: Tue Oct 8 10:23:03 2024 +0200
tdf#163219: only create local With variable for function results
The problem is, that having a local variable referring the same value
as the original expression 'foo' is not the same as referencing 'foo'
itself. After 'foo' is re-assigned, the local variable still refers
to the original value, not the new one.
It seems impossible to implement the reference using existing codegen
primitives, to imitate the true reference to 'foo', not to its value.
If we implement it by changing the runtime, the bytecode won't work
identically in older versions; if we introduce a new bytecode, that
would be an incompatible change.
As a workaround, only create the local With variable, when the block
variable is created using some function (as much as known by parser).
I think that there would be cases when this would still not work as
intended: an example is a property implemented using getter function;
the parser would likely treat the property as a variable, and avoid
creation of the local variable; and the getter would be called every
time a dot access will happen (which was the essence of tdf#132064).
However, this seems a better alternative to the bug fixed here.
Change-Id: I50bf679762fd2e73f215a000fa0ab60fd6ae7453
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/174564
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>
(cherry picked from commit ff3791f67a6421d64f5f9d6a09feaead1a63ff92)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/174453
Reviewed-by: Xisco Fauli <xiscofa...@libreoffice.org>
diff --git a/basic/qa/basic_coverage/test_With.bas
b/basic/qa/basic_coverage/test_With.bas
index c09fc037bb06..b22e0d94ba1f 100644
--- a/basic/qa/basic_coverage/test_With.bas
+++ b/basic/qa/basic_coverage/test_With.bas
@@ -69,6 +69,17 @@ Sub test_with
fields = "n = " & foo_var.n & " s = " & foo_var.s
TestUtil.AssertEqual(fields, "n = 6 s = baz", "Field values of foo_var")
+ ' tdf#163219: make sure that assigning the variable in the block works
+ Set foo_var = Nothing
+ TestUtil.Assert(foo_var Is Nothing, "foo_var Is Nothing")
+ With foo_var
+ foo_var = New foo
+ .n = 7
+ .s = "something"
+ End With
+ TestUtil.AssertEqual(foo_var.n, 7, "foo_var.n")
+ TestUtil.AssertEqual(foo_var.s, "something", "foo_var.s")
+
' tdf#162935: Test an UNO struct - it used to copy into the With variable,
not used by ref
Dim uno_struct As New com.sun.star.table.CellRangeAddress
With uno_struct
diff --git a/basic/source/comp/loops.cxx b/basic/source/comp/loops.cxx
index 997e2a9db23c..b638c82fd151 100644
--- a/basic/source/comp/loops.cxx
+++ b/basic/source/comp/loops.cxx
@@ -273,6 +273,63 @@ void SbiParser::For()
// WITH .. END WITH
+namespace
+{
+// Generate a '{_with_library.module_offset} = rVar'
+// 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
+struct WithLocalVar
+{
+ WithLocalVar(SbiParser& rParser, SbiExpression& rVar)
+ : m_rParser(rParser)
+ , m_aWithParent(createLocalVar(rParser))
+ {
+ // Assignment
+ m_aWithParent.Gen();
+ rVar.Gen();
+ m_rParser.aGen.Gen(SbiOpcode::PUTC_);
+ }
+
+ ~WithLocalVar()
+ {
+ // {_with_library.module_offset} = Nothing
+ m_aWithParent.Gen();
+ m_rParser.aGen.Gen(SbiOpcode::RTL_,
m_rParser.aGblStrings.Add(u"Nothing"_ustr), SbxOBJECT);
+ m_rParser.aGen.Gen(SbiOpcode::PUTC_);
+ }
+
+ static SbiExpression createLocalVar(SbiParser& rParser)
+ {
+ // Create the unique name
+ OUStringBuffer moduleName(rParser.aGen.GetModule().GetName());
+ for (auto parent = rParser.aGen.GetModule().GetParent(); parent;
+ parent = parent->GetParent())
+ moduleName.insert(0, parent->GetName() + ".");
+
+ OUString uniqueName
+ = "{_with_" + moduleName + "_" +
OUString::number(rParser.aGen.GetOffset()) + "}";
+ while (rParser.pPool->Find(uniqueName) != nullptr)
+ {
+ static sal_Int64 unique_suffix;
+ uniqueName = "{_with_" + moduleName + "_" +
OUString::number(rParser.aGen.GetOffset())
+ + "_" + OUString::number(unique_suffix++) + "}";
+ }
+ SbiSymDef* pWithParentDef = new SbiSymDef(uniqueName);
+ pWithParentDef->SetType(SbxOBJECT);
+ rParser.pPool->Add(pWithParentDef);
+
+ // DIM local variable: work with Option Explicit
+ rParser.aGen.Gen(SbiOpcode::LOCAL_, pWithParentDef->GetId(),
pWithParentDef->GetType());
+
+ return SbiExpression(&rParser, *pWithParentDef);
+ }
+
+ SbiParser& m_rParser;
+ SbiExpression m_aWithParent;
+};
+}
+
void SbiParser::With()
{
SbiExpression aVar( this, SbOPERAND );
@@ -287,47 +344,15 @@ void SbiParser::With()
else if( pDef->GetType() != SbxOBJECT )
Error( ERRCODE_BASIC_NEEDS_OBJECT );
-
pNode->SetType( SbxOBJECT );
- // 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() + ".");
+ std::optional<WithLocalVar> oLocalVar;
+ if (pDef->GetProcDef())
+ oLocalVar.emplace(*this, aVar);
- 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::PUTC_);
-
- OpenBlock(NIL, aWithParent.GetExprNode());
+ OpenBlock(NIL, oLocalVar ? oLocalVar->m_aWithParent.GetExprNode() :
aVar.GetExprNode());
StmntBlock( ENDWITH );
CloseBlock();
-
- // {_with_library.module_offset} = Nothing
- aWithParent.Gen();
- aGen.Gen(SbiOpcode::RTL_, aGblStrings.Add(u"Nothing"_ustr), SbxOBJECT);
- aGen.Gen(SbiOpcode::PUTC_);
}
// LOOP/NEXT/WEND without construct
