MSVS does not support move semantics for some STL constructs

[hroest]

We are currently facing this issue for

• Map
• CVTermList
• DataProcessing
• Precursor

which all have a member of type std::map, Map, std::set, std::set or inherit from it. Currently its not quite clear how to solve this. this is also related to #3758 and #4276 where the same issue came up and has been discussed in #4290

How to deal with MSVC

The main problem seems to be that MSVC does not make the std::map move constructor noexcept while GCC does [1]. This means that classes that have a std::map member or inherit from it cannot have a noexcept move constructor either, which actually has a performance impact when using these objects in STL containers. Since the STL cannot be sure that the move constructor does not throw, it will use the copy constructor instead when copying around arrays etc. There are two ways to deal with this behavior

1. Only add the noexcept keyword for GCC/Clang and they can profit from the enhanced performance. We then also need to make sure people dont assume that move operations are basically free when they cost on MSVS and are free on other compilers
2. Ignore the fact that MSVC containers are not exception safe and assume that any exception thrown from the move constructor is irrecoverable anyways within OpenMS. The question is under what circumstances would a move constructor throw (mainly out of memory I assume) and can we reasonably recover from those circumstances / do we care to recover? Currently, this approach is taken in Feature/move semantics #3758 does and also seems to be somewhat dangerous. Especially in the context of a GUI application like TOPPView or downstream code using OpenMS code.

Refs

1. https://stackoverflow.com/questions/57299324/why-is-stdmaps-move-constructor-not-noexcept

[hroest]

One worry is that using noexcept is actually leading to a performance loss on MSVS:

#4290 (comment)

@cbielow @hroest I was wondering if we have the choice to override "noexcept"ness if we go back to manually specified constructors (before the changes in #4276). And if so, if we should go this potentially dangerous route. According to https://developercommunity.visualstudio.com/content/problem/425370/c11-noexcept-implementation-still-a-performance-lo.html (and the linked slides) manual noexcept in MSVC can even lead to worse performance in some cases.

I dont think that is a really worry because (i) this only affects the move constructor and (ii) if the move constructor is actually used by MSVC (and not the copy constructor) then we will have a net performance gain even if the noexcept move constructor is slightly slower than one without noexcept. In addition, the noexcept move constructor can be used in many more places than the one without, specifically it can be used within the STL itself. See https://stackoverflow.com/questions/26922113/when-should-i-declare-a-move-constructor-without-noexcept

It is especially important to use noexcept for types that are intended to be used with the standard library containers. If the move constructor for an element type in a container is not noexcept then the container will use the copy constructor rather than the move constructor.

[jpfeuffer]

Is there in theory a third option to use (unique) pointers to the maps in those classes? I am aware though that it would require larger changes.

I would have no problems with 2) if it will be clear where the error comes from, when the error occurs (std::bad_alloc?). I agree that we probably cannot recover the cases where it fails anyhow.

[jpfeuffer]

I also agree now that we probably do not need to worry about noexcept vs. non-noexcept for now, as long as the move constructor is used.

[hroest]

I would have no problems with 2) if it will be clear where the error comes from, when the error occurs (std::bad_alloc?). I agree that we probably cannot recover the cases where it fails anyhow.

I dont think you will be a in a great place when a noexcept method throws an exception, it will trigger std::terminate https://stackoverflow.com/questions/36393203/why-is-it-allowed-to-throw-an-exception-inside-a-noexcept-tagged-function

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.