Fix iterator issue reported by GLibC debug mode

[cbjeukendrup]

Resolves: failure from https://github.com/musescore/MuseScore/actions/runs/17277108381/job/49038944461?pr=29508

[cbjeukendrup]

@juli27 Could you please review this?

[juli27]

A cleaner (but a bit more involved) fix would be to rewrite the logic in terms of std::remove_if. But this is fine too (once it compiles with GCC 10)

Did not know that reverse iterators worked differently. What fun!

[cbjeukendrup]

Thanks for the hint about static_assert(false)! Will apply that.

About std::remove_if, note that we're working with std::list here, for which std::remove_if is not efficient; and std::list::remove_if doesn't take iterators as arguments. So unless std::remove_if is specialised for std::list iterators, which it isn't as far I know, I think this custom logic is the best we can get, right?

[juli27]

TLDR: The best for performance would be to replace the list with a vector. But short of that, yes, the custom logic is better.

std::list is bad for performance in general. (see this FAQ entry by Bjarne Stroustrup)

My suggestion is:

• don't store data unnecessarily,
• keep data compact, and
• access memory in a predictable manner.

I emphasize the importance of cache effects. In my experience, all but true experts tend to forget those when algorithms are discussed.
And, yes, my recomendation is to use std::vector by default. More generally, use a contiguous representation unless there is a good reason not to.

The performance hit of visiting each element far outweigh the (often) trivial costs of moving elements around in a vector caused by removing. (especially when the contents can be std::memcpyd)