Bug 145279 - ByRef argument modified in called Sub is returned incorrectly
Summary: ByRef argument modified in called Sub is returned incorrectly
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: BASIC (show other bugs)
Version:
(earliest affected)
7.2.2.2 release
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: Macro-StarBasic
  Show dependency treegraph
 
Reported: 2021-10-22 16:48 UTC by Jean-Pierre Ledure
Modified: 2024-01-18 14:08 UTC (History)
7 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 Jean-Pierre Ledure 2021-10-22 16:48:09 UTC
Description:
The same Basic function executed
1. once with a default argument
2. once with an explicit argument
(both arguments having the same value),
does not show the same behaviour.

See code below that adds 1 to the value 10. The results are
1. 10 (obviously not what is expected)
2. 11

REM ***********************************************
Sub Main()
	MsgBox Plus1()			'	Displays 10		=> NOK
	MsgBox Plus1(10)		'	Displays 11		=> OK
End Sub

REM ***********************************************
Function Plus1(Optional a)
	If IsMissing(a) Then a = 10
	Add1(a)
	Plus1 = a
End Function

REM **********************************************
Sub Add1(ByRef a)			'	Note the "ByRef"
	a = a + 1
End Sub	

Steps to Reproduce:
1. Copy the example code in a Basic module of a new empty document
2. Run the Main() Sub

Actual Results:
1. The value "10" is returned
2. The value "11" is returned

Expected Results:
The value "11" should be returned twice


Reproducible: Always


User Profile Reset: No



Additional Info:
Version: 7.2.2.2 / LibreOffice Community
Build ID: 02b2acce88a210515b4a5bb2e46cbfb63fe97d56
CPU threads: 6; OS: Linux 5.4; UI render: default; VCL: kf5 (cairo+xcb)
Locale: en-US (en_US.UTF-8); UI: en-US
Calc: threaded
Comment 1 Andreas Heinisch 2021-10-25 11:37:49 UTC
Tested with MS, returns both times 10. Unfortunately, I cannot confirm it since I don't know enough about macro programming. However, the behaviour can be reproduced. In the macro guide (https://www.pitonyak.org/book/) there is a reminder:

"Constants passed as arguments by reference cause unexpected behavior if their value is modified in the
called routine. The value may arbitrarily change back inside the called routine."
Comment 3 Jean-Pierre Ledure 2021-10-25 15:18:48 UTC
(In reply to Andreas Heinisch from comment #1)
> Tested with MS, returns both times 10. 

I would expect twice 11, not 10 ??
> 
> "Constants passed as arguments by reference cause unexpected behavior if
> their value is modified in the
> called routine. The value may arbitrarily change back inside the called
> routine."

This reminder does not seem relevant in the actual context. Passing a variable instead of a constant in the Main() routine does not change the behaviour.
Comment 4 Rafael Lima 2021-10-25 19:47:51 UTC
In Excel if you remove the parenthesis from the line "Add1(a)" it will return 11 in both cases. It should look like:

Sub Main()
	MsgBox Plus1()
	MsgBox Plus1(10)
End Sub

Function Plus1(Optional a)
	If IsMissing(a) Then a = 10
	Add1 a   ' <--- Note the change here
	Plus1 = a
End Function

Sub Add1(ByRef a)
	a = a + 1
End sub

It must have something to do with evaluation of expressions... when there is parenthesis, it is treated as an expression instead of a variable being passed by reference. So removing the parenthesis make it work as expected on Excel.

However, on LibreOffice Basic the error persists even when the parenthesis is removed. So I believe this is a bug.
Comment 5 Alain Romedenne 2021-10-28 09:41:06 UTC
I confirm this is a bug. Reproduced in Windows 10! The argument is received properly while the expression is not computed correctly:

REM ***********************************************
Sub Main()
	MsgBox Plus1()			'	Displays 10		=> NOK
	MsgBox Plus1(10)		'	Displays 11		=> OK
	MsgBox Plus1(10.)		'	Displays 11		=> OK
	MsgBox Plus1(-10.5)		'	Displays -9.5	=> OK
	MsgBox Plus1("10")		'	Displays "101"	=> OK
End Sub

Function Plus1(Optional a)
	If IsMissing(a) Then a = 10
	Add1(a)
	Plus1 = a
End Function

Sub Add1(ByRef b)			'	Note the "ByRef"
	Print b, TypeName(b),	' <<<< stack parm values, type names >>>>
	b = b + 1
End Sub
Comment 6 Andreas Heinisch 2021-10-28 10:41:22 UTC
Proposed patch: https://gerrit.libreoffice.org/c/core/+/124289
Comment 7 Andreas Heinisch 2021-10-31 21:53:45 UTC
Investigated the error and imho it is not trivial, because every parameter is considered by reference if not explicitly set. Then the runtime decides if the parameter will be handled either by value or by reference.

For instance, if a parameter is not fixed and not of type variant the parameter will be by value. I tried to remove the fixed part, but it breaks some tests.

Maybe someone else finds a solution to this problem. The test can be taken from the proposed patch.
Comment 8 QA Administrators 2023-11-04 03:14:56 UTC Comment hidden (obsolete)
Comment 9 Jean-Pierre Ledure 2023-11-04 08:13:27 UTC
The bug could be reproduced in:

Version: 7.6.2.1 (X86_64) / LibreOffice Community
Build ID: 56f7684011345957bbf33a7ee678afaf4d2ba333
CPU threads: 6; OS: Linux 5.15; UI render: default; VCL: gtk3
Locale: fr-BE (en_US.UTF-8); UI: en-US
Calc: threaded

No reason to change the bug status IMHO.
Comment 10 Vladimir Sokolinskiy 2024-01-18 14:08:14 UTC
As far as I know, in Excel the situation is as follows.
If a procedure parameter is described without the ByVal keyword, then:
1. If an argument is omitted when calling a procedure, a temporary argument (variable) of the appropriate type is created.
2. If the argument is a variable, then if the types of the argument and parameter do not match, an error is thrown.
3. If the argument is not a variable, then a temporary argument is created, which contains the calculated value of the argument.

Microsoft glossary and comments here: https://learn.microsoft.com/en-us/office/vba/language/concepts/getting-started/passing-arguments-efficiently