[PATCH] D32788: Fix std::inplace_merge to be stable for all inputs

[Jan Wilken Dörrie via Phabricator via cfe-commits]

jdoerrie added inline comments.


================
Comment at: include/algorithm:4471
                              _RBi(__middle), _RBi(__first),
                              _RBi(__last), __negate<_Compare>(__comp));
     }
----------------
As marshall pointed out in https://bugs.llvm.org//show_bug.cgi?id=31166 this is where the problem lies. While reversing the input ranges is a very nifty trick to be able to use the moved out space at the end of `[first, last)`, simply negating the comparison functor is not enough to preserve stability. 

Assuming a default `__comp` of `std::less`, in line 4431 a negated `__comp` will evaluate to true for equivalent elements, resulting in a move from `*__first2` and thus breaking stability. The fix for this to to still have `__comp` evaluate to false for equivalent elements, what the change in 748 accomplishes.

Given that now `__negate` technically does not negate anymore, maybe it should be renamed to a more suiting `__reverse` or `__swap` or similar. Doing a quick grep over the code base line 4471 seems to be the only place where this is used, so a rename should not have further consequences. Furthermore, the unary `bool operator()` could be deleted.


https://reviews.llvm.org/D32788