aaron.ballman added a comment. In D155809#4527199 <https://reviews.llvm.org/D155809#4527199>, @danlark wrote:
> In D155809#4521494 <https://reviews.llvm.org/D155809#4521494>, @rsmith wrote: > >> This looks correct to me, but it's still a little subtle. Perhaps it'd be >> clearer to map the method to an integer (0 for copy assignment, 1 for move >> assignment, 2 for destructor, 3 for equality comparison), and then order >> them by that integer? That'd be more obviously a strict weak order. > > > > In D155809#4520765 <https://reviews.llvm.org/D155809#4520765>, @shafik wrote: > >> I am not sure about this change but I think we at least need a test and this >> does not seem non-functional if it prevents a crash. > > This is NFC as it only prevents further assert to fire when stable_sort > compares the element with itself Richard's suggestion makes sense to me as a clarifying change to the code. I also agree with Shafik -- if this prevents an assertion from firing in practice, then it's a functional change that should come with tests. Or are you saying the assertion isn't happening in practice and this is a defensive change? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155809/new/ https://reviews.llvm.org/D155809 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
