Fix clazy issues on main
#12028
Fix clazy issues on main
#12028
Conversation
|
It looks like all these improvements also apply to 2.4. Did you consider to retarget this PR to 2.4? |
| @@ -73,6 +73,7 @@ void WEffectChainPresetButton::populateMenu() { | |||
| tooltip.append("<br/>"); | |||
| tooltip.append(effectNames.join("<br/>")); | |||
There was a problem hiding this comment.
I am afraid join() is also a reallocation in a loop as well as append() above.
Clear() below deletes all QStrings in the QList(), keeping only the space for the data-pointer.
Getting rid of the whole string list would be better or reuse the QStrings in it.
There was a problem hiding this comment.
I agree, feel free to fix this in a follow-up. At least this should fix the failing CI build after we merged 2.4 to main.
Holzhaus
force-pushed
the
clazy-main
branch
from
ee92757 to
c2b1487
Compare
Almost. No idea why the CI on the clazy PR passed though. I opened #12031. This contains a fix which only applies to |
|
FYI clazy build checks are failing, because #12031 has not been merged yet. But this PR may already be merged independently. |
| const auto sampleRates = m_pSoundManager->getSampleRates(); | ||
| for (const auto& sampleRate : sampleRates) { |
There was a problem hiding this comment.
what warning does this fix? just out of curiosity because I don't see why one would be preferred over the other.
There was a problem hiding this comment.
In the new code the container is const.
Fixes: https://github.com/KDE/clazy/blob/master/docs/checks/README-range-loop-detach.md
There was a problem hiding this comment.
Ah right, I would've preferred std::as_const(m_pSoundManager->getSampleRates()) instead though, without the extra variable. Otherwise it would make me think that sampleRates is used outside the loop as well. Any reason you decided against that?
There was a problem hiding this comment.
It's not possible:
src/preferences/dialog/dlgprefsound.cpp: In constructor ‘DlgPrefSound::DlgPrefSound(QWidget*, std::shared_ptr<SoundManager>, UserSettingsPointer)’:
src/preferences/dialog/dlgprefsound.cpp:71:48: error: use of deleted function ‘void std::as_const(const _Tp&&) [with _Tp = QList<mixxx::audio::SampleRate>]’
71 | for (const auto& sampleRate : std::as_const(m_pSoundManager->getSampleRates())) {
| ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /usr/include/qt/QtCore/qglobal.h:47,
from /usr/include/qt/QtCore/qalgorithms.h:43,
from /usr/include/qt/QtCore/qdebug.h:44,
from /usr/include/qt/QtCore/QDebug:1,
from /home/jan/Projects/mixxx/src/audio/frame.h:3,
from /home/jan/Projects/mixxx/build/CMakeFiles/mixxx-lib.dir/cmake_pch.hxx:5,
from <command-line>:
/usr/include/c++/13.2.1/utility:112:10: note: declared here
112 | void as_const(const _Tp&&) = delete;
| ^~~~~~~~
```There was a problem hiding this comment.
Ah right, of course. thats probably why C++ now supports an init statement...
that has been shot down previously in code review (without good reason IMO) so I'm not suggesting to use it.
for (const auto sampleRates = m_pSoundManager->getSampleRates(); const auto& sampleRate : sampleRates) {
Holzhaus
force-pushed
the
clazy-main
branch
from
c2b1487 to
f8e6666
Compare
Holzhaus
force-pushed
the
clazy-main
branch
from
f8e6666 to
4ee05eb
Compare
|
LGTM! Thank you! |
No description provided.