This is one of two related bug reports (the other is bug 133887); attachment 161869 [details] contains the tests for both. Please run TestByRefFromInvoke, and only check last two lines of output (OutputByRef3 and OutputByRef4) for the purposes of this bug. If there is a routine with a ByRef parameter (i.e., inout parameter), invoking it using XScript::invoke [1] requires passing *in* values through the first array (sequence), and getting their modified *out* values in the last passed array. This bug is for explicit Long and Double parameters: > Sub Foo(v As Long) ' default ByRef > v = 100000 > End Sub > Sub TestInvoke > oScript = thisComponent.getScriptProvider().getScript("vnd.sun.star.script:Standard.Module1.Foo?language=Basic&location=document") > oScript.invoke(Array(0&), aOutInd, aOutArr) > MsgBox aOutArr(0) & " " & TypeName(aOutArr(0)) > End Sub TestInvoke above passes "0&" (i.e., an explicit Long value) as initial value of the variable passed to the Foo, and expects to get the modified value in the aOutArr(0) - i.e., it should output "100000 Long". However, the actual returned value is 0 of type *Integer*. It is possible to get the correct result by modifying the value passed to invoke call like this: > oScript.invoke(Array(200000), aOutInd, aOutArr) So it is not enough to pass an explicitly-typed long value; it turns into a smaller type if its value fits that smaller value. The conversion was introduced in https://git.libreoffice.org/core/+/11f9aa4fcb2ad0a5fada8561c7041e7f25a059fc, and was made conditional (dependent on VBASupport) in https://git.libreoffice.org/core/+/9cdb73ff28c4cd6380412468f34ff10e46292a07. The reason for the conversion is expressed as "Choose "smallest" represention for int values because up cast is allowed, downcast not". I *suppose* that the idea was that it's possible to call some UNO API like this: > Dim n As Long > n = 1 > oSomeObject.AMethodExpecting16BitValue(n) ... which would fail if value of n would be passed as 32-bit integer to the method. Thus, I suppose that removal of the conversion could be an incompatible change breaking infinite number of existing macros (so unacceptable; yet, I don't have a proof of this suspicion). I also don't know if it's possible to find a proper condition to disallow downcasting for the purposes of ByRef parameters.
(In reply to Mike Kaganski from comment #0) > This is one of two related bug reports (the other is bug 133887); attachment > 161869 [details] contains the tests for both. Please run > TestByRefFromInvoke, and only check last two lines of output (OutputByRef3 > and OutputByRef4) for the purposes of this bug. > > If there is a routine with a ByRef parameter (i.e., inout parameter), > invoking it using XScript::invoke [1] requires passing *in* values through > the first array (sequence), and getting their modified *out* values in the > last passed array. This bug is for explicit Long and Double parameters: > > > Sub Foo(v As Long) ' default ByRef > > v = 100000 > > End Sub > > > Sub TestInvoke > > oScript = thisComponent.getScriptProvider().getScript("vnd.sun.star.script:Standard.Module1.Foo?language=Basic&location=document") > > oScript.invoke(Array(0&), aOutInd, aOutArr) > > MsgBox aOutArr(0) & " " & TypeName(aOutArr(0)) > > End Sub > > TestInvoke above passes "0&" (i.e., an explicit Long value) as initial value > of the variable passed to the Foo, and expects to get the modified value in > the aOutArr(0) - i.e., it should output "100000 Long". > > However, the actual returned value is 0 of type *Integer*. It is possible to > get the correct result by modifying the value passed to invoke call like > this: > > > oScript.invoke(Array(200000), aOutInd, aOutArr) > > So it is not enough to pass an explicitly-typed long value; it turns into a > smaller type if its value fits that smaller value. > > The conversion was introduced in > https://git.libreoffice.org/core/+/11f9aa4fcb2ad0a5fada8561c7041e7f25a059fc, > and was made conditional (dependent on VBASupport) in > https://git.libreoffice.org/core/+/9cdb73ff28c4cd6380412468f34ff10e46292a07. > The reason for the conversion is expressed as "Choose "smallest" > represention for int values because up cast is allowed, downcast not". I > *suppose* that the idea was that it's possible to call some UNO API like > this: > > > Dim n As Long > > n = 1 > > oSomeObject.AMethodExpecting16BitValue(n) > > ... which would fail if value of n would be passed as 32-bit integer to the > method. Thus, I suppose that removal of the conversion could be an > incompatible change breaking infinite number of existing macros (so > unacceptable; yet, I don't have a proof of this suspicion). I also don't > know if it's possible to find a proper condition to disallow downcasting for > the purposes of ByRef parameters. I run your `TestByRefFromInvoke` and got this Message Box: OutputByRef0: [ (Empty)] [Error 91 (Error)] [6.75 (String)] [True (Boolean)] [7 (Integer)] [7 (Integer)] [7 (Integer)] [7 (Integer)] (Ref: 6.75) OutputByRef1: [ (Empty)] [Error 91 (Error)] [ (String)] [True (Boolean)] [0 (Integer)] [0 (Integer)] [0 (Integer)] [0 (Integer)] (Ref: True) OutputByRef2: [ (Empty)] [Error 91 (Error)] [ (String)] [False (Boolean)] [1 (Integer)] [1 (Integer)] [1 (Integer)] [1 (Integer)] (Ref: 1) OutputByRef3: [ (Empty)] [Error 91 (Error)] [ (String)] [False (Boolean)] [0 (Integer)] [0 (Integer)] [0 (Integer)] [0 (Integer)] (Ref: 100000) OutputByRef4: [ (Empty)] [Error 91 (Error)] [ (String)] [False (Boolean)] [0 (Integer)] [0 (Integer)] [0 (Integer)] [0 (Integer)] (Ref: 12.5) But I don't understand why this line: oScript.invoke(a1Param, aOutInd, aOutArr) You declared `invoke` like this: Function Invoke(aParams()) As String
I can only see this way to workaround the "Choose "smallest" represention" hack here: the invoke should check the expected byRef type, and if it's an integer type that is managed by the said hack, and the type of passed actual param is smaller, then "upgrade" the value before making it fixed in BasicScriptImpl::invoke.
(In reply to Mike Kaganski from comment #2) > I can only see this way to workaround the "Choose "smallest" represention" > hack here: the invoke should check the expected byRef type, and if it's an > integer type that is managed by the said hack, and the type of passed actual > param is smaller, then "upgrade" the value before making it fixed in > BasicScriptImpl::invoke. From my understanding it seems fine until user purposely passes any number as Integer(not Long) and expecting value not be changed. For example - In line oScript.invoke(Array(0&), aOutInd, aOutArr) user is passing value 0 as a long data type and expecting return value to be 100000. But what if user intentionally passes 0 as Integer like oScript.invoke(Array(0), aOutInd, aOutArr) and expecting return value also to be 0.I mean idk when this case will come but if we fix this bug as mentioned by you it will be like - 1)Getting return value to be 100000 when user passes 0 as Integer. 2)Getting return value to be 100000 when user passes 0 as Long. I also don't know whether my point is valid. But i just want to know can user intentionally expect 0 by passing 0 as Integer ?
(In reply to Tushar Kumar Rai from comment #3) You are absolutely correct that this is what would happen; you understand the data flow perfectly. However, I suppose that this *workaround* (yes, I admit that I suggest a workaround) is very likely an appropriate one. 1. We are facing some code that is intended to avoid *unknown* problem, and reverting it has a huge potential for regressions, which would actually break existing code. 2. On the other hand, the discussed case (using invoke()) implies that the call likely is done across some boundaries - maybe from a different language. In that case, it's unlikely that the caller is passing some value, expects that it is unchanged, *but uses the new value* in the following code - when they must explicitly read the new value from the out array. I believe that a user who calls invoke() would not read out params to get expectedly old value of passed byRef param. Do you agree?
(In reply to Mike Kaganski from comment #4) > (In reply to Tushar Kumar Rai from comment #3) > > 2. On the other hand, the discussed case (using invoke()) implies that the > call likely is done across some boundaries - maybe from a different > language. In that case, it's unlikely that the caller is passing some value, > expects that it is unchanged, *but uses the new value* in the following code > - when they must explicitly read the new value from the out array. I believe > that a user who calls invoke() would not read out params to get expectedly > old value of passed byRef param. Do you agree? Yeah.Makes sense.If user really wants to use the old value user can make copy of the varable before calling invoke ;-).It makes sense to me also. I will start working on this. Thanks.
tushar committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/61d2014254a6bf1da68e2f13d3de2c099fcb8883 tdf#133889 Upcasting the type of actual parameter. It will be available in 7.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.