Bug 136801 - BASIC: CInt("+2") returns 0
Summary: BASIC: CInt("+2") returns 0
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: BASIC (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:7.2.0 target:7.1.0.0.beta2
Keywords: difficultyBeginner, easyHack, skillCpp
Depends on: 123158
Blocks:
  Show dependency treegraph
 
Reported: 2020-09-16 09:20 UTC by Mike Kaganski
Modified: 2020-12-30 10:23 UTC (History)
8 users (show)

See Also:
Crash report or crash signature:
Regression By:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mike Kaganski 2020-09-16 09:20:15 UTC
In BASIC, CInt("+2") returns 0, instead of 2 (the "+" is the problem here). It is both unexpected, and inconsistent with VBA.

Code pointer: ImpScan in basic/source/sbx/sbxscan.cxx.
Comment 1 Xisco Faulí 2020-09-16 11:14:42 UTC
@Andreas, I thought you might be interested in this issue
Comment 2 Alain Romedenne 2020-09-17 08:48:12 UTC
Tests should be performed for all number formats

MsgBox CDbl("+308E45") throws a Basic error which is equally incosistent with VBA.

IsNumeric() function may need to be reviewed too…
Comment 3 Wolfgang Jäger 2020-09-17 12:25:01 UTC
(The teacher's remark:)
Even the subchapter 
5.3 Constant Numbers [of the current specification (Part 4)]
is incomplete and misleading. 
Firstly the semantic remark "..., negative numbers are positive numbers with a prefix "-" (HYPHEN-MINUS, U+002D) operator." is neither well worded nor acceptable as a replacement for the needed syntactical generator. 
Secondly The minus character isn't an operator when used as a sign to write a negative number, but a syntactical part of the number itself. In the same way a plus character may be interpreted if prefixed to an unsigned number. That it has no effect in this case is irrelevant when syntax is addressed.

Calc accepts the following examples as formulas, e.g:
(1:) =5+-----+++-12.34
(2:) =5+-----+++ -12.34
(3:) =5------+++ +12.34
and any similar. 
A thorough description of the situation would require to distinguish 3 different roles of the plus and minus characters:
1. role: Sign like the last minus in (2:)
2. role: Unary operator
2. case "+": Identity semantics
2. case "-": ChangeSign semantics
2. explanation: All the plus or minus in the given example except the first one and the last one in each line can be interpreted this way. Concerning the last one there is a choice (if no whitespace is behind).
3. role: Binary operator 
3. explanation: Exactly the first plus or minus in every line is to interpret this way.
Reading also a plus or minus immediately prefixed to an unsigned number as a unary operator would not cahnge the result. Making this interpretation mandatory would, however not leave any way to write a negative number as a constant, since -5 (e.g.) would only exist as an expression then.  

BNF has means to implicitly decribe the preferred interpretation where not undisputable anyway.

And: 
The Basic function IsNumeric actually behaves faultily regaring the "+" as a sign (my above position applied).
Please note : 
IsNumeric("- 1") --> False is correct!
IsNumeric("-1,555,999") --> True (Locale dependent!) is very bad. 
IsNumeric("-1 555 999") --> False accomplishes this.
How can we get rid of the explicitly deprecated characters misused as GroupSeparators?
Comment 4 An-Kh 2020-11-25 03:36:31 UTC
Hi

IsNumeric("+2")
 
gives false.

I have only worked on the IsNumeric part.
Should I open a new bug for it and report my patch there or should I report it here?
Comment 5 Mike Kaganski 2020-11-25 06:28:39 UTC
(In reply to An-Kh from comment #4)

Hi!
FTR: it's https://gerrit.libreoffice.org/c/core/+/106556
Of course, you are welcome to cover IsNumeric here. However, the task is not to apply separate workarounds to each specific higher-level functions one by one. The task is to find the common (set of) lower-level number parsing functions used by those higher-level functions, and implement a common fix on that lower level.

Thank you for your work!
Comment 6 Commit Notification 2020-11-27 09:15:56 UTC
Anshu committed a patch related to this issue.
It has been pushed to "master":

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

tdf#136801 : IsNumeric("+2") and CInt("+2") return correct values

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.
Comment 7 Alain Romedenne 2020-11-27 12:43:11 UTC
Wolfgang,

Can you provide a link to the quoted documentation?
Comment 8 Wolfgang Jäger 2020-11-27 19:33:45 UTC
(In reply to Alain Romedenne from comment #7)
> Wolfgang,
> 
> Can you provide a link to the quoted documentation?

The Link is https://docs.oasis-open.org/office/OpenDocument/v1.3/cs01/part4-formula/OpenDocument-v1.3-cs01-part4-formula.html#__RefHeading__1017936_715980110.

IMO the definition should read:  

5.3 (rectified) Constant Numbers
Constant numbers are written using '.' (FULL STOP, U+002E) dot as the decimal separator. Optional "E" or "e" denotes scientific notation (exponent separator). Syntactically, negative numbers are unsigned numbers with a prefix "-" (HYPHEN-MINUS, U+002D) operator. Evaluators should also accept the "+" (PLUS SIGN, U+002B) prefixed to and unsigned number. A constant number is of type Number. 
Number::=StandardNumber|[-+]?'.'[0-9]+([eE][-+]?[0-9]+)?
StandardNumber::=[-+]?[0-9]+('.'[0-9]+)?([eE][-+]?[0-9]+)?

Evaluators should be able to read the Number format, which accepts a decimal fraction that starts with decimal point '.' (FULL STOP, U+002E), without a leading zero. Evaluators shall write numbers only using the StandardNumber format, which requires a leading digit, and shall not write numbers with a leading '.' (FULL STOP, U+002E). Whether positive numbers are written with the plus sign or without may be implementation dependent. 

===
Clearer:
Sign::=U002d|U002b
Decimalseparator::=U002e
Exponentseparator::=E|e
Unsignedinteger::=Digit+
Integer::=Sign?Unsignedinteger
Decimalfraction::=DecimalseparatorUnsignedinteger
Exponentpart::=ExponentseparatorInteger
Nicenumber::=IntegerDecimalfraction?Exponentpart?
Allowednumber::=Nicenumber|Sign?DecimalfractionExponentpart?
Comment 9 Commit Notification 2020-11-30 10:14:56 UTC
Anshu committed a patch related to this issue.
It has been pushed to "libreoffice-7-1":

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

tdf#136801 : IsNumeric("+2") and CInt("+2") return correct values

It will be available in 7.1.0.0.beta2.

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 Wolfgang Jäger 2020-11-30 18:41:16 UTC
(In reply to Commit Notification from comment #9)
> Anshu committed a patch related to this issue.
> It has been pushed to "libreoffice-7-1":
> 
> https://git.libreoffice.org/core/commit/
> a1a0f496aa7c39785551b2515b2b64d54a52b524
> 
> tdf#136801 : IsNumeric("+2") and CInt("+2") return correct values
> 
> It will be available in 7.1.0.0.beta2.
> ...

Why wasn't the state changed to RESOLVED then?
Comment 11 Buovjaga 2020-11-30 19:31:10 UTC
(In reply to Wolfgang Jäger from comment #10)
> (In reply to Commit Notification from comment #9)
> > Anshu committed a patch related to this issue.
> > It has been pushed to "libreoffice-7-1":
> > 
> > https://git.libreoffice.org/core/commit/
> > a1a0f496aa7c39785551b2515b2b64d54a52b524
> > 
> > tdf#136801 : IsNumeric("+2") and CInt("+2") return correct values
> > 
> > It will be available in 7.1.0.0.beta2.
> > ...
> 
> Why wasn't the state changed to RESOLVED then?

Because nobody changed it yet.
Comment 12 Buovjaga 2020-12-10 13:29:06 UTC
Anshu: please close this as resolved fixed, so you will be credited in the statistics.