Bug 147132 - Flatten Basic function implementations
Summary: Flatten Basic function implementations
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: All All
: lowest enhancement
Assignee: Not Assigned
URL:
Whiteboard: target:7.4.0 target:7.5.0 reviewed:20...
Keywords: difficultyBeginner, easyHack, skillCpp, topicCleanup
Depends on:
Blocks: Dev-related
  Show dependency treegraph
 
Reported: 2022-02-02 14:15 UTC by Mike Kaganski
Modified: 2023-08-09 07:58 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 2022-02-02 14:15:40 UTC
In basic/source/runtime/methods.cxx, there are lots of SbRtl_* functions, that implement various Basic functions and procedures. They typically have an argument check, and do the actual work in 'else' branch of the check.

This easyhack is to flatten those functions to increase readability, by returning from the failed check.

For example:

void SbRtl_Tan(StarBASIC *, SbxArray & rPar, bool)
{
    if (rPar.Count() < 2)
    {
        StarBASIC::Error( ERRCODE_BASIC_BAD_ARGUMENT );
    }
    else
    {
        SbxVariableRef pArg = rPar.Get(1);
        rPar.Get(0)->PutDouble(tan(pArg->GetDouble()));
    }
}

This function may be re-written as

void SbRtl_Tan(StarBASIC *, SbxArray & rPar, bool)
{
    if (rPar.Count() < 2)
        return StarBASIC::Error( ERRCODE_BASIC_BAD_ARGUMENT );

    SbxVariableRef pArg = rPar.Get(1);
    rPar.Get(0)->PutDouble(tan(pArg->GetDouble()));
}
Comment 1 Ramreiso Kashung 2022-02-19 10:10:45 UTC
Reset as NEW; Multi-Hack!
Comment 2 Commit Notification 2022-02-22 08:35:07 UTC
Ramreiso Kashung committed a patch related to this issue.
It has been pushed to "master":

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

tdf#147132: Flatten Basic function implementations

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 3 Commit Notification 2022-04-03 10:33:39 UTC
offtkp committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/6bc187b2eb6b9520c942f232b1abcb4fb29bfa04

tdf#147132 Flatten Basic function implementations

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 4 Commit Notification 2022-05-30 06:46:42 UTC
Roman Kuznetsov committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/43458f83f29067752cfb3df2ccfe0eeb940f4b71

tdf#147132 Flatten Basic function implementation

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 5 Commit Notification 2022-06-03 08:18:19 UTC
Roman Kuznetsov committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/66fe1fa945a161b2f617ab3576ddef23d84148f8

tdf#147132 Flatten Basic function

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 6 Commit Notification 2022-06-03 14:37:28 UTC
Roman Kuznetsov committed a patch related to this issue.
It has been pushed to "master":

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

tdf#147132 Flatten Basic function

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 7 Commit Notification 2022-07-13 12:58:12 UTC
Roman Kuznetsov committed a patch related to this issue.
It has been pushed to "master":

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

tdf#147132 Flatten a Basic function

It will be available in 7.5.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 8 Hossein 2022-08-19 08:23:57 UTC
Re-evaluating the EasyHack in 2022

This enhancement is still relevant. One may search for this statement to find instances of the proposed change in basic/source/runtime/methods.cxx:

    StarBASIC::Error( ERRCODE_BASIC_BAD_ARGUMENT );

The result of this search should be checked to makes sure that there is no other expression after the matching else.

There are somehow similar conditions:

    if ( ... )
    {
        StarBASIC::Error( ERRCODE_BASIC_BAD_ARGUMENT );
        return;
    }

which can also be converted to a shorter form:

    if ( ... )
        return StarBASIC::Error( ERRCODE_BASIC_BAD_ARGUMENT );
Comment 9 Commit Notification 2022-09-05 14:05:37 UTC
Arnaud VERSINI committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/6b3e66cd7a355061bf1dec76bbc4f389b6b60f2d

tdf147132 basic : flaten some functions

It will be available in 7.5.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 2023-01-07 20:29:15 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

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

Resolves: tdf#152917 Add ConvertFromUrl() put result back, tdf#147132 follow-up

It will be available in 7.6.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 Commit Notification 2023-01-07 22:43:28 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-7-5":

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

Resolves: tdf#152917 Add ConvertFromUrl() put result back, tdf#147132 follow-up

It will be available in 7.5.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.
Comment 12 Commit Notification 2023-01-20 16:57:04 UTC
Radhey Parekh committed a patch related to this issue.
It has been pushed to "master":

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

tdf#147132 Flatten Basic function implementations

It will be available in 7.6.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 Buovjaga 2023-03-08 11:21:31 UTC
Sunny: please don't assign this to yourself as this is a multi-dev hack.
Comment 14 sunny 2023-03-08 11:24:21 UTC
Sorry, somehow I missed the multi-hack tag. Thanks for reminding me!
Comment 15 sunny 2023-03-18 17:24:48 UTC
Hi Hossein and Mike

Can you please review my change, gerrit: https://gerrit.libreoffice.org/c/core/+/148519

It's been some time since it's open.
Comment 16 Commit Notification 2023-03-22 10:02:45 UTC
adityasingh22 committed a patch related to this issue.
It has been pushed to "master":

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

tdf#147132: Simplify usage of StarBASIC::Error()

It will be available in 7.6.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 17 Commit Notification 2023-08-09 07:58:40 UTC
sahil committed a patch related to this issue.
It has been pushed to "master":

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

tdf#147132 Flatten Basic function implementations

It will be available in 24.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.