Bug 43460 - Replace rtl::OUString getLength()==0 with isEmpty() etc.
Summary: Replace rtl::OUString getLength()==0 with isEmpty() etc.
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
Master old -3.6
Hardware: Other All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:4.1.0 target:4.2.0
Keywords: easyHack
Depends on:
Blocks:
 
Reported: 2011-12-02 01:56 UTC by Stephan Bergmann
Modified: 2015-12-18 10:02 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments
plugin script (4.84 KB, text/plain)
2011-12-02 01:56 UTC, Stephan Bergmann
Details
log (1.10 MB, text/plain)
2011-12-02 01:57 UTC, Stephan Bergmann
Details
plugin script (with StringBuffer) (4.99 KB, text/plain)
2013-03-09 20:51 UTC, Thomas Arnhold
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Stephan Bergmann 2011-12-02 01:56:04 UTC
Created attachment 54059 [details]
plugin script

Lots of calls to rtl::OString/rtl::OUString getLength() check whether the string is empty, something that can be expressed more cleanly with calling isEmpty() instead.

The attached emptylength.py can be used with <https://fedorahosted.org/gcc-python-plugin/> to find such calls at compile time.  See the script for further details.

The attached emptylength.txt contains the notes produced by the plugin on a recent unxlngx6 (--enable-dbgutil) master (towards LO 3.5) build.  As mentioned in emptylength.py:  "The re-constructed original comparison and the suggested improvement should be taken with a grain of salt."  And: "Produces false positives [...]"  But it should be a helpful starting point.
Comment 1 Stephan Bergmann 2011-12-02 01:57:26 UTC
Created attachment 54060 [details]
log
Comment 2 Olivier Hallot 2011-12-08 12:01:27 UTC
Hi

The main issue with this Easy Hack is to control the 8451 occurences of the target string, where no automated replacement is recomended (false positives), so the job has to be done with extreme care. No-brainer but very extensive.

My suggestion: chunks per module (e.g. basctl, basic, avmedia, etc...)

Besides a simple compilation/build, is there a way to test the results? The occurence spans all over the application.

Advise welcome
Comment 3 Stephan Bergmann 2011-12-09 02:03:33 UTC
Re comment 2, what do you mean with "the 8451 occurences of the target string, where no automated replacement is recomended (false positives)"?  Can you give a link to one of those 8451 occurrences?
Comment 4 Olivier Hallot 2011-12-09 03:44:30 UTC
Number of lines of the log file in comment#1. AFAIK, each line must be inspected... did I missed something?
Comment 5 Stephan Bergmann 2011-12-09 04:44:21 UTC
Now I got you.  Yes, there are false positives (I found at least one, as documented), and there might be cases where the replacement suggestion's negation-status (i.e., whether or not to prefix isEmpty() with !) is wrong, due to the original getLength() comparison already being re-written by the compiler's optimization steps.

But if this could blindly be done mechanically, it would already have been done so.  :)
Comment 6 Stephan Bergmann 2011-12-09 14:38:45 UTC
<http://cgit.freedesktop.org/libreoffice/core/commit/?id=f43311dfb77342f0d003bee5336215f92500f15c> addresses occurrences in UnoControls, accessibility, and avmedia.
Comment 9 Olivier Hallot 2011-12-12 12:43:11 UTC
Module
basic (small cosmetic fix per demand of Ivan Timofeev)
binaryurp
bridges 

http://cgit.freedesktop.org/libreoffice/core/commit/?id=9201704ede70498a850bee6d15f0340d58f3889c
Comment 15 Olivier Hallot 2011-12-17 05:40:45 UTC
Modules
cppu 
cppuhelper
cpputools 

http://cgit.freedesktop.org/libreoffice/core/commit/?id=1b99d8800e399f45404ab62827163a873d2a1aec
Comment 21 Olivier Hallot 2011-12-23 11:38:10 UTC
modules
drawinglayer, dtrans, editeng

http://cgit.freedesktop.org/libreoffice/core/commit/?id=b575f4b1a2a2217282cddc995951b350936b47b1
Comment 26 Olivier Hallot 2011-12-31 03:22:08 UTC
module
i18npool, idl, idlc, io, javaunohelper, jvmaccess

http://cgit.freedesktop.org/libreoffice/core/commit/?id=a17fb882569046cd9f6940cf2e87435200bb666b
Comment 31 Olivier Hallot 2012-01-02 10:25:26 UTC
Modules 
padmin, pyuno, rdbmaker, regexp, registry, rsc, sal

http://cgit.freedesktop.org/libreoffice/core/commit/?id=85d1ce27ad9ce7a3740bd8bbbaf1d3abe643ba10
Comment 33 Olivier Hallot 2012-01-05 06:37:15 UTC
Modules
jvmfwk, l10ntools, lingucomponent

http://cgit.freedesktop.org/libreoffice/core/commit/?id=c47f3523338b8e58c1ea18cc583064761f60df90
Comment 34 Olivier Hallot 2012-01-06 08:43:46 UTC
Modules 
sax, scaddins, sccomp, scripting

http://cgit.freedesktop.org/libreoffice/core/commit/?id=fffd541c3e626bee162ab4b473b6bd6cd180244e
Comment 37 Olivier Hallot 2012-01-13 09:30:57 UTC
MOdule 
sdext

http://cgit.freedesktop.org/libreoffice/core/commit/?id=f19d269
Comment 38 Olivier Hallot 2012-01-13 09:37:57 UTC
Modules
shell, slideshow, sot, starmath

http://cgit.freedesktop.org/libreoffice/core/commit/?id=806dce17d6631738c7388d8d68d8b5ac2e4c11a8
Comment 39 Olivier Hallot 2012-01-13 09:46:05 UTC
Module 
stoc

http://cgit.freedesktop.org/libreoffice/core/commit/?id=71dc235
Comment 49 Olivier Hallot 2012-02-06 02:51:51 UTC
At this point I have parsed all entries of the log file of comment #1, with the exception of binfilter (which is expected to become deprecated or with no further development).

@ Stephan Bergman: 
Can you run the script once more to generate a last log, just to catch the replacements I may have left? Thank you.
Comment 50 Michael Meeks 2012-02-29 04:40:29 UTC
nice to see this linked in your slides Stephan :-) be even better to double check the last bits & get this closed [ if that's easy ].
Comment 51 Commit Notification 2013-03-09 19:38:49 UTC
Thomas Arnhold committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=a2454a42ad2236f0f6b88be166bc7e6a8f90f036

fdo#43460: use isEmpty()



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 52 Commit Notification 2013-03-09 19:39:09 UTC
Thomas Arnhold committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=d2f57b6c1275e0e6ed7f214d471ddb84ecb1b4ad

fdo#43460: use isEmpty()



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 53 Thomas Arnhold 2013-03-09 20:51:38 UTC
Created attachment 76242 [details]
plugin script (with StringBuffer)
Comment 54 Commit Notification 2013-03-09 20:52:25 UTC
Thomas Arnhold committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=6dddefc6e7ff6dc5cde46dde069ba1d7db4ac34b

fdo#43460: use isEmpty()



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 55 Thomas Arnhold 2013-03-09 20:53:15 UTC
I've reopened it, because it also applies to O(U)StringBuffer now.
Comment 56 Jelle van der Waa 2013-06-15 20:26:31 UTC
Ok I am working on replacing O(U)StringBuffe. I am recompiling LO with the python plugin atm.
Comment 57 Commit Notification 2013-06-16 16:05:08 UTC
Jelle van der Waa committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=040710161c507f6e4d0120cfb61d9d82bc6a0527

fdo#43460 use isEmpty()



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 58 Commit Notification 2013-06-17 15:10:14 UTC
Jelle van der Waa committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=d30df91b1e5ce90826a96e4f494791c0b61b8b7c

fdo#43460 chart2: use isEmpty()



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 59 Commit Notification 2013-06-17 19:39:09 UTC
Jelle van der Waa committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=97460c421abec14150c4ddde27daeef892c86b16

fdo#43460 configmgr: use isEmpty()



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 60 Commit Notification 2013-06-17 19:39:30 UTC
Jelle van der Waa committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=2f731e7a19f2c36da64b9631cf4f2de0f6d2a86e

fdo#43460 startmath,codemaker: use isEmpty()



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 61 Commit Notification 2013-06-17 19:56:53 UTC
Jelle van der Waa committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=9a95669e5035758fc115f4c7be3e0a00651fe993

fdo#43460 dbaccess: use isEmpty()



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 62 Commit Notification 2013-06-17 20:05:12 UTC
Jelle van der Waa committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=af7cfa0071f5b2be33e9b887e6221c5047d21262

fdo#43460 forms: use isEmpty()



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 63 Commit Notification 2013-06-17 20:05:33 UTC
Jelle van der Waa committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=50e6713e40cd239f7e568f00ad7adf44bda3453f

fdo#43460 oox: use isEmpty()



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 64 Commit Notification 2013-06-18 15:18:14 UTC
Chr. Rossmanith committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=f8d1dcc621b04fd17577905e9b1bdf71f33c7b57

fdo#43460: Use isEmpty() instead of getLength() in svgio



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 65 Commit Notification 2013-06-18 17:23:23 UTC
Jelle van der Waa committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=733d88433c560af8f51f010fcaae9ce7a29b0325

fdo#43460 tools: use isEmpty()



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 66 Commit Notification 2013-06-18 17:31:49 UTC
Jelle van der Waa committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=5883e1926b80334cfdb7a3dd63d6391b1738c2a6

fdo#43460 sw: use isEmpty()



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 67 Commit Notification 2013-06-18 17:40:30 UTC
Jelle van der Waa committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=caab53cf21bc38ead3927941795b3c8a1432589a

fdo#43460 connectivity,extensions,filter,idl,idlc: use isEmpty()



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 68 Commit Notification 2013-06-18 17:40:49 UTC
Jelle van der Waa committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=84f1f1d149b6ba95aca8adb7e34b001e102f07fe

fdo#43460 include,registry,svtools,svx,unodevtools: use isEmpty()



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 69 Commit Notification 2013-06-19 10:19:59 UTC
Jelle van der Waa committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=ad09b2f7efac628ac4261b86f9fd085f83ebe717

fdo#43460 xmloff: use isEmpty()



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 70 Commit Notification 2013-06-19 10:20:19 UTC
Jelle van der Waa committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=b5f3f55ce59b400f885c41413a3087e3406a424d

fdo#43460 unoxml,writerfilter,xmlsecurity: use isEmpty()



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 71 Commit Notification 2013-06-24 09:04:29 UTC
Jelle van der Waa committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=34f50399f1bc896849a0e3fc3598ab1225d760c5

fdo#43460 shell,vcl,xmlreader: use isEmpty()



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 72 Commit Notification 2013-06-24 13:21:48 UTC
Jelle van der Waa committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=97f71c5f8be85f47d7978259a2d82708412043fd

fdo#43460 svl: use isEmpty()



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 73 Commit Notification 2013-06-24 21:51:19 UTC
Jelle van der Waa committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=51daa4de4fbb86903aeb9cdfefbb089e8d00c001

fdo#43460 sd,rsc,ucb,sdext: use isEmpty()



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 74 Commit Notification 2013-06-24 21:51:39 UTC
Jelle van der Waa committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=126827b0fdc2277d728d57d4fe68b446fa2f7a08

fdo#43460 framework,i18npool,accessibility: use isEmpty()



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 75 Commit Notification 2013-06-28 12:14:03 UTC
Jelle van der Waa committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=f042e22f535e3143332eb788f908f06900eeb40d

fdo#43460 sc: use isEmpty()



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 76 Commit Notification 2013-07-02 05:38:31 UTC
Chr. Rossmanith committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=82a1d75ee59c46e6bb361b98c520cc4eff2e770c

fdo#43460: Use isEmpty() instead of getLength() in svgio



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 77 Jelle van der Waa 2013-08-08 17:36:53 UTC
I think this easyhack can be closed since the only three occurrences I get when recompiling core. And those occurrences seem valid.

core/basegfx/source/polygon/b2dsvgpolypolygon.cxx:179:17: note: [EmptyLength plugin] replace "getLength() > 0" with "!isEmpty()"
core/svl/source/numbers/zformat.cxx:2492:21: note: [EmptyLength plugin] replace "getLength() == 0" with "isEmpty()"
core/vcl/unx/x11/x11sys.cxx:111:9: note: [EmptyLength plugin] replace "getLength() > 0" with "!isEmpty()"
Comment 78 Stephan Bergmann 2013-08-09 12:41:37 UTC
thanks everybody for working on this
Comment 79 Robinson Tryon (qubit) 2015-12-18 10:02:54 UTC
Migrating Whiteboard tags to Keywords: (EasyHack)
[NinjaEdit]