[PATCH] D150264: [libcxx] Add strict weak ordering checks to sorting algorithms

Louis Dionne via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 17 08:22:24 PDT 2023


ldionne added inline comments.


================
Comment at: libcxx/include/__algorithm/sort.h:704
   typedef typename iterator_traits<_RandomAccessIterator>::difference_type difference_type;
   using _Comp_ref = __comp_ref_type<_Compare>;
   // Upper bound for using insertion sort for sorting.
----------------
Sorry for being so late to the party, but I think there's a problem here. We should decide whether we want to use these added facilities or `__comp_ref_type` to catch these kinds of issues. Right now, we have a weird situation where both can potentially be used at the same time. This means that we have the following mechanisms in place and enabled independently:

1. The assertions I added that ensure that we would not dereference iterators out-of-bounds inside `std::sort`. Those are arguably hardening assertions, not only debug mode assertions.
2. The existing `__comp_ref_type` mechanism which aims to catch invalid strict weak orders at the predicate level.
3. The new mechanism introduced by this patch.

Is there a reason why we'd want to have these three severely overlapping mechanisms (and especially within the same algorithm)?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150264/new/

https://reviews.llvm.org/D150264



More information about the llvm-commits mailing list