Bug 148251 - Use std::swap instead of using temporary values
Summary: Use std::swap instead of using temporary values
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: reviewed:2022 target:7.5.0 target:7.6...
Keywords: difficultyBeginner, easyHack, skillCpp, topicCleanup
Depends on:
Blocks: Dev-related
  Show dependency treegraph
 
Reported: 2022-03-29 13:19 UTC by Paris Oplopoios
Modified: 2024-02-06 16:48 UTC (History)
6 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 Paris Oplopoios 2022-03-29 13:19:42 UTC
Description:
There's places in code where a swap between two variables happens like this:

T temp = a;
a = b;
b = temp;

There's no need to clutter code with 3 lines where just one is enough:

std::swap(a, b);

Furthermore, std::swap uses move semantics which can improve performance for big data members.

How to find occurrences:

You can use this regex that currently finds 78 results:

(\w+)[\n\r\s]+(\w+)[\n\r\s]+=[\n\r\s]+(\w+);[\n\r\s]+(\3)[\n\r\s]+=[\n\r\s]+(\w+);[\n\r\s]+(\5)[\n\r\s]+=[\n\r\s]+(\2)

[\n\r\s]+ matches newlines and spaces. This exhaustively searches for any weird combination of spaces between the type/variable names/semicolons

See this https://gerrit.libreoffice.org/c/core/+/132194 for an example

Steps to Reproduce:
-

Actual Results:
-

Expected Results:
-


Reproducible: Always


User Profile Reset: No



Additional Info:
-
Comment 1 Roman Kuznetsov 2022-03-29 13:39:00 UTC
Set to NEW
Comment 2 Commit Notification 2022-08-23 08:46:45 UTC
Liu Hao committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/3f284b6f12e3eb047398998b223663b38a9bfec8

tdf#148251 Use std::swap instead of using temporary values

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 3 Hossein 2022-08-25 12:23:26 UTC
Re-evaluating the EasyHack in 2022

This issue is still relevant, and there are many instances remaining. For instance:

basegfx/source/workbench/gauss.hxx:65

        /* interchange rows 'max' and 'i' */
        for(int k=0; k<cols; ++k)
        {
            temp = matrix[ i*cols + k ];
            matrix[ i*cols + k ] = matrix[ max*cols + k ];
            matrix[ max*cols + k ] = temp;
        }

For this workbench, you have to use the Makeflie in basegfx/source/workbench.
Comment 4 Radhey Parekh 2022-09-02 17:15:54 UTC
I would like to work on this.
Comment 5 Commit Notification 2022-09-15 16:08:03 UTC
Radhey Parekh committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/51a780a26e4fbf339d4136ff0c7d75e01d6fad08

tdf#148251 Use std::swap() instead of using temporary values

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 6 Hossein 2022-10-01 11:08:32 UTC
Resetting the issue status back to NEW.

This is a multi-hacker EasyHack. Multiple people can work on it at the same time. Please work on it without assigning it to yourself.
Comment 7 Commit Notification 2023-03-06 12:39:42 UTC
Supriyo Paul committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/9ba89c54076990b21b8e052fdd8d43bf8688edbe

tdf#148251 Use std::swap instead of using temporary values

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 8 Commit Notification 2023-04-23 15:51:27 UTC
Nabeel Siddiqui committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/52522f4772806d9e74ab7df9cc3e8046f68c5809

tdf#148251 Use std::swap instead of using temporary values

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 9 Commit Notification 2023-05-13 04:37:44 UTC
Dr. David Alan Gilbert committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/389b801cea9c9f3c311dcbe9d32df62a66e5ea4a

tdf#148251 qt: Use std::swap instead of using temporary values

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 10 Commit Notification 2023-05-13 06:42:58 UTC
Dr. David Alan Gilbert committed a patch related to this issue.
It has been pushed to "master":

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

tdf#148251 doccomp: Use std::swap instead of using temporary values

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 Dave Gilbert 2023-05-13 11:17:53 UTC
If anyone is set up for Windows stuff (which I'm not) then maybe:
  embedserv/source/embed/tracker.cxx NormalizeRect
and
  extensions/test/ole/AxTestComponents/Basic.cpp
seems to have a whole bunch of them in the 'inOut' set of methods
 (would it make the inoutObject simpler??)
Comment 12 Commit Notification 2024-01-01 13:28:41 UTC
Luv Sharma committed a patch related to this issue.
It has been pushed to "master":

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

tdf#148251 Use std::swap instead of using temporary values

It will be available in 24.8.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 Devansh Varshney 2024-02-06 16:48:01 UTC
Maybe this is now completed as I am unable to find any occurrence of the swap with the temp variable with the regex running in the vscode -

(\w+)[\n\r\s]+(\w+)[\n\r\s]+=[\n\r\s]+(\w+);[\n\r\s]+(\3)[\n\r\s]+=[\n\r\s]+(\w+);[\n\r\s]+(\5)[\n\r\s]+=[\n\r\s]+(\2)

With the above regex there were only 17 results in the single file -libreoffice/cli_ure/qa/climaker/testobjects.cs

moreover I also tried this in my terminal -

git grep -E "(\w+)[[:space:]]*=[[:space:]]*(\w+)[[:space:]]*;[[:space:]]*(\w+)[[:space:]]*=[[:space:]]*(\3)[[:space:]]*;[[:space:]]*(\w+)[[:space:]]*=[[:space:]]*(\2)[[:space:]]*;" -- *.cxx

There were no result of this.