starred/pkl
1
0
Fork 0
mirror of https://github.com/apple/pkl.git synced 2026-01-11 14:20:35 +01:00
Code Issues 198 Packages Projects Releases 10 Wiki Activity

[Bug] sorting #132

New Issue
Closed
opened 2025-12-30 01:21:20 +01:00 by adam · 4 comments
adam commented 2025-12-30 01:21:20 +01:00
Owner
Copy Link

Originally created by @jw-y on GitHub (Apr 4, 2024).

local com = (a, b) -> a >= b
local a = List(0, 0, 0, 0, 1, 1, 1, 1, 1, 1)
b = a.sortWith(com)

expected output:

b = List(1, 1, 1, 1, 1, 1, 0, 0, 0, 0)

got:

b = List(1, 1, 1, 1, 0, 0, 0, 0, 1, 1)
Originally created by @jw-y on GitHub (Apr 4, 2024). ``` pkl local com = (a, b) -> a >= b local a = List(0, 0, 0, 0, 1, 1, 1, 1, 1, 1) b = a.sortWith(com) ``` expected output: ``` b = List(1, 1, 1, 1, 1, 1, 0, 0, 0, 0) ``` got: ``` b = List(1, 1, 1, 1, 0, 0, 0, 0, 1, 1) ```
adam added the buggood first issue labels 2025-12-30 01:21:20 +01:00
adam closed this issue 2025-12-30 01:21:20 +01:00
adam commented 2025-12-30 01:21:21 +01:00
Author
Owner
Copy Link

@holzensp commented on GitHub (Apr 4, 2024):

This is wild! I managed to reproduce. Thanks for the bug report.

@holzensp commented on GitHub (Apr 4, 2024): This is _wild_! I managed to reproduce. Thanks for the bug report.
adam commented 2025-12-30 01:21:21 +01:00
Author
Owner
Copy Link

@odenix commented on GitHub (Apr 4, 2024):

I don't think this is a bug.
The documentation of List.sortWith says:

comparator should return true if its first argument comes before its second argument in the sort order, and false otherwise.

So it should be local com = (a, b) -> a > b instead of local com = (a, b) -> a >= b.

I checked Swift's and Scala's sortWith functions, and they have the same requirement. Swift additionally states that the comparator function must be irreflexive.

@odenix commented on GitHub (Apr 4, 2024): I don't think this is a bug. The documentation of `List.sortWith` says: > comparator should return true if its first argument comes before its second argument in the sort order, and false otherwise. So it should be `local com = (a, b) -> a > b` instead of `local com = (a, b) -> a >= b`. I checked Swift's and Scala's `sortWith` functions, and they have the same requirement. Swift additionally states that the comparator function must be *irreflexive*.
adam commented 2025-12-30 01:21:21 +01:00
Author
Owner
Copy Link

@jw-y commented on GitHub (Apr 4, 2024):

Either case, I don't think this is any way intended.

FYI,
found the location of the bug.

58ed8242af/pkl-core/src/main/java/org/pkl/core/stdlib/base/MergeSort.java (L61)
first mid should be mid-1

@jw-y commented on GitHub (Apr 4, 2024): Either case, I don't think this is any way intended. FYI, found the location of the bug. https://github.com/apple/pkl/blob/58ed8242af664ecf156ad1ea72a79214c794b4bc/pkl-core/src/main/java/org/pkl/core/stdlib/base/MergeSort.java#L61 first `mid` should be `mid-1`
adam commented 2025-12-30 01:21:22 +01:00
Author
Owner
Copy Link

@odenix commented on GitHub (Apr 4, 2024):

That indeed looks like a bug. I guess a reflexive comparator function should only give you an unstable/inefficient sort, not a wrong sort.

@odenix commented on GitHub (Apr 4, 2024): That indeed looks like a bug. I guess a reflexive comparator function should only give you an unstable/inefficient sort, not a wrong sort.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
bug
duplicate
enhancement
good first issue
help wanted
pull-request
Mirrored from GitHub Pull Request
No Label bug good first issue
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
adam
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/pkl#132
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?