Skip to content

Conversation

cantino
Copy link
Owner

@cantino cantino commented Feb 2, 2025

Fix #448 in a simpler way than #451

@cantino cantino mentioned this pull request Feb 2, 2025
@workingjubilee
Copy link

image

pub fn sort_unstable_by(&mut self, compare: F)
where
F: FnMut(&T, &T) -> Ordering,

Sorts the slice with a comparison function, without preserving the initial order of equal elements.

...

Panics

May panic if compare does not implement a total order.

This does not actually seem to solve the problem.

@cantino
Copy link
Owner Author

cantino commented Feb 7, 2025

@unexge, does this fix the issue for you?

@workingjubilee
Copy link

Because the panic will happen dynamically, and the sorts are different, if the comparison function is bad, then you can have the same data trigger one panic and not the other.

@unexge
Copy link

unexge commented Feb 7, 2025

Ah, I missed that sort_unstable_by can still panic. It solved the panic for me, I started using this patch ~3 weeks ago when I created the original issue and since then didn't experience any panic. Before this patch I was experiencing panics frequently.

@cantino cantino merged commit d185602 into master Feb 9, 2025
39 checks passed
@cantino
Copy link
Owner Author

cantino commented Feb 9, 2025

Let's merge it for now.

@cantino cantino deleted the fix-448-simpler branch February 9, 2025 05:53
@cantino
Copy link
Owner Author

cantino commented Feb 11, 2025

https://github.com/cantino/mcfly/releases/tag/v0.9.3

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.

Search panics with user-provided comparison function does not correctly implement a total order
3 participants