Fix various compiler warnings in main branch #3320
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes:
src/widget/wsearchrelatedtracksmenu.cpp:79:5: error: Pass a context object as 2nd singleShot parameter [-Wclazy-connect-3arg-lambda]
addAction(
^
Fixes:
src/widget/wsearchrelatedtracksmenu.cpp:98:5: error: auto deduced to be QStringBuilder instead of QString. Possible crash. [-Wclazy-auto-unexpected-qstringbuilder]
const auto actionTextPrefixWithSeparator =
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
const QString actionTextPrefixWithSeparator
src/widget/wsearchrelatedtracksmenu.cpp:321:13: error: auto deduced to be QStringBuilder instead of QString. Possible crash. [-Wclazy-auto-unexpected-qstringbuilder]
const auto locationPathWithTerminator =
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
const QString locationPathWithTerminator
Fixes:
src/library/dao/playlistdao.cpp:177:9: error: c++11 range-loop might detach Qt container (QList) [-Wclazy-range-loop]
for (const auto& trackId : getTrackIds(playlistId)) {
^
|
Thanks for taking this task, Jan! It was also on my list, but didn't have to time to start it ;) |
Be-ing
reviewed
Nov 15, 2020
src/library/dao/playlistdao.cpp
Outdated
| // TODO: QSet<T>::fromList(const QList<T>&) is deprecated and should be | ||
| // replaced with QSet<T>(list.begin(), list.end()). | ||
| // However, the proposed alternative has just been introduced in Qt | ||
| // 5.14. Until the minimum required Qt version of Mixx is increased, |
There was a problem hiding this comment.
Suggested change
| // 5.14. Until the minimum required Qt version of Mixx is increased, | |
| // 5.14. Until the minimum required Qt version of Mixxx is increased, |
uklotzde
approved these changes
Nov 15, 2020
| @@ -78,6 +78,7 @@ void WSearchRelatedTracksMenu::addTriggerSearchAction( | |||
| elidableTextSuffix); | |||
| addAction( | |||
| mixxx::escapeTextPropertyWithoutShortcuts(elidedActionText), | |||
| this, | |||
There was a problem hiding this comment.
Good catch 👍 This is easy to get wrong. Though here it is not essential, because everything happens in the UI thread.
| @@ -95,7 +96,7 @@ QString WSearchRelatedTracksMenu::elideActionText( | |||
| // This should never fail | |||
| return actionTextPrefix; | |||
| } | |||
| const auto actionTextPrefixWithSeparator = | |||
| const QString actionTextPrefixWithSeparator = | |||
There was a problem hiding this comment.
This warning is a false positive if at least one argument is a QString as is here. But it doesn't hurt to be explicit.
|
Waiting for at least one of the macOS builds to finish, just in case. |
|
@Be-ing I'll fix the typo right before merging. |
|
There are many more of those "Mixx" typos that need to be fixed separately starting with 2.3. |
|
macOS build passed. Merging. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.