Skip to content

AST: switch to std::sort from llvm::array_pod_sort #23620

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 28, 2019

Conversation

compnerd
Copy link
Member

Swap the array_pod_sort to sort. array_pod_sort is meant to be
used with POD types only. Switching to std:sort should be slightly
faster, and more importantly, allows the emission of the warnings on
Windows to be sorted identically to Linux and macOS.

Replace this paragraph with a description of your changes and rationale. Provide links to external references/discussions if appropriate.

Resolves SR-NNNN.

Swap the `array_pod_sort` to `sort`.  `array_pod_sort` is meant to be
used with POD types only.  Switching to `std:sort` should be slightly
faster, and more importantly, allows the emission of the warnings on
Windows to be sorted identically to Linux and macOS.
@compnerd
Copy link
Member Author

CC: @DougGregor

@compnerd
Copy link
Member Author

@swift-ci please test

@compnerd compnerd merged commit d909dd1 into swiftlang:master Mar 28, 2019
@compnerd compnerd deleted the generic-generic-programming branch March 28, 2019 18:45
@jrose-apple
Copy link
Contributor

Are the constraint types not POD in practice? But if the sort isn't stable, that may be enough reason to do this anyway.

@davidungar
Copy link
Contributor

I believe that std::sort isn't stable, but there's a std::stable_sort. I think you need the latter if you want stability.

@jrose-apple
Copy link
Contributor

Oops, right, and we've definitely run into that before.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants