Consider C++20 uniform erasure for erase/erase_if

[ivan-cukic]

Hi Daniel, thanks for the patch.

I like the idea. The only problem I see with it is that when C++20 is enabled, non-std types will stop working, unless they provide their own ADL-enabled erase and erase_if. You can easily check this by adding a test that uses QVector (Qt5, Qt6 before 6.1).

One possible solution would be to add an if constexpr in the code that is C++20-only to see if the type can be used with std::erase and fall-back to the old code if it can't be. Something like this:

#if __cplusplus >= 202002L
    using std::erase;
    if constexpr (requires { erase(container, std::forward<Value>(value)); }) {
        return erase(container, std::forward<Value>(value));
    } else
#endif
    ::: // current implementation of erase

BTW, any particular reason for allowing ADL here with using std::erase? I don't have much against ADL, but I know a lot of people who do ;)

[daniel-petrovic]

Hi Ivan, thanks for the comment!

Yes, it would not work with the non-std types and older Qt versions, so I've made a 2nd proposal.

Regarding ADL: the newer Qt versions also provide cpp20 style erase/erase_if but it's in a global namespace. See for ex: QList::erase/erase_if. So this will not work with std::erase and that's why ADL could help here in order to invoke these custom non-std implementations.

[ivan-cukic]

Looks Ok now.

Regarding ADL ... Fair enough.

[daniel-petrovic]

great, thanks..

p.s sorry, have accidentally removed 2nd reviewer, but cannot revert

[jesperkdab]

Merged, Thanks for the contribution, and sorry about the long time it took, has been both busy and sick :-)