Bug 124605 - Empty Variant leads to the following operator being ignored
Summary: Empty Variant leads to the following operator being ignored
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: BASIC (show other bugs)
Version:
(earliest affected)
6.0.7.3 release
Hardware: All All
: medium normal
Assignee: Eike Rathke
URL:
Whiteboard: target:6.3.0 target:6.2.4
Keywords: bibisected, bisected
Depends on:
Blocks:
 
Reported: 2019-04-08 09:36 UTC by Zsolt
Modified: 2019-05-05 06:47 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments
the ods has 5 function to demonstrate the failure (11.30 KB, application/vnd.oasis.opendocument.spreadsheet)
2019-04-08 09:39 UTC, Zsolt
Details
TestEmptyParam.xlsm (15.48 KB, application/vnd.ms-excel.sheet.macroEnabled.12)
2019-04-09 16:33 UTC, Oliver Brinzing
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Zsolt 2019-04-08 09:36:29 UTC
Description:
I have macros. The macros becomes variable typed as Variant. If the macro makes a division with the variable then
if X as Variant  is emty  then X/100 is 100.
if X as Variant  ix 0 (null) then X/100 is 0.

If a emty Variant divided by a Number, the output is the divisor.

I upgraded my LibreOffice from 5.2.2 to 6.2.2. The 5.2.2 worked fine. An empty Variant divided by a Number was always 0.



Steps to Reproduce:
1. write a basic macro like this
Public Function FuncA1( X ) as Double
	FuncA1 = X / 100
End Function

2. Call the Function on a Cell (write "=FuncA1(C1)" in to the Cell C2)
3. write 0 in to the input cell, then delete it ...

Actual Results:
If the input cell (C1) is empty, you get the divisor 100.
If the input cell is 0, you get the correct result 0.

Expected Results:
The macro should give always the correct result 0.


Reproducible: Always


User Profile Reset: No



Additional Info:
Comment 1 Zsolt 2019-04-08 09:39:44 UTC
Created attachment 150604 [details]
the ods has 5 function to demonstrate the failure

Here is a little example to show, what is my problem.
Comment 2 Oliver Brinzing 2019-04-08 16:29:38 UTC
> If the input cell is empty, you get the divisor 100.

reproducible with:

Version: 6.0.7.3 (x64)
Build-ID: dc89aa7a9eabfd848af146d5086077aeed2ae4a5
CPU-Threads: 4; BS: Windows 10.0; UI-Render: Standard; 
Gebietsschema: de-DE (de_DE); Calc: 

Version: 6.3.0.0.alpha0+ (x64)
Build ID: 421e6fc3cd2e6fe37afbef341e2d0ad7b8edde37
CPU threads: 4; OS: Windows 10.0; UI render: default; VCL: win; 
Locale: de-DE (de_DE); UI-Language: en-US
Calc: threaded

but *not* with:

Version: 5.4.7.2 (x64)
Build-ID: c838ef25c16710f8838b1faec480ebba495259d0
CPU-Threads: 4; BS: Windows 6.19; UI-Render: Standard; 
Gebietsschema: de-DE (de_DE); Calc: single
Comment 3 Oliver Brinzing 2019-04-08 17:10:20 UTC
this seems to have started with:

https://gerrit.libreoffice.org/plugins/gitiles/core/+/3ed87ba36e59f774917eac49389770da404cbc06

commit 3ed87ba36e59f774917eac49389770da404cbc06	[log]
author	Matti Lehtonen <m-Matti-a.Lehtonen@IKI.Fi>Sat Oct 07 12:56:38 2017 +0300
committer Eike Rathke <erack@redhat.com>	Mon Nov 20 17:08:05 2017 +0100
tree 26e582ec05501c0810ae512ed1e8be6dd89ab15b
parent 66dbd4da3afcadb1393daf9be9cecff71b86509a [diff]

tdf#89216 forward empty cells as empty to BASIC instead of 0.0

This change causes that either empty or unknown cells are forwarded to macros
as empty, not as zero (double).

Change-Id: Ia73bcb2ab48e08f97b46cdb45ae4dc3d21bbffd5
Reviewed-on: https://gerrit.libreoffice.org/43226
Reviewed-by: Eike Rathke <erack@redhat.com>
Tested-by: Eike Rathke <erack@redhat.com>
    sc/source/core/tool/interpr4.cxx[diff]
1 file changed


according to commit message i can imagine the change was intended?
Comment 4 Eike Rathke 2019-04-08 20:46:30 UTC
Isn't that rather a BASIC error if an expression X/100 results in 100 if X is Empty? i.e. ignores the Variant *and* the following operator (same with X-100, for example, the result is 100).

The same behaviour with

Dim X
Sub Main
    print X/100
End Sub

(where Dim X is a so far empty Variant) so how's that supposed to be defined?

FWIW, it can be checked with

Function foo( X )
    If IsEmpty(X) Then
        X = 0
    End If
    foo = X / 100
End Function


Or should we only put Empty if it was a cell range argument, but not for single cells? What does Excel do?
Comment 5 Zsolt 2019-04-09 07:21:39 UTC
MS Excell 2010 on Win 7 Prof makes an implizit type conversion.
I tried it, X / 100 give 0 back also, if the X is empty.

The LO does not work consequent now.

It is OK that X is a Variant, and X is "Empty" and not 0.0
It is also OK that 0.0 != Empty.

But if int("Empty") makes 0, or after
double y = Empty
is y = 0.0
then
X / 100 should make an implicit type conversion.

It should be also logical and OK, 
0.0 != Empty
X / 100 raise an IllegalArgumentException
int(X) should raise also an IllegalArgumentException
This is a little bit stupid logic, but consequent.

But the LO ignores the Empty Variant *and* the following operator, as you wrote.
It is an Error. The Emty should be Emty and not a command: "kill the next operator".
Comment 6 Zsolt 2019-04-09 07:22:03 UTC Comment hidden (obsolete)
Comment 7 Oliver Brinzing 2019-04-09 16:33:24 UTC
Created attachment 150624 [details]
TestEmptyParam.xlsm
Comment 8 Oliver Brinzing 2019-04-09 16:43:54 UTC
just noticed:

enter a string, e.g. "X" in cell c3 =FUNCA1(C3) / c8 =FUNCB1(B8:C8)
-> macro crash (even with LO 5.4), while excel results in #VALUE!

enter string "X" in cell c4/c5/c9:
-> string is converted to double/int 0.

anyway, i think it's good practice to check params before calculation.
Comment 9 Eike Rathke 2019-04-29 21:54:19 UTC
Investigating.
Comment 10 Commit Notification 2019-05-02 20:36:46 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/15c39bb2e75df40c30bcbf789d815376dd2e31ce%5E%21

Resolves: tdf#124605 ditch "if operand 1 is Empty, result is operand 2"

It will be available in 6.3.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 11 Eike Rathke 2019-05-02 20:39:03 UTC
Pending review https://gerrit.libreoffice.org/71705 for 6-2
Comment 12 Commit Notification 2019-05-03 15:11:49 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-6-2":

https://git.libreoffice.org/core/+/0f96df766e2b51ad49fed1e4d3f3d2660f80da12%5E%21

Resolves: tdf#124605 ditch "if operand 1 is Empty, result is operand 2"

It will be available in 6.2.4.

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 13 Oliver Brinzing 2019-05-05 06:47:47 UTC
with

Version: 6.3.0.0.alpha0+ (x64)
Build ID: c2ebd00b653c77985af36accde4b647b5279cb28
CPU threads: 12; OS: Windows 10.0; UI render: GL; VCL: win; 
Locale: de-DE (de_DE); UI-Language: en-US
Calc: threaded

and  empty cell c3/c8, corresponding formula =FUNCA1(C3) / =FUNCB1(B8:C8) result is 0 now