Skip to content

clang-tidy: Apply modernize-loop-convert everywhere - #6481

Merged
DomClark merged 20 commits into
LMMS:masterfrom
sakertooth:modernize-loop-convert
Sep 27, 2022
Merged

clang-tidy: Apply modernize-loop-convert everywhere#6481
DomClark merged 20 commits into
LMMS:masterfrom
sakertooth:modernize-loop-convert

Conversation

@sakertooth

@sakertooth sakertooth commented Aug 9, 2022

Copy link
Copy Markdown
Contributor

Extremely useful here in my opinion.

This clang-tidy check transforms for loops into their range-based loop equivalent where applicable. This in particular was ran using the MinConfidence option set to reasonable, which is the default.

Comment thread plugins/GigPlayer/GigPlayer.cpp Outdated
Comment thread plugins/GigPlayer/GigPlayer.cpp Outdated
Comment thread plugins/GigPlayer/GigPlayer.cpp Outdated
Comment thread plugins/LadspaEffect/LadspaControlDialog.cpp Outdated
Comment thread plugins/Lv2Instrument/Lv2Instrument.cpp Outdated
Comment thread plugins/Sf2Player/Sf2Player.cpp Outdated
Comment thread plugins/Sfxr/Sfxr.cpp Outdated
Comment thread plugins/Sfxr/Sfxr.cpp Outdated
Comment thread plugins/Sfxr/Sfxr.cpp Outdated
Comment thread plugins/Sid/SidInstrument.cpp Outdated
Comment thread src/core/AutomationClip.cpp Outdated
Comment thread src/core/ConfigManager.cpp
Comment thread src/core/Song.cpp Outdated
Comment thread src/core/Song.cpp Outdated
sakertooth and others added 6 commits August 10, 2022 23:06
…ument.cpp

Co-authored-by: Dominic Clark <mrdomclark@gmail.com>
…Sfxr.cpp

Co-authored-by: Dominic Clark <mrdomclark@gmail.com>
…fxr.cpp

Co-authored-by: Dominic Clark <mrdomclark@gmail.com>
…ithin plugins/Sfxr/Sfxr.cpp

Co-authored-by: Dominic Clark <mrdomclark@gmail.com>
Co-authored-by: Dominic Clark <mrdomclark@gmail.com>
In the future, if an effort was made to remove unnecessary heap allocations (which is the reason why most classes are a pointer anyways), having "for (auto ..." would make copies. Plus, it will make things even more difficult because "for (auto ..." would still compile, causing a hard to track performance hit if it isn't replaced with "for (auto& ..." right now.

@allejok96 allejok96 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks good, just have some optional suggestions.

Comment thread plugins/MidiExport/MidiExport.cpp Outdated
Comment thread src/core/AutomationClip.cpp Outdated
Comment thread src/core/AutomationClip.cpp Outdated
Comment thread src/core/Mixer.cpp Outdated
Comment thread src/gui/MixerView.cpp Outdated
Comment thread src/tracks/PatternTrack.cpp Outdated
Comment thread src/core/Piano.cpp Outdated
@JohannesLorenz

Copy link
Copy Markdown
Contributor

I originally considered reviewing this, but it already has 2 reviews, which should be enough.

@sakertooth Can you please fix the merge conflicts?

@sakertooth

Copy link
Copy Markdown
Contributor Author

I originally considered reviewing this, but it already has 2 reviews, which should be enough.

@sakertooth Can you please fix the merge conflicts?

Will do.

@DomClark
DomClark merged commit 2f7a655 into LMMS:master Sep 27, 2022
tresf pushed a commit that referenced this pull request Oct 1, 2022
Fix bug that shows only one effect in the effect chain
@sakertooth
sakertooth deleted the modernize-loop-convert branch December 8, 2022 05:26
sakertooth added a commit to sakertooth/lmms that referenced this pull request May 30, 2023
Co-authored-by: allejok96 <allejok96@gmail.com>
Co-authored-by: Dominic Clark <mrdomclark@gmail.com>
sakertooth added a commit to sakertooth/lmms that referenced this pull request May 30, 2023
Fix bug that shows only one effect in the effect chain
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants