Fix: inconsistent/incorrect implementations of Ord
, PartialOrd
, PartialEq
and Eq
leading to panics on sort
#28
+150
−26
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Since Rust >= 1.81, sort algorithms panic when they detect inconsistent/incorrect implementations of
Ord
,PartialOrd
,PartialEq
andEq
traits.This PR:
AbstractValue
,FunctionReference
,Path
andPathEnum
types to ensure that their implementations of the above four traits are consistent.Hash
implementation (where applicable) is also consistent withEq
(and hencePartialEq
) for the aforementioned types.Ord
for theFunctionReference
andPathEnum
types.References:
https://blog.rust-lang.org/2024/09/05/Rust-1.81.0.html#new-sort-implementations
https://doc.rust-lang.org/nightly/std/cmp/trait.Ord.html#how-can-i-implement-ord
https://github.com/rust-lang/rust/blob/1.81.0/library/core/src/slice/sort/shared/smallsort.rs#L847-L863
https://doc.rust-lang.org/nightly/std/hash/trait.Hash.html#hash-and-eq
Fixes # (issue)
N/A
Type of change
How Has This Been Tested?
A contrived unit test for
AbstractValue
has been added.Panics typically happen at these sort operations in the
summarize
function.Suggestions for how to implement more and/or better tests are welcome.
In general, it's quite complicated (for me anyway) to create minimal tests that cause MIRAI to visit
summarize
in these inconsistent states. However, this repo from this recent issue is also affected.Checklist:
main
.