Bug 130426 - Overflow error for unicode beyond Chr(&H7FFF)
Summary: Overflow error for unicode beyond Chr(&H7FFF)
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: BASIC (show other bugs)
Version:
(earliest affected)
6.4.0.3 release
Hardware: All All
: medium normal
Assignee: Stephan Bergmann
URL:
Whiteboard: target:7.0.0 target:6.4.2 target:7.4....
Keywords: regression
Depends on:
Blocks:
 
Reported: 2020-02-04 12:34 UTC by Andreas Säger
Modified: 2022-01-10 11:00 UTC (History)
2 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 Andreas Säger 2020-02-04 12:34:20 UTC
c = Chr(&F7FFF) 'OK
c = Chr(&F8000) 'Inadmissable type of value. Overflow error

I found https://gerrit.libreoffice.org/plugins/gitiles/core/+diff/ca6ddfcc9385f1c31531eae31dfa81a9dda246f0%5E%21 in the 6.4 release notes.
Comment 1 Xisco Faulí 2020-02-19 17:21:25 UTC
(In reply to Andreas Säger from comment #0)
> c = Chr(&F7FFF) 'OK
> c = Chr(&F8000) 'Inadmissable type of value. Overflow error
> 
> I found
> https://gerrit.libreoffice.org/plugins/gitiles/core/+diff/
> ca6ddfcc9385f1c31531eae31dfa81a9dda246f0%5E%21 in the 6.4 release notes.

Hi Stephan,
Could you please take a look ?
Comment 2 Stephan Bergmann 2020-02-20 10:51:02 UTC
(In reply to Andreas Säger from comment #0)
> c = Chr(&F7FFF) 'OK
> c = Chr(&F8000) 'Inadmissable type of value. Overflow error

Here and in the summary (which I updated now), I assume you mean &H... rather than &F...
Comment 3 Mike Kaganski 2020-02-20 11:54:02 UTC
I am not sure this should work actually. It's incorrect to use Chr with negative value; and "helpful" Basic rules mandate that &F8000 is negative (see tdf#62326). A positive integral value should be passed, and to make it possible, tdf#130476 should be implemented instead, to allow using

Chr(&F8000&)
Comment 4 Stephan Bergmann 2020-02-20 12:02:27 UTC
(In reply to Mike Kaganski from comment #3)
> I am not sure this should work actually. It's incorrect to use Chr with
> negative value; and "helpful" Basic rules mandate that &F8000 is negative
> (see tdf#62326). A positive integral value should be passed, and to make it
> possible, tdf#130476 should be implemented instead, to allow using
> 
> Chr(&F8000&)

"Chr(&H8000&)", I assume.

Still, I guess that existing code using Chr(&H8000) etc. is not too uncommon, and avoiding a regression with the small change of <https://gerrit.libreoffice.org/c/core/+/89090> "tdf#130426 Support Basic Chr(&H8000), ..., Chr(&HFFFF) again" should be rather harmless.  But I don't care too much either way.
Comment 5 Mike Kaganski 2020-02-20 12:25:46 UTC
FTR:

In StarBasic, Chr works differently in Option VBASupport mode (where only 0-255 are valid).

MS documentation [1] mentions that Chr takes *Long*, while our documentation [2] states it takes Integer. Btw: see MS documentation about extended Chr range in some locales.

Possibly it actually makes sense to allow it taking negative values in non-VBASupport mode. Then passing valid positive integers (in range 0 - 65535) would be treated normally, and negatives (in -1 - -32768) mapped to positive values - i.e. what is Stephan is doing in his patch. But shouldn't that be documented then?

[1] https://docs.microsoft.com/en-us/office/vba/Language/Reference/User-Interface-Help/chr-function
[2] https://help.libreoffice.org/6.4/en-US/text/sbasic/shared/03120102.html
Comment 6 Stephan Bergmann 2020-02-20 13:10:04 UTC
(In reply to Mike Kaganski from comment #5)
> Possibly it actually makes sense to allow it taking negative values in
> non-VBASupport mode. Then passing valid positive integers (in range 0 -
> 65535) would be treated normally, and negatives (in -1 - -32768) mapped to
> positive values - i.e. what is Stephan is doing in his patch. But shouldn't
> that be documented then?

<https://gerrit.libreoffice.org/c/core/+/89090/1..2#message-605f7c8dcbf5fa6af15aae4f311a519fc5f32b4b> "Agreed, see <https://gerrit.libreoffice.org/c/help/+/89100> "tdf#130426 Update documentation for Basic Chr and ChrW functions"."
Comment 7 Andreas Säger 2020-02-20 14:38:36 UTC
> Still, I guess that existing code using Chr(&H8000) etc. is not too
> uncommon, and avoiding a regression with the small change of
> <https://gerrit.libreoffice.org/c/core/+/89090> "tdf#130426 Support Basic
> Chr(&H8000), ..., Chr(&HFFFF) again" should be rather harmless.  But I don't
> care too much either way.

It affects the extension compiler by B. Marcelly: https://forum.openoffice.org/en/forum/viewtopic.php?f=47&t=100994
Comment 8 Commit Notification 2020-02-20 15:01:47 UTC
Stephan Bergmann committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/5fc8b470f22bc7a8a5dc9a6bd86a591090abfcf3

tdf#130426 Support Basic Chr(&H8000), ..., Chr(&HFFFF) again

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 9 Commit Notification 2020-02-20 15:02:03 UTC
Stephan Bergmann committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/help/commit/bf2dff69f84ad28836924d3bf311910248aca1f9

tdf#130426 Update documentation for Basic Chr and ChrW functions
Comment 10 Commit Notification 2020-02-21 11:34:54 UTC
Stephan Bergmann committed a patch related to this issue.
It has been pushed to "libreoffice-6-4":

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

tdf#130426 Support Basic Chr(&H8000), ..., Chr(&HFFFF) again

It will be available in 6.4.2.

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 Commit Notification 2020-02-24 09:57:57 UTC
Stephan Bergmann committed a patch related to this issue.
It has been pushed to "libreoffice-6-4":

https://git.libreoffice.org/help/commit/5521fc8d7175abd8fe2b457c677c6fab38114778

tdf#130426 Update documentation for Basic Chr and ChrW functions
Comment 12 Commit Notification 2022-01-07 11:19:11 UTC
Andreas Heinisch committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/260a2036d688afddc6e6e477764e1196cc915194

tdf#130426 - Rename file for tdf#145693

It will be available in 7.4.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 13 Commit Notification 2022-01-10 11:00:53 UTC
Andreas Heinisch committed a patch related to this issue.
It has been pushed to "libreoffice-7-3":

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

tdf#130426 - Rename file for tdf#145693

It will be available in 7.3.0.2.

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.