D25039: Fix Clazy performance issues, const

This revision was automatically updated to reflect the committed changes. Closed by commit R241:f2a3a78972b2: Fix Clazy performance issues, const (authored by meven). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25039?vs=69178=69179 REVISION DETAIL

D25039: Fix Clazy performance issues, const

meven updated this revision to Diff 69178. meven added a comment. Rebasing on master REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25039?vs=69121=69178 BRANCH arcpatch-D25039 REVISION DETAIL https://phabricator.kde.org/D25039 AFFECTED FILES

D25039: Fix Clazy performance issues, const

kossebau added a comment. In D25039#557609 , @meven wrote: > Feel free to do the code removal, Done now, as I had the files still open. REPOSITORY R241 KIO BRANCH arcpatch-D25039 REVISION DETAIL https://phabricator.kde.org/D25039

D25039: Fix Clazy performance issues, const

meven added a comment. In D25039#557579 , @kossebau wrote: > Not tested, only read code. Looks good to me. > Please remove the newInstance method in a direct commit before, and drop change from this patch. (If you prefer, can do the remove

D25039: Fix Clazy performance issues, const

kossebau accepted this revision. kossebau added a comment. This revision is now accepted and ready to land. Not tested, only read code. Looks good to me. Please remove the newInstance method in a direct commit before, and drop change from this patch. (If you prefer, can do the remove commit

D25039: Fix Clazy performance issues, const

meven updated this revision to Diff 69121. meven marked 7 inline comments as done. meven added a comment. Review feedback REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25039?vs=69083=69121 BRANCH arcpatch-D25039 REVISION DETAIL

D25039: Fix Clazy performance issues, const

kossebau added inline comments. INLINE COMMENTS > ahmadsamir wrote in ftp.cpp:1376 > IIUC, the compiler will use a temporary object to hold the return of > tempurl.fileName(). > > The temporary is used to initialize filename, and then it's gone: > const QString filename = tempurl.filename(); >

D25039: Fix Clazy performance issues, const

ahmadsamir added inline comments. INLINE COMMENTS > kossebau wrote in ftp.cpp:1376 > `QUrl::fileName()` returns a value QString, so just > > const QString filename = empurl.fileName(); > > While > > const QString = empurl.fileName(); > > also is fine code IIRC, as I once learned to my

D25039: Fix Clazy performance issues, const

kossebau added inline comments. INLINE COMMENTS > ftp.cpp:1376 > QString parentDir; > -QString filename = tempurl.fileName(); > +const QString = tempurl.fileName(); > Q_ASSERT(!filename.isEmpty()); `QUrl::fileName()` returns a value QString, so just const QString filename

D25039: Fix Clazy performance issues, const

meven added a comment. In D25039#557004 , @anthonyfieroni wrote: > Not using references is not a big problem with QString nor QUrl since they are reference counting objects, say if you don't change their content they are immutable, so > >

D25039: Fix Clazy performance issues, const

anthonyfieroni added a comment. Not using references is not a big problem with QString nor QUrl since they are reference counting objects, say if you don't change their content they are immutable, so const QString s = other; // it's fine void func(QString s) { const

D25039: Fix Clazy performance issues, const

meven updated this revision to Diff 69083. meven edited the summary of this revision. meven added a comment. amend commit message REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25039?vs=69082=69083 BRANCH arcpatch-D25039 REVISION DETAIL

D25039: Fix Clazy performance issues, const

meven edited the summary of this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D25039 To: meven, #frameworks, dfaure Cc: kossebau, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25039: Fix Clazy performance issues, const

meven edited the summary of this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D25039 To: meven, #frameworks, dfaure Cc: kossebau, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25039: Fix Clazy performance issues, const

meven updated this revision to Diff 69082. meven edited the summary of this revision. meven added a comment. amend commit message REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25039?vs=69081=69082 BRANCH arcpatch-D25039 REVISION DETAIL

D25039: Fix Clazy performance issues, const

meven retitled this revision from "Fix Clazy performance issues, const &, noexcept" to "Fix Clazy performance issues, const &". REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D25039 To: meven, #frameworks, dfaure Cc: kossebau, kde-frameworks-devel, LeGast00n, GB_2,

D25039: Fix Clazy performance issues, const &, noexcept

meven updated this revision to Diff 69081. meven marked 5 inline comments as done. meven added a comment. Remove noexcept changes, avoid touching kpac, some code formatting REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25039?vs=68955=69081 BRANCH

D25039: Fix Clazy performance issues, const &, noexcept

kossebau added a comment. As excuse for bad drive-by comment, here now hopefully making up a bit by giving some in-detail review, please see in-line comments. No idea about exceptions. I would the noexcept also make a different commit, for more separation of concerns. When looking

D25039: Fix Clazy performance issues, const &, noexcept

meven marked 2 inline comments as done. meven added a comment. If somebody could accept this before KF 5.64 tagging, it would to modify `SlaveBase::configValue` signature that were added for the same value. I don't think any of this is subject to much concern. I may commit it without

D25039: Fix Clazy performance issues, const &, noexcept

kossebau added inline comments. INLINE COMMENTS > kossebau wrote in slavebase.h:351 > Would be a proper change, but sadly also changes the signature of a API under > ABI promise, so here and in all other public API places not possibe. > Can be only done when breaking the ABI with KF6. > >

D25039: Fix Clazy performance issues, const &, noexcept

kossebau added a comment. (Quick drive-by comment only as I had this in a search result...) INLINE COMMENTS > slavebase.h:351 > */ > -bool configValue(QString key, bool defaultValue) const; > +bool configValue(const QString , bool defaultValue) const; > Would be a proper

D25039: Fix Clazy performance issues, const &, noexcept

meven created this revision. meven added reviewers: Frameworks, dfaure. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. meven requested review of this revision. REVISION SUMMARY - Fix a lot of missing const & or &, avoiding copying mostly QString. - Add