[libcxx-commits] [PATCH] D93661: [libc++] [P0879] constexpr std::sort, and add new tests for it

Arthur O'Dwyer via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Thu Feb 4 11:12:27 PST 2021


Quuxplusone marked 2 inline comments as done.
Quuxplusone added inline comments.


================
Comment at: libcxx/include/algorithm:5364
+    }
+}
+
----------------
danlark wrote:
> ldionne wrote:
> > Quuxplusone wrote:
> > > In the `nth_element` review (D93557) I decided to constexpr-ify the algorithm itself. In this one, I decided that the simplest course of action would be to switch on `is_constant_evaluated` and if we're doing it at compile-time let's just do a plain old heapsort because that's already constexpr-friendly, and then we don't have to worry about making `__sort` itself constexpr.
> > > 
> > > So the name here might be misleading, and I'd take suggestions for better names. Here `__sort_constexpr` means "This algorithm is guaranteed constexpr-friendly," but it's //above// the switch-dispatch-thingie, whereas in e.g. `__copy_constexpr` it's //below// the switch-dispatch-thingie.
> > > 
> > > I don't want to change the name of `__sort` itself in this specific patch, because that would have ABI implications (some explicit template instantiations are placed in "libc++.a"). I'm excited for the prospect of ABI breakage ;) but I'd like to confine any ABI breakage to a separate PR.
> > If we get rid of the `__unwrap_iter` overload for `sort()`, we can simply remove `__sort_constexpr` and that solves the question. WDYT?
> One thing to think about:
> 
> Yet, std::sort does not guarantee the order of equal elements. With the current patch it may happen that constexpr std::sort and usual std::sort return different ranges because of different stability guarantees and that might be not user friendly, i.e. the property of determinism is lost.
> 
> I don't have a strong opinion on that question, however, I'd prefer constexprify this function instead of calling partial_sort
> With the current patch it may happen that constexpr std::sort and usual std::sort return different ranges [...] i.e. the property of determinism is lost.

That's a good point I hadn't thought of: Maybe constexpr sort produces (1, 2a, 2c, 2b, 3) while runtime sort produces (1, 2b, 2a, 2c, 3).  However, arguably, libc++ doesn't want to be in the business of making guarantees about sort's behavior/stability —  the user should just avoid writing code that relies on those outcomes.

Vice versa, if we make the existing sort algorithm constexpr-friendly in order to make users happy, I think users would //even more so// want us to do it without altering any of its existing outcomes. They might object if libc++ v12.0 sort produces (1, 2b, 2a, 2c, 3) [at runtime only] while libc++ v13.0 sort produces (1, 2b, 2c, 2a, 3) [at runtime and compile-time].


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93661



More information about the libcxx-commits mailing list