Bug 130476 - BASIC: Data Type Characters with literals don't affect resulting value type
Summary: BASIC: Data Type Characters with literals don't affect resulting value type
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: BASIC (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: All All
: medium normal
Assignee: Andreas Heinisch
URL:
Whiteboard: target:7.0.0
Keywords:
Depends on:
Blocks: Macro-StarBasic
  Show dependency treegraph
 
Reported: 2020-02-06 06:50 UTC by Mike Kaganski
Modified: 2020-05-10 11:45 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mike Kaganski 2020-02-06 06:50:50 UTC
Consider this code:


> sub typesnames
>   ' Integer % - doesn't work, Long &
>   ' This should print Integer Long, but prints Integer Integer
>   MsgBox TypeName(&hff) & " " & TypeName(&hff&)
> end sub

As mentioned in comment, is should output "Integer Long", but gives "Integer Integer" instead. The trailing data type characters # and & at the literals should explicitly define the type of the literal, instead of using the default handling depending on the value of the number.

The code works in Excel as expected, taking value type characters into account.

The impact of the problem is that it's impossible to explicitly define the size of the value, e.g. when using bitwise operations. Consider the following code:

> Sub ApplyMask
>   a = &h0F0F0F0F&
>   b = &h0000FF00&
>   MsgBox "&h" & Hex(a And b)
> End Sub

The code applies the mask b to number a, and is expected to output "&hF00", but actually outputs "&hF0F0F00", because b is handled as Integer, which is negative, which then gets converted into corresponding negative Long when used in bitwise "And", with all higher bits set, which distorts the mask into &hFFFFFF00&.

Tested with Version: 6.4.0.3 (x64)
Build ID: b0a288ab3d2d4774cb44b62f04d5d28733ac6df8
CPU threads: 12; OS: Windows 10.0 Build 18363; UI render: GL; VCL: win; 
Locale: ru-RU (ru_RU); UI-Language: en-US
Calc: CL

and with OpenOffice.org 3.2.0
OOO320m12 (Build:9483)
Comment 1 Mike Kaganski 2020-02-06 07:01:11 UTC
(In reply to Mike Kaganski from comment #0)
> The trailing data type characters # and & at the literals

Please ignore the "#", which is a leftover from experiments, and is not expected to work (at least, it doesn't in Excel).
Comment 2 Andreas Heinisch 2020-02-06 20:03:51 UTC
Confirmed with

Version: 6.3.3.2 (x64)
Build-ID: a64200df03143b798afd1ec74a12ab50359878ed
CPU-Threads: 6; BS: Windows 10.0; UI-Render: GL; VCL: win; 
Gebietsschema: de-DE (de_DE); UI-Sprache: de-DE
Calc: CL
Comment 3 Mike Kaganski 2020-03-23 21:47:44 UTC
See SbiScanner::NextSym for hex/octal numbers (and no GetSuffixType call there).
Comment 4 Andreas Heinisch 2020-03-24 07:54:14 UTC
I already implemented the change on my local machine, but I'll wait until https://gerrit.libreoffice.org/c/core/+/90858 has been sumbmitted in order to rebase the change on it to avoid some side effects I encountered with SbiOpcode::CONST_.
Comment 5 Mike Kaganski 2020-03-24 07:55:57 UTC
(In reply to Andreas Heinisch from comment #4)
> I already implemented the change on my local machine

Great! Sorry if that disturbed you: I just came across this code and decided to post it as a code pointer in case it would be useful.
Comment 6 Andreas Heinisch 2020-03-24 08:00:01 UTC
Thank you! Do we consider the String type as well in 
basic/source/comp/symtbl.cxx?
Comment 7 Mike Kaganski 2020-03-24 08:07:27 UTC
(In reply to Andreas Heinisch from comment #6)
> Do we consider the String type as well in basic/source/comp/symtbl.cxx?

I didn't check that - but likely yes, since we can declare string variables using something like

> Dim s$

No idea what should it do for things like

> Dim s : s = 123$
Comment 8 Andreas Heinisch 2020-03-24 14:11:31 UTC
https://gerrit.libreoffice.org/c/core/+/90978
Comment 9 Commit Notification 2020-03-27 13:17:34 UTC
Andreas Heinisch committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/384afeaa3df919585d9df1df5b4cf6c93536e319

tdf#130476 - take into account trailing data type characters

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 10 Commit Notification 2020-03-30 06:58:49 UTC
Andreas Heinisch committed a patch related to this issue.
It has been pushed to "master":

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

Cleanup for tdf#130476, tdf#62323 and tdf#62326

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 11 Wolfgang Jäger 2020-05-10 11:45:51 UTC
(In reply to Mike Kaganski from comment #0)
> Consider this code:
> 
> 
> > sub typesnames
> >   ' Integer % - doesn't work, Long &
> >   ' This should print Integer Long, but prints Integer Integer
> >   MsgBox TypeName(&hff) & " " & TypeName(&hff&)
> > end sub
> 
> As mentioned in comment, is should output "Integer Long", but gives "Integer
> Integer" instead. ...

My attention only was drawn to this bug by announcing it fixed in 7.0.

Why did you expect as you did?
I dont't know anything about the usage of type-declaration characters with constants. 
Is there something like a specification?
q& = &hff works as expected.
From my point of view the bug is that a trailing type-declaration character appended to a constant is accepted at all. It should throw an error. 
You may expect to get a Long value by &h00000FF . Hmmm? 
Shall the implicit type be determined by the value or by the syntax? 
Specify it!
Currently you get a Double value for &h100FF. That's a bug?
You cannot tell a bug from a feature if there is no specification.

(The idea of implicit type declarations was a misconception from its beginning with FORTRAN. In the 1950ies there was an excuse, however. Now is none.)

What actually is missing is a conversion function like CLong().

Pascal has a well proven concept insofar: The name of any simple type can be used as the name of the respective conversion function (where applicable). 
Double(1)