[PATCH] D45140: [Support] Change std::sort to llvm::sort in response to r327219

Mandeep Singh Grang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 9 11:32:02 PDT 2018


mgrang added a comment.

In https://reviews.llvm.org/D45140#1061971, @jordan_rose wrote:

> > Are there instances in llvm where we perform range-based sorting? I see that std has an experimental range-based sort (std::experimental::ranges::sort) which I don't see being used in llvm.
>
> I think in practice almost *every* sort we do would be range-based (i.e. we almost always sort an entire container). We generally don't use `std::experimental` things in LLVM because they're not reliably present across platforms, but that doesn't mean we don't provide our own implementations for good ideas.
>
> > Also what should be the semantics of a range-based shuffle sort? Will it just shuffle the given range? Or should it shuffle the entire container but just sort the range?
>
> I don't understand this question. You can't get to the entire container from a range.




> I think in practice almost *every* sort we do would be range-based (i.e. we almost always sort an entire container).

Exactly that's what I thought too, and llvm::sort already accepts a range (Container.begin(), Container.end()). In that case, could you please clarify what exactly do you mean by a "range-based variant" and its use cases in LLVM?
My idea of range-based was maybe you wanted to sort a subset of the container. Apologies if I did not understand the original requirement :)


Repository:
  rL LLVM

https://reviews.llvm.org/D45140





More information about the llvm-commits mailing list