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

Louis Dionne via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Thu Jan 28 08:02:47 PST 2021

ldionne requested changes to this revision.
ldionne added inline comments.
This revision now requires changes to proceed.

Comment at: libcxx/include/algorithm:5245
+template <class _Compare, class _RandomAccessIterator>
I don't think we need this declaration. Isn't it the same as the definition on line 5165?

Comment at: libcxx/include/algorithm:5502
+sort(__wrap_iter<_Tp*> __first, __wrap_iter<_Tp*> __last, _Compare __comp)
In a second step, couldn't we remove this overload to replace it by calls to `__unwrap_iter`?

Comment at: libcxx/include/algorithm:5364
+    }
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?

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list