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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to