Bug 132347

Summary: BASIC VBASupport=1 erroneously inherited by StrComp() in module without VBASupport
Product: LibreOffice Reporter: Jean-Pierre Ledure <jp>
Component: BASICAssignee: Not Assigned <libreoffice-bugs>
Status: NEW ---    
Severity: normal CC: elmau, himajin100000, jp, mentoring, xiscofauli
Priority: medium Keywords: difficultyMedium, easyHack, skillCpp
Version: 6.4.3.2 release   
Hardware: All   
OS: All   
Whiteboard:
Crash report or crash signature: Regression By:
Bug Depends on:    
Bug Blocks: 107659    
Attachments: Small test case
Small test case (TestStrComp.odt)

Description Jean-Pierre Ledure 2020-04-23 10:28:46 UTC 
Description:
When I run next Basic script:

Sub CompStr
Dim MyStr1, MyStr2, MyComp
MyStr1 = "ABCD": MyStr2 = "abcd"
MyComp = StrComp(MyStr1, MyStr2, 1)    ' Returns 0 (VBA) / -1 (no VBA)
MyComp = StrComp(MyStr1, MyStr2, 0)    ' Returns -1 (VBA) / 0 (no VBA)
MyComp = StrComp(MyStr2, MyStr1)    ' Returns 1.
End Sub

the behaviour of the StrComp() builtin function, with its "Compare" argument set, is different depending on the presence/absence on the top of the module of

Option VBASupport 1

See the comments after each line above. This is already annoying but I don't see how to correct this without breaking backward compatibility with many scripts.

However, THE REAL NEW BUG IS HERE:

When I store above script in a module WITHOUT VBASupport set, and I call it from another module like this:

Option VBASUpport 1
Sub Comp_Str_Call
	CompStr
End Sub

the CompStr routine bahaves as if the VBASupport in its module was set to 1, which is not the case.



Steps to Reproduce:
1. Open TestStrComp.odt file
2. Open Basic IDE
3. 2 modules: NOVBA and VBA resp. with/without VBASupport
4. Step into (F8) the 3 proposed Subs
	CompStr_NOVBASUPPORT in module NOVBA
	CompStr_VBASUPPORT in module VBA
	CompStr_Call_NOVBASUPPORT
while watching the variable MyComp in the watch window

Actual Results:
When running CompStr_Call_NOVBASUPPORT() the called sub CompStr_NOVBASUPPORT() computes the results of the StrComp() function as if Option VBASupport 1 was present.

Expected Results:
The results of StrComp() should be computed without VBASupport


Reproducible: Always


User Profile Reset: No



Additional Info:
Version: 6.4.3.2
Build ID: 747b5d0ebf89f41c860ec2a39efd7cb15b54f2d8
CPU threads: 4; OS: Linux 4.15; UI render: default; VCL: kf5; 
Locale: fr-BE (en_US.UTF-8); UI-Language: en-US
Calc: threaded
Comment 1 Jean-Pierre Ledure 2020-04-23 10:32:10 UTC 
Created attachment 159842 [details]
Small test case
Comment 2 Jean-Pierre Ledure 2020-04-23 10:58:25 UTC 
Created attachment 159843 [details]
Small test case (TestStrComp.odt)
Comment 3 libre officer 2020-06-17 12:55:34 UTC 
I confirm this :

> When I store above script in a module WITHOUT VBASupport set, and I call it
> from another module like this:
> 
> Option VBASUpport 1
> Sub Comp_Str_Call
> 	CompStr
> End Sub
> 
> the CompStr routine bahaves as if the VBASupport in its module was set to 1,
> which is not the case.

But did you try to explicitly add `Option VBASupport 0` in the beginning of the module WITHOUT VBASupport.
Comment 4 Jean-Pierre Ledure 2020-06-17 15:59:17 UTC 
> But did you try to explicitly add `Option VBASupport 0` in the beginning of the module WITHOUT VBASupport.

The presence or absence of the statement `Option VBASupport 0` in the NOVBA module does not make any difference.

JPL
Comment 5 Mike Kaganski 2021-07-02 18:44:58 UTC 
All over the place, the current compatibility mode is tested in the code like this:

    SbiInstance* pInst = GetSbData()->pInst;
    bool bCompatibility = pInst && pInst->IsCompatibility();

But SbiInstance::IsCompatibility is ~useless, as it's set to some "random" value (unrelated to the currently executed function, current module opened in IDE, and even last built module).

Instead, the code should ask for *current module* to detect its compatibility mode - and the proper method to find the current module seems to be StarBASIC::GetActiveModule.

So the places like SbRtl_StrComp should use something like this:

    SbiInstance* pInst = GetSbData()->pInst;
    SbModule* pMod = StarBASIC::GetActiveModule();
    bool bCompatibility = pMod ? pMod->IsVBACompat() : pInst && pInst->IsCompatibility();

Whoever decides to fix the bug, should make sure to do that function-by-function; and should provide unit tests for each such function. This would require to create a mixed unit test, similar to what currently basic/qa/cppunit/test_vba.cxx does, but its list of tests should possibly contain pairs of source files (one with compat mode, one without) to load them both, and test the resulting interaction.