[libcxx-commits] [PATCH] D97162: [libcxx] adds std::ranges::swap, std::swappable, and std::swappable_with

Louis Dionne via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Thu Mar 4 15:04:58 PST 2021

ldionne accepted this revision.
ldionne added inline comments.
This revision is now accepted and ready to land.

Comment at: libcxx/include/concepts:298
+    {
+      // FIXME(cjdb): replace loop with ranges::swap_ranges.
+      for (size_t __i = 0; __i < _Size; ++__i) {
ldionne wrote:
> cjdb wrote:
> > miscco wrote:
> > > I *think* this should not be done. We would lose a tone of throughput due to the additional checking done inside `ranges::swap_ranges`.
> > > 
> > > Is there a measurable benefit in using `ranges::swap_ranges`
> > http://eel.is/c++draft/concept.swappable#2.2 explicitly mentions it as the call, but if we can skip the call, I'm all for it.
> I don't understand Michael's comment about the loss of throughput for using `swap_ranges` here. It seems to me like that would be the correct way of implementing this. Why reinvent the wheel?
Thinking further, obviously we can't use `ranges::swap_ranges` before it has been implemented. I think we should add a TODO and come fix it later once it's implemented so that we are in line with the spec.

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list