Bug 132064 - "With" statement likely evaluates its argument on each unqualified member access
Summary: "With" statement likely evaluates its argument on each unqualified member access
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: BASIC (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: All All
: medium normal
Assignee: Mike Kaganski
URL:
Whiteboard: target:25.2.0 target:24.8.1
Keywords:
: 99554 (view as bug list)
Depends on:
Blocks: Macro-StarBasic
  Show dependency treegraph
 
Reported: 2020-04-12 12:27 UTC by Mike Kaganski
Modified: 2024-09-30 17:53 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mike Kaganski 2020-04-12 12:27:29 UTC
Look at this BASIC code:

> sub TestRootAccess1
>   GlobalScope.BasicLibraries.LoadLibrary("Tools")
>   Dim KeyWriter As Object
>   KeyWriter = GetRegistryKeyContent("org.openoffice.Office.Calc/Input", True)
>   KeyWriter.setPropertyValue("ExpandReference", True)
>   KeyWriter.commitChanges()
>   ' Use *different* object to read the setting
>   MsgBox GetRegistryKeyContent("org.openoffice.Office.Calc/Input").getByName("ExpandReference")
> end sub

This code will show "True", irrespective of previous setting value (which means it works as expected).

Now modify that like this:

> sub TestRootAccess2
>   GlobalScope.BasicLibraries.LoadLibrary("Tools")
>   With GetRegistryKeyContent("org.openoffice.Office.Calc/Input", True)
>     .setPropertyValue("ExpandReference", False)
>     .commitChanges()
>   End With
>   ' Use *different* object to read the setting
>   MsgBox GetRegistryKeyContent("org.openoffice.Office.Calc/Input").getByName("ExpandReference")
> end sub

Now this code will not actually change the setting; e.g., if TestRootAccess2 run after the previous TestRootAccess1, the result of TestRootAccess2 will be True, not the expected False.

The problem seems to be that the With statement does not evaluate its argument (GetRegistryKeyContent("org.openoffice.Office.Calc/Input", True)) at entry to use it later at each member access, but evaluates it each time anew when it accesses the members of the object using unqualified ".member" syntax. That means that commitChanges() is executed for a different object than setPropertyValue(), and the changes are not committed.

The help for With statement [1] mentions "single object":

> Use With and End With if you have several properties or methods for a single object.

VBA help [2] also says about that:

> Executes a series of statements on a single object or a user-defined type.

VB help [3] goes even further:

> objectExpression Required. An expression that evaluates to an object. The expression may be arbitrarily complex and is evaluated only once.

So the implementation of With looks inconsistent, unexpected, inefficient (in the best case) and wrong.

[1] https://help.libreoffice.org/6.4/en-US/text/sbasic/shared/03090411.html
[2] https://docs.microsoft.com/en-us/office/vba/language/reference/user-interface-help/with-statement
[3] https://docs.microsoft.com/en-us/dotnet/visual-basic/language-reference/statements/with-end-with-statement

Tested with Version: 6.4.3.1 (x64)
Build ID: 4d2b2b47cca498fed6abf712a36d0788901091eb
CPU threads: 12; OS: Windows 10.0 Build 18363; UI render: default; VCL: win; 
Locale: en-US (ru_RU); UI-Language: en-US
Calc: threaded
Comment 1 Mike Kaganski 2022-03-17 06:24:47 UTC
Already repro with OOo 2.2.0
Comment 2 Mike Kaganski 2022-03-17 07:02:01 UTC
*** Bug 99554 has been marked as a duplicate of this bug. ***
Comment 3 QA Administrators 2024-03-17 03:14:16 UTC Comment hidden (obsolete)
Comment 4 Mike Kaganski 2024-08-17 17:30:39 UTC
https://gerrit.libreoffice.org/c/core/+/171980
Comment 5 Commit Notification 2024-08-17 20:19:06 UTC
Mike Kaganski committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/f3f46b5fe729876d128f63f7ab158954ab6657d7

tdf#132064: make With statement only evaluate its argument once

It will be available in 25.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 6 Commit Notification 2024-08-18 08:26:47 UTC
Mike Kaganski committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/fa8a0f514a4c722974410e4cbfa8ea12f3304003

Related: tdf#132064 Use set to clear the With internal variable

It will be available in 25.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 7 Commit Notification 2024-08-19 21:00:13 UTC
Mike Kaganski committed a patch related to this issue.
It has been pushed to "libreoffice-24-8":

https://git.libreoffice.org/core/commit/bf87977617ead6221e0a42fea3d3e69c983dd805

tdf#132064: make With statement only evaluate its argument once

It will be available in 24.8.1.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 8 Commit Notification 2024-08-19 21:00:15 UTC
Mike Kaganski committed a patch related to this issue.
It has been pushed to "libreoffice-24-8":

https://git.libreoffice.org/core/commit/46dfd2c75bfc9a56659db00f1a8b3946bc8d8340

Related: tdf#132064 Use set to clear the With internal variable

It will be available in 24.8.1.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 9 Roland Baudin 2024-09-22 13:52:01 UTC
This patch breaks the TexMaths extension. Indeed in TexMaths I frequently use Basic code like the following one to create resizable dialogs:


' Create new outer base dialog
' oParent: parent window peer
' Return dialog window
Function CreateBaseDialog(oParent as Variant) as Variant
     
	Dim oToolkit as Variant
	oToolkit = oParent.getToolkit()
	
	Dim WA as Variant, aRect as Variant, oDesc as Variant
	WA = com.sun.star.awt.WindowAttribute
	aRect = CreateUnoStruct("com.sun.star.awt.Rectangle")
	oDesc = CreateUnoStruct("com.sun.star.awt.WindowDescriptor")

	With oDesc

    	.Type = com.sun.star.awt.WindowClass.SIMPLE
        .WindowServiceName = "dialog"
        .Parent = oParent
        .ParentIndex = -1
        .Bounds = aRect
        .WindowAttributes = WA.CLOSEABLE OR WA.MOVEABLE OR WA.SIZEABLE OR WA.BORDER

	End With

	Dim oDialog as Variant
	oDialog = oToolkit.createWindow(oDesc)

	' Create listeners
	oDialog.addTopWindowListener(CreateUnoListener("MainWindowListener_", "com.sun.star.awt.XTopWindowListener"))
	oDialog.addWindowListener(CreateUnoListener("MainWindowListener_", "com.sun.star.awt.XWindowListener"))

	CreateBaseDialog = oDialog

End Function


The above function is called like this:

Sub Test

	' Parent window
	Dim oParent as Variant
	oParent = ThisComponent.getCurrentController().getFrame().getContainerWindow()

	' Create base outer window as modal dialog
	Dim oDlgBaseMain as Variant
	oDlgBaseMain = CreateBaseDialog(oParent)
	

	' Set window title
	oDlgBaseMain.setTitle("Dialog Title") 
	
	
End Sub


Running the Test() subprogram in LibreOffice 24.8.1.2 results in the following error message: "Basic runtime error. Property or method not found: setTitle."

This worked without problem in LibreOffice 24.8.0.2.

A simple workaround is to replace:

	With oDesc

    	.Type = com.sun.star.awt.WindowClass.SIMPLE
        .WindowServiceName = "dialog"
        .Parent = oParent
        .ParentIndex = -1
        .Bounds = aRect
        .WindowAttributes = WA.CLOSEABLE OR WA.MOVEABLE OR WA.SIZEABLE OR WA.BORDER

	End With

with:


	'With oDesc

    	oDesc.Type = com.sun.star.awt.WindowClass.SIMPLE
        oDesc.WindowServiceName = "dialog"
        oDesc.Parent = oParent
        oDesc.ParentIndex = -1
        oDesc.Bounds = aRect
        oDesc.WindowAttributes = WA.CLOSEABLE OR WA.MOVEABLE OR WA.SIZEABLE OR WA.BORDER

	'End With

in function CreateBaseDialog().

This problem probably affects other extensions...
Comment 10 Mike Kaganski 2024-09-22 13:58:36 UTC
*This* one is RESOLVED FIXED. Any regression from this are *separate* bugs (with this in See Also).

Now please take a look at bug 162962, and see if its fix possibly fixes your issue, too. If not, please do file it separately. Thank you.
Comment 11 Mike Kaganski 2024-09-22 14:04:05 UTC
... or maybe more likely bug 162935.