This bug directs us to another bug fix https://bz.apache.org/ooo/show_bug.cgi?id=66055.
Basically the changes made in this bug fix were -

if (xSbxVar->GetType() != SbxVARIANT ){
   xSbxVar->SetFlag( SbxFlagBits::Fixed );
}
where variant bits of variable are made fixed conditionally.

However this change is not needed from my understanding.Since passing the variable by default makes sense only when scope of variable is global.
For example -


function Foo(p as Integer)
    p = 1
end function

Sub TestInvoke
     var = 0
     Foo(var)
     MsgBox var
End Sub

Consider the given code.Executing this code will give value of var = 0. And according to bug fix https://bz.apache.org/ooo/show_bug.cgi?id=66055 it should give value of var =1(which is definitely not happening).The value of var will be equal to 1 only when var will be global variable or it will be type declared inside TestInvoke(and it will always be 1 irrespective of whether we set variant flag bits or not).

If we remove the part of the bug fix where conditional setting of variant flag of variable is done , then Invoking a Basic routine with ByRef Variant parameter will "not" return wrong value type(Float variable will not be rounded off to the Integer value and variable will retain its value).

This will solve the issue.Suggest me if i am missing something else i will submit the patch.

Hi Tushar Kumar Rai!

(In reply to Tushar Kumar Rai from comment #1)
> However this change is not needed from my understanding.Since passing the
> variable by default makes sense only when scope of variable is global.
> For example -
> 
> 
> function Foo(p as Integer)
>     p = 1
> end function
> 
> Sub TestInvoke
>      var = 0
>      Foo(var)
>      MsgBox var
> End Sub
> 
> Consider the given code.Executing this code will give value of var = 0. And
> according to bug fix https://bz.apache.org/ooo/show_bug.cgi?id=66055 it
> should give value of var =1(which is definitely not happening).The value of
> var will be equal to 1 only when var will be global variable or it will be
> type declared inside TestInvoke(and it will always be 1 irrespective of
> whether we set variant flag bits or not).

I am not sure I follow your idea. *Possibly* you are confused by the comment there - "Enable passing by ref" - thinking that the introduced code actually *enables* passing by reference, while in fact, it simply makes sure that variables passed by ref (which was possible also before that patch) retain their correct type. But maybe I just misunderstand?

> This will solve the issue.Suggest me if i am missing something else i will
> submit the patch.

It's better and easier to discuss actual code changes in the gerrit change. There, you show the actual proposed change, and we may comment specific changed lines in the code. Please prepare the change you have in mind, and let us discuss it there. Thanks!

(In reply to Tushar Kumar Rai from comment #1)
> However this change is not needed from my understanding.Since passing the
> variable by default makes sense only when scope of variable is global.

Why? How "global" scope is relevant here? It would be good if you compared two different Basic code snippets that behave differently for you, to see your point. Do you mean that this code:

Global var

function Foo(p as Integer)
    p = 1
end function

Sub TestInvoke
     var = 0
     Foo(var)
     MsgBox var
End Sub

would make MsgBox show 1? It should not, despite var being global in this case.
> 

> according to bug fix https://bz.apache.org/ooo/show_bug.cgi?id=66055 it
> should give value of var =1(which is definitely not happening).

No, passing by ref (the default) has some pre-requisites, and only works (correctly) when the actual passed variable type matches the type of formal argument. If the types do not match, Basic *rightfully* uses ByVal passing.

The above code would work if you declare "var" to be Integer. By default, variables that don't use type characters have Variant type, so you try to pass a Variant to a function taking Integer; Basic sees that mismatch, and uses ByVal instead of ByRef.

> The value of
> var will be equal to 1 only when var will be global variable or it will be
> type declared inside TestInvoke(and it will always be 1 irrespective of
> whether we set variant flag bits or not).

Sorry, don't follow this. Do you mean that removing the lines from the discussed commit mentioned in comment 0, and running *this* code:

function Foo(p as Integer)
    p = 1
end function

Sub TestInvoke
     Dim var As Integer
     var = 0
     Foo(var)
     MsgBox var
End Sub

would still work - showing "1" in the message box? Of course it will, because the discussed patch does not affect the code that is used when you run "TestInvoke" in that Basic code. Please check the sample file attached to the AOO bugzilla issue - it has the actual sample that would break when you remove the discussed code - and that is using ThisComponent.getScriptProvider().getScript(...).invoke(), just like in comment 0.

> If we remove the part of the bug fix where conditional setting of variant
> flag of variable is done , then Invoking a Basic routine with ByRef Variant
> parameter will "not" return wrong value type(Float variable will not be
> rounded off to the Integer value and variable will retain its value).

Of course, as discussed in comment 0, we should make that code *not* executed in *some* cases; specifically, in case of Invoking something that takes Variant.

Hope this makes sense :-)

(In reply to Mike Kaganski from comment #3)
> (In reply to Tushar Kumar Rai from comment #1)
> > However this change is not needed from my understanding.Since passing the
> > variable by default makes sense only when scope of variable is global.
> 
> Why? How "global" scope is relevant here? It would be good if you compared
> two different Basic code snippets that behave differently for you, to see
> your point. Do you mean that this code:
> 
> Global var
> 
> function Foo(p as Integer)
>     p = 1
> end function
> 
> Sub TestInvoke
>      var = 0
>      Foo(var)
>      MsgBox var
> End Sub
> 
> would make MsgBox show 1? It should not, despite var being global in this
> case.
> > 
> 
> > according to bug fix https://bz.apache.org/ooo/show_bug.cgi?id=66055 it
> > should give value of var =1(which is definitely not happening).
> 
> No, passing by ref (the default) has some pre-requisites, and only works
> (correctly) when the actual passed variable type matches the type of formal
> argument. If the types do not match, Basic *rightfully* uses ByVal passing.
> 
Ahhh Okay.I was not declaring the type of the variable and was expecting the value to be 1.Now i understand it.
> The above code would work if you declare "var" to be Integer. By default,
> variables that don't use type characters have Variant type, so you try to
> pass a Variant to a function taking Integer; Basic sees that mismatch, and
> uses ByVal instead of ByRef.
> 
> > The value of
> > var will be equal to 1 only when var will be global variable or it will be
> > type declared inside TestInvoke(and it will always be 1 irrespective of
> > whether we set variant flag bits or not).
> 
> Sorry, don't follow this. Do you mean that removing the lines from the
> discussed commit mentioned in comment 0, and running *this* code:
> 
> function Foo(p as Integer)
>     p = 1
> end function
> 
> Sub TestInvoke
>      Dim var As Integer
>      var = 0
>      Foo(var)
>      MsgBox var
> End Sub
> 
> would still work - showing "1" in the message box? Of course it will,
> because the discussed patch does not affect the code that is used when you
> run "TestInvoke" in that Basic code. Please check the sample file attached
> to the AOO bugzilla issue - it has the actual sample that would break when
> you remove the discussed code - and that is using
> ThisComponent.getScriptProvider().getScript(...).invoke(), just like in
> comment 0.
> 
> > If we remove the part of the bug fix where conditional setting of variant
> > flag of variable is done , then Invoking a Basic routine with ByRef Variant
> > parameter will "not" return wrong value type(Float variable will not be
> > rounded off to the Integer value and variable will retain its value).
> 
> Of course, as discussed in comment 0, we should make that code *not*
> executed in *some* cases; specifically, in case of Invoking something that
> takes Variant.
> 
Understood it now.But what is the difference between normally calling any function or calling through ThisComponent.getScriptProvider().getScript(...).invoke() which leads to different results ?
> Hope this makes sense :-)

(In reply to Tushar Kumar Rai from comment #4)
> what is the difference between normally calling any
> function or calling through
> ThisComponent.getScriptProvider().getScript(...).invoke() which leads to
> different results ?

Very good question.
The difference is that when you call a Basic sub/function from another Basic code directly, it all is managed by the running Basic, while using invoke means marshalling values pack and forth though a generic UNO API, that is language-agnostic, and has no idea about specifics of Basic type system. It may be used e.g. to call Python function from C# code, or the like; so the values are passed as an array of UNO Any values, and the Basic invoke implementation must correctly process the values passed as such Any's into what Basic will handle (SbxVariable) and back. Hence, if invoke applies some incorrect processing to such values, like this:

== Basic 1 ==
 An SbxVariable of type Variant (having no Fixed bit set) is stored into an array passed to invoke
== Basic-to-UNO code (SbUnoObject::Notify) ==
The original Variant value is converted to Any
== UNO-to-Basic code (BasicScriptImpl::invoke) ==
The Any is converted to an SbxVariable *with Fixed bit set* - i.e., of type other than Variant (e.g., Integer), and passed to the called function
== Basic 2 ==
Gets an Integer as the actual parameter to a formal ByRef argument of type Variant. This is allowed, but any value stored in that actual parameter will be converted to Integer type.

Compare this to the processing in normal case:

== Basic 1 ==
 An SbxVariable of type Variant (having no Fixed bit set) is passed to the called function
== Basic 2 ==
Gets a Variant as the actual parameter to a formal ByRef argument of type Variant. Any value stored in that actual parameter will be stored as is, since Variant can hold value of any actual type.

(In reply to Mike Kaganski from comment #5)
> == Basic 1 ==
>  An SbxVariable of type Variant (having no Fixed bit set) is stored into an
> array passed to invoke
> == Basic-to-UNO code (SbUnoObject::Notify) ==
> The original Variant value is converted to Any

One more thing to note here is that we can't pass a true "Variant" to the invoke here as Any. Its type will become the type of actual value stored in the original variable converted to Any. E.g., both Variant holding 1, and fixed-type Integer holding 1, will arrive as Any with value of type BYTE. So there's no way to reliably detect on the receiving side that a Variant was passed originally.

This is reflected by the "The introduced code in practice made the variable fixed unconditionally, since unoToSbxValue does not return an SbxVARIANT" in comment 0.

> == UNO-to-Basic code (BasicScriptImpl::invoke) ==
> The Any is converted to an SbxVariable *with Fixed bit set* - i.e., of type
> other than Variant (e.g., Integer), and passed to the called function
> == Basic 2 ==
> Gets an Integer as the actual parameter to a formal ByRef argument of type
> Variant. This is allowed, but any value stored in that actual parameter will
> be converted to Integer type.

The end result is that we should *assume* on the receiving side that a Variant was passed, *if* Variant is expected. Thus, when calling code *actually* passes a fixed-type value, it still should be treated on the receiving end as if a Variant was passed, this way off-loading the type conversion to the calling side: e.g., calling side passes fixed-type Integer to invoke, but receives a Double, and it should do with the resulting Double whatever it wants in case if it wants to store that in Integer again. In the "normal" case, passing an Integer would make the Double-to-Integer conversion on the called function side, because it knows exactly, which type of variable was actually passed.

(In reply to Mike Kaganski from comment #6)
> (In reply to Mike Kaganski from comment #5)
> > == Basic 1 ==
> >  An SbxVariable of type Variant (having no Fixed bit set) is stored into an
> > array passed to invoke
> > == Basic-to-UNO code (SbUnoObject::Notify) ==
> > The original Variant value is converted to Any
> 
> One more thing to note here is that we can't pass a true "Variant" to the
> invoke here as Any. Its type will become the type of actual value stored in
> the original variable converted to Any. E.g., both Variant holding 1, and
> fixed-type Integer holding 1, will arrive as Any with value of type BYTE. So
> there's no way to reliably detect on the receiving side that a Variant was
> passed originally.
> 
> This is reflected by the "The introduced code in practice made the variable
> fixed unconditionally, since unoToSbxValue does not return an SbxVARIANT" in
> comment 0.
> 
> > == UNO-to-Basic code (BasicScriptImpl::invoke) ==
> > The Any is converted to an SbxVariable *with Fixed bit set* - i.e., of type
> > other than Variant (e.g., Integer), and passed to the called function
> > == Basic 2 ==
> > Gets an Integer as the actual parameter to a formal ByRef argument of type
> > Variant. This is allowed, but any value stored in that actual parameter will
> > be converted to Integer type.
> 
> The end result is that we should *assume* on the receiving side that a
> Variant was passed, *if* Variant is expected. Thus, when calling code
> *actually* passes a fixed-type value, it still should be treated on the
> receiving end as if a Variant was passed, this way off-loading the type
> conversion to the calling side: e.g., calling side passes fixed-type Integer
> to invoke, but receives a Double, and it should do with the resulting Double
> whatever it wants in case if it wants to store that in Integer again. In the
> "normal" case, passing an Integer would make the Double-to-Integer
> conversion on the called function side, because it knows exactly, which type
> of variable was actually passed.

So the whole point of this is that when a function is invoked through ThisComponent.getScriptProvider().getScript(...).invoke() we need to ensure that the function receives the variant(not the actual type like Integer) as happens in normal case.Right ?

(In reply to Tushar Kumar Rai from comment #7)
> So the whole point of this is that when a function is invoked through
> ThisComponent.getScriptProvider().getScript(...).invoke() we need to ensure
> that the function receives the variant(not the actual type like Integer) as
> happens in normal case.Right ?

*Only* in case when the function *expects* Variant. So we need to check here what type of parameter the called function expect here, just as SbiRuntime::SetParameters does.

(In reply to Mike Kaganski from comment #8)
> (In reply to Tushar Kumar Rai from comment #7)
> > So the whole point of this is that when a function is invoked through
> > ThisComponent.getScriptProvider().getScript(...).invoke() we need to ensure
> > that the function receives the variant(not the actual type like Integer) as
> > happens in normal case.Right ?
> 
> *Only* in case when the function *expects* Variant. So we need to check here
> what type of parameter the called function expect here, just as
> SbiRuntime::SetParameters does.

But in normal case it doesn't matter what function is expecting as mentioned in comment 5 "An SbxVariable of type Variant (having no Fixed bit set) is passed to the called function"?
Thanks.

(In reply to Tushar Kumar Rai from comment #9)
> But in normal case it doesn't matter what function is expecting as mentioned
> in comment 5 "An SbxVariable of type Variant (having no Fixed bit set) is
> passed to the called function"?

Comment 5 was based on a specific called problematic example, namely that is described in comment 0:

> Sub Foo(v) ' default ByRef As Variant
>     v = 6.75
> End Sub

That comment 5 was written in context of question in comment 4:

> But what is the difference between normally calling any
> function or calling through
> ThisComponent.getScriptProvider().getScript(...).invoke() which leads to
> different results ?

... and the latter discussed the *different results* in context of comment 3:
> Of course, as discussed in comment 0, we should make that code *not*
> executed in *some* cases; specifically, in case of Invoking something that
> takes Variant.

Note that "something that takes Variant".

(In reply to Tushar Kumar Rai from comment #9)
> But in normal case it doesn't matter what function is expecting as mentioned
> in comment 5 "An SbxVariable of type Variant (having no Fixed bit set) is
> passed to the called function"?

So again: it does matter what is expected there in normal variant. See again comment #3:
> passing by ref (the default) has some pre-requisites, and only works
> (correctly) when the actual passed variable type matches the type of formal
> argument. If the types do not match, Basic *rightfully* uses ByVal passing.
> 
> The above code would work if you declare "var" to be Integer. By default,
> variables that don't use type characters have Variant type, so you try to
> pass a Variant to a function taking Integer; Basic sees that mismatch, and
> uses ByVal instead of ByRef.

So passing a Variant to a function taking Integer does change its ByRef behavior.

Generally I suggest you to try different combinations of expected and actual types in normal calling case in your Basic, to get the idea. Otherwise, it seems that you try to grasp it only by asking me, not by experimenting.

(In reply to Mike Kaganski from comment #12)
> Generally I suggest you to try different combinations of expected and actual
> types in normal calling case in your Basic, to get the idea. Otherwise, it
> seems that you try to grasp it only by asking me, not by experimenting.

Sorry for lots of questions. I was experimenting many cases already.
Thanks for help.

tushar committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/3b3591d3c127c306f4d5f6bbb34118b03c3124c0

tdf#133887 Set flag of variable when formal parameter is not expecting variant.

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.

This was my first contribution to Libre Office. Thank You for help.
Looking forward for more such contributions.