Bug 36737 - default values of string type and object type for optional parameters are not processed
Summary: default values of string type and object type for optional parameters are not...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: BASIC (show other bugs)
Version:
(earliest affected)
3.5.0 Beta3
Hardware: x86 (IA32) Windows (All)
: medium normal
Assignee: Andreas Heinisch
URL:
Whiteboard: target:7.0.0
Keywords:
Depends on:
Blocks: Macro-StarBasic
  Show dependency treegraph
 
Reported: 2011-05-01 04:18 UTC by jan.benda
Modified: 2020-03-02 09:22 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:


Attachments
Function to demonstrate problem (13.24 KB, application/vnd.oasis.opendocument.spreadsheet)
2014-07-25 14:01 UTC, Bob
Details

Note You need to log in before you can comment on or make changes to this bug.
Description jan.benda 2011-05-01 04:18:38 UTC
Example code /how to reproduce/
REM  *****  BASIC  *****
Option VBASupport 1
Option Explicit

Sub Main()

MsgBox func1   //output empty box
MsgBox func1("Par1_SetValue") --output box with Par1_SetValue

End Sub


Function func1(Optional MyPar1 As String = "Par1_DefaultValue") as String
func1 = MyPar1
End Function
Comment 1 Björn Michaelsen 2011-12-23 12:05:28 UTC Comment hidden (obsolete)
Comment 2 August Sodora 2012-01-08 16:29:24 UTC
Reproduced on master with the following code:

REM  *****  BASIC  *****
Option VBASupport 1
Option Explicit

Sub Main()

MsgBox(func1())
MsgBox func1("Par1_SetValue")

End Sub

Function func1(Optional MyPar1 As String = "Par1_DefaultValue") as String
func1 = MyPar1
End Function

----------

Result: An empty message box pops up followed by a message box that says "Par1_SetValue"

Expected Result: A message box that says "Par1_DefaultValue" followed by a message box that says "Par1_SetValue"

It looks to me like this is in some vba-specific part of the code because I get syntax errors if I remove the Option VBASupport 1.
Comment 3 QA Administrators 2014-06-25 17:38:03 UTC Comment hidden (obsolete)
Comment 4 Bob 2014-07-25 14:00:41 UTC
I was about to open a very similar bug report and can provide a simple test case to demonstrate the problem still occurs of LO 4.3.0.3

VBA allows an optional parameter to be defined as - optional str as string = "abc" - the variable is not initialized to "abc" in LO

Alternatively if no default is provided - optional mstr as string mstr  - should not be initialized and isMissing(mstr) should return True it actually returns false and mstr is a null string

LO works correctly for data type Double and Variant,  I have not tested other data types

This error is particularly insidious when porting from Excel since no error is raised and differences in function execution may not be readily apparent.

I used the following function to show the problem:

function TestOpt(optional num as double =123,  optional str as string = "abc", optional numv  =456,  optional strv = "def", optional miss as double, optional mstr as string) as string 
Dim printstr as string 

printstr =              "num: >>" &num &"<<  isMissing " & isMissing(num) 
printstr = printstr & "; str: >>" &str &"<<  isMissing " & isMissing(str) 

printstr = printstr & "; numv: >>" &numv &"<<  isMissing " & isMissing(numv) 
printstr = printstr & "; strv: >>" &strv &"<<  isMissing " & isMissing(strv)
rem print printstr
printstr = printstr & "; miss: >>" &miss &"<<  isMissing " & isMissing(miss)
printstr = printstr & "; mstr: >>" &mstr &"<<  isMissing " & isMissing(mstr)
print printstr
TestOpt = printstr
end function 

Output is 
num: >>123<<  isMissing False; str: >><<  isMissing False; numv: >>456<<  isMissing False; strv: >>def<<  isMissing False; miss: >>Error ǀ<<  isMissing True; mstr: >><<  isMissing False

str and mstr are handled incorrectly, Double and Variant formats are correct
Comment 5 Bob 2014-07-25 14:01:33 UTC
Created attachment 103446 [details]
Function to demonstrate problem
Comment 6 tommy27 2016-04-16 07:24:33 UTC Comment hidden (obsolete)
Comment 7 jan.benda 2016-04-18 19:33:09 UTC
Tested again and problem still occurs on LO version  5.1.2.2, Windows 7 and Windows XP
Comment 8 QA Administrators 2017-05-22 13:25:36 UTC Comment hidden (obsolete)
Comment 9 jan.benda 2017-05-23 06:28:30 UTC
bug is still present

LO Version: 5.3.3.2
ID : 5.3.3-1
OS: Linux 4.9
nation: cs-CZ (en_US.utf8)
Comment 10 QA Administrators 2019-01-17 04:03:00 UTC Comment hidden (obsolete)
Comment 11 Luis 2019-05-08 21:43:30 UTC
Tested with last version.
Bug still present.

Versión: 6.2.3.2 (x64)
Id. de compilación: aecc05fe267cc68dde00352a451aa867b3b546ac
Subprocs. CPU: 4; SO: Windows 10.0; Repres. IU: predet.; VCL: win; 
Configuración regional: es-ES (es_ES); Idioma de IU: es-ES
Calc: CL
Comment 12 Xisco Faulí 2019-08-09 09:24:53 UTC
Dear Jens Carl,
This bug has been in ASSIGNED status for more than 3 months without any
activity. Resetting it to NEW.
Please assigned it back to yourself if you're still working on this.
Comment 14 Andreas Heinisch 2020-01-15 16:21:15 UTC
In order to solve this problem, I would need some clarifications:

- if no options are set, LO accepts optionals, which will not be initialized:
function TestOpt(optional num as double) MsgBox num end function


- with option compatible, LO accepts optionals with default values. Missing values will not be initialized:
function TestOpt(optional num as double) MsgBox num end function

- with option VBA support, LO accepts optionals with default values, which are initialized with their default values, e.g., an empty string or 0 for integer.

Are these statements correct?
Comment 15 Mike Kaganski 2020-01-15 17:20:23 UTC
(In reply to Andreas Heinisch from comment #14)

Your question is unclear. Are you asking about status quo, or about intended behaviour?

If the former, then your statements (at least first and last) are incorrect. If the latter, then first and last seem correct (assuming that VBASupport should mimic VBA, because VBA behaves like you wrote: initializes missing optional arguments without explicit defaults with type-default values); but I cannot comment on option Compatible - because I just don't know what this mode is, and if there is some documentation about what it is intended for.
Comment 16 Andreas Heinisch 2020-01-15 17:34:14 UTC
I am talking about the intended behaviour. 

For the option compatible I refer to https://help.libreoffice.org/6.3/en-US/text/sbasic/shared/compatible.html and to https://documentation.libreoffice.org/assets/Uploads/Documentation/en/MACROS/RefCards/LibOBasic-5-ExecLib-Flat-A4-EN-v101.pdf

Are these two options the same?
Comment 17 Mike Kaganski 2020-01-15 18:21:18 UTC
(In reply to Andreas Heinisch from comment #16)

So based on those two pages, it looks like your second statement should also be true ... I wish there was some formal spec on that :-(

But note, that only optional arguments with types other than Variant should be initialized in VBASupport mode.
Comment 18 LibreOfficiant 2020-01-15 19:12:21 UTC
(In reply to Andreas Heinisch from comment #16)
> I am talking about the intended behaviour. 
> 
> For the option compatible I refer to
> https://help.libreoffice.org/6.3/en-US/text/sbasic/shared/compatible.html
> and to
> https://documentation.libreoffice.org/assets/Uploads/Documentation/en/MACROS/
> RefCards/LibOBasic-5-ExecLib-Flat-A4-EN-v101.pdf
> 
> Are these two options the same?

Hi 

I would definitely expect optional parameters 'default values' to be supported when 'Option Compatible' is set. 'Option VBA Support 1' enforces Option Compatible as per Mike's statement (somewhere in ask.libO.org) based on his .cxx readings.

I created "Option Compatible" - https://help.libreoffice.org/6.5/en-US/text/sbasic/shared/03103350.html?&DbPAR=BASIC - and amended other 'Option xx' 6.5 help pages according to "Compatibility of Star/OpenOffice Basic to VBA" found at https://jezzper.com/jezzper/discussions.nsf/0/3C030B7CC3F24D5FC1256F65002BC1B5.

I noticed that named arguments are supported when introducing 'Option Compatible'.

Summin up: It looks like 'Opt Compatible' adds VBA syntax sugar at COMPILE time but does not necessarily modifies LibO Basic functions' behaviour (to be checked). While CompatibilityMode(bool) and 'Opt VBASupport 1' both affect RUNTIME.

Enum statement and Err object come as extras with 'VBASupport 1'

As for MS Optional doc, it's here:
https://docs.microsoft.com/en-us/dotnet/visual-basic/programming-guide/language-features/procedures/optional-parameters
Comment 19 Mike Kaganski 2020-01-16 06:32:09 UTC
(In reply to LibreOfficiant from comment #18)
> As for MS Optional doc, it's here:
> https://docs.microsoft.com/en-us/dotnet/visual-basic/programming-guide/
> language-features/procedures/optional-parameters

Please don't confuse Visual Basic and VBA. These are two distinct languages, with differencies e.g. right at that page, which requires each optional to have an explisitly specified default for VB, while it's not required for VBA (where the implicit default value is the one assigned when constructing a new value using DIM, and even then it's different for Variant, where IsMissing is useful).

The actual VBA documentation is 

https://docs.microsoft.com/en-us/office/vba/language/reference/keywords-visual-basic-for-applications

and

https://docs.microsoft.com/en-us/office/vba/language/reference/user-interface-help/function-statement
Comment 21 Mike Kaganski 2020-01-16 06:49:33 UTC
I suppose that in the absence of formal specification, "Option Compatible" could be some kind of superset of original LibreOffice/OOo Basic, so that each correct LO Basic is also correct Option Compatible Basic (and working the same!), while Option VBASupport might change things incompatibly, so that some correct LO Basic programs could be not well-formed in VBASupport, or if valid, they could behave differently: like in this issue, where

> Sub foo (Optional a As String)
> MsgBox IsMissing(a)
> End Sub

would output "True" for a call like

> foo

in plain LO Basic and in Option Compatible Basic, but would output False in VBA and Option VBASupport Basic.
Comment 22 Andreas Heinisch 2020-01-16 07:58:17 UTC
The more I dig into this problem, the more I get confused, for instance, Option Compatible without Option VBASupport has 
no impact on the runtime. It just changes the behaviour of the parser:
https://opengrok.libreoffice.org/xref/core/basic/source/classes/sbxmod.cxx?r=91937535#1090
https://opengrok.libreoffice.org/xref/core/basic/source/classes/sbxmod.cxx?r=91937535#1170

Therefore, I cannot check the compatibility mode in the StepParam function:
https://opengrok.libreoffice.org/xref/core/basic/source/runtime/runtime.cxx?r=175a2063#4023

Is this an additional bug? Do they have to appear together?
Comment 23 Andreas Heinisch 2020-01-16 08:08:12 UTC
The same applies to Option Base:
https://opengrok.libreoffice.org/xref/core/basic/source/runtime/methods1.cxx?r=9e0b3423#721

I will definitely pass Option Compatible to the runtime in order to handle optional parameters correct.
Comment 24 Mike Kaganski 2020-01-16 08:24:42 UTC
(In reply to Andreas Heinisch from comment #22)
> Therefore, I cannot check the compatibility mode in the StepParam function:
> https://opengrok.libreoffice.org/xref/core/basic/source/runtime/runtime.
> cxx?r=175a2063#4023

Why do you need to check Option Compatible *here*? It should not affect this specific runtime behaviour - as discussed above: it should behave exactly like plain LO Basic in this step.
Comment 25 Mike Kaganski 2020-01-16 08:26:47 UTC
(In reply to Andreas Heinisch from comment #23)
> The same applies to Option Base:
> https://opengrok.libreoffice.org/xref/core/basic/source/runtime/methods1.
> cxx?r=9e0b3423#721

This is offtopic here; yet - how Option Base should differ for Option Compatible at runtime from plain LO Basic?
Comment 26 Andreas Heinisch 2020-01-16 08:49:27 UTC
(In reply to Mike Kaganski from comment #24)
> (In reply to Andreas Heinisch from comment #22)
> > Therefore, I cannot check the compatibility mode in the StepParam function:
> > https://opengrok.libreoffice.org/xref/core/basic/source/runtime/runtime.
> > cxx?r=175a2063#4023
> 
> Why do you need to check Option Compatible *here*? It should not affect this
> specific runtime behaviour - as discussed above: it should behave exactly
> like plain LO Basic in this step.

In LO Base, no default values for optionals are allowed. With Option Compatible they are and will not be initialized to their default values. Or do they? Then I don't need it in StepPARAM. Otherwise, I need the Option Compatible Flag in order to distinguish these cases.

To sum up (intended behaviour):
LO Basic: Optionals allowed, Default values: NO, Initialization: NO
Compatible: Optionals allowed, Default values: YES, Initialization: NO
VBASupport: Optionals allowed, Default values: YES, Initialization: YES

Is this correct? Or is it (intended behaviour):
LO Basic: Optionals allowed, Default values: NO, Initialization: NO
--> Compatible: Optionals allowed, Default values: YES, Initialization: YES
VBASupport: Optionals allowed, Default values: YES, Initialization: YES
Comment 27 Mike Kaganski 2020-01-16 08:57:54 UTC
(In reply to Andreas Heinisch from comment #26)
> In LO Base, no default values for optionals are allowed. With Option
> Compatible they are and will not be initialized to their default values. Or
> do they? Then I don't need it in StepPARAM. Otherwise, I need the Option
> Compatible Flag in order to distinguish these cases.

Please check first. I don't know. Then it will be clear if we need it here. But I have a feeling that *currently* default values are handled somewhere else, and what happens here is only assigning blanks unconditionally, breaking defaults later. I *suppose* that at some stage it just checks if there's an already compiled default, and if it's present, and the argument is absent, it is set to the default (thus setting arguments to blanks here breaks that processing). If this my assumption is correct, then you also don't need to have Compatible at runtime here: because the compiled default would be enough, and you only would need to distinguish between VBASupport and no VBASupport to see if to initialize the optional arguments without defaults to empty values.

> To sum up (intended behaviour):

This:

> LO Basic: Optionals allowed, Default values: NO, Initialization: NO
> Compatible: Optionals allowed, Default values: YES, Initialization: NO
> VBASupport: Optionals allowed, Default values: YES, Initialization: YES
Comment 28 Mike Kaganski 2020-01-16 09:07:09 UTC
(In reply to Mike Kaganski from comment #27)
> But I have a feeling that *currently* default values are handled somewhere
> else, and what happens here is only assigning blanks unconditionally,
> breaking defaults later. I *suppose* that at some stage it just checks if
> there's an already compiled default, and if it's present, and the argument
> is absent, it is set to the default (thus setting arguments to blanks here
> breaks that processing). If this my assumption is correct, then you also
> don't need to have Compatible at runtime here: because the compiled default
> would be enough, and you only would need to distinguish between VBASupport
> and no VBASupport to see if to initialize the optional arguments without
> defaults to empty values.

Ah! I should have looked further at the SbiRuntime::StepPARAM code - starting from line 4060; yes, it behaves just as described. It processes the optionals with defaults there; and it's *there* where VBASupport'ed code should assign blanks to all non-defaulted missing optional arguments.
Comment 29 LibreOfficiant 2020-01-16 09:19:00 UTC
(In reply to Mike Kaganski from comment #19)
> (In reply to LibreOfficiant from comment #18)
> > As for MS Optional doc, it's here:
> > https://docs.microsoft.com/en-us/dotnet/visual-basic/programming-guide/
> > language-features/procedures/optional-parameters
> 
> Please don't confuse Visual Basic and VBA. These are two distinct languages,
> with differencies e.g. right at that page, which requires each optional to
> have an explisitly specified default for VB, while it's not required for VBA
> (where the implicit default value is the one assigned when constructing a
> new value using DIM, and even then it's different for Variant, where
> IsMissing is useful).
> 
> The actual VBA documentation is 
> 
> https://docs.microsoft.com/en-us/office/vba/language/reference/keywords-
> visual-basic-for-applications
> 
> and
> 
> https://docs.microsoft.com/en-us/office/vba/language/reference/user-
> interface-help/function-statement

Indeed I know this. Thx for correcting this point.
Comment 30 Commit Notification 2020-02-24 11:04:00 UTC
Andreas Heinisch committed a patch related to this issue.
It has been pushed to "master":

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

Renaming variables for upcoming commit for tdf#36737

It will be available in 7.0.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 31 Commit Notification 2020-03-02 09:09:41 UTC
Andreas Heinisch committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/8e323fcacebad1afe9d867b846722a6b9bf20f78

tdf#36737 - Initialize default values of optionals

It will be available in 7.0.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.