Skip to content

[stdlib][fix] Fixed wrong sorting behavior #19107

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.

Sign up for GitHub

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 3 commits into from
Sep 4, 2018
Merged

[stdlib][fix] Fixed wrong sorting behavior #19107

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
524c361
[stdlib] Fixed wrong sorting behavior
kchibisov Sep 2, 2018
6582b6e
[stdlib] Refactored siftDown.
kchibisov Sep 2, 2018
5c45083
[stdlib][test] Added tests for heapSort.
kchibisov Sep 2, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion stdlib/public/core/Sort.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,7 @@ extension MutableCollection where Self: RandomAccessCollection {
// If a child is bigger than the current node, swap them and continue sifting
// down.
if largest != i {
swapAt(idx, largest)
swapAt(i, largest)
Copy link
Collaborator

@xwu xwu Sep 2, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is no reason ever to use the original value idx, then the first line of the function body should be var idx = idx. Even if the proposed change here fixes this particular bug, leaving both i and idx around is a risk for the next edit made to this function.

i = largest
countToIndex = distance(from: range.lowerBound, to: i)
countFromIndex = distance(from: i, to: range.upperBound)
Expand Down