Skip to content

Fix mixed OrderedFloat equality for NaN values - #185

Merged
mbrubeck merged 1 commit into
reem:masterfrom
RanaPriyansh:fix/mixed-equality-143
Oct 11, 2026
Merged

mbrubeck merged 1 commit into
reem:masterfrom
RanaPriyansh:fix/mixed-equality-143

Conversation

@RanaPriyansh

Copy link
Copy Markdown
Contributor

Problem

OrderedFloat<T> == T used primitive equality, so NaN values compared unequal despite the wrapper equality contract.

Change

Reuse OrderedFloat<T> equality for mixed equality.

Add f32 and f64 regression tests for NaN payloads, finite values, infinities, signed zero, and inequality.

Verification

  • cargo fmt --check
  • cargo test
  • cargo test --no-default-features
  • cargo test --no-default-features --features libm
  • cargo test --features std,schemars
  • cargo test --features rand,randtest
  • cargo test --features arbitrary,bytemuck,rand,randtest,serde,schemars,proptest,rkyv,rkyv_ck,speedy,facet
  • cargo clippy --tests --features arbitrary,bytemuck,rand,randtest,serde,schemars,proptest,rkyv,rkyv_ck,speedy,facet

All checks pass with Rust 1.94.1.

This change does not add mixed PartialOrd or reverse-comparison APIs.

Related to #143. This addresses the equality discrepancy only.

The Rust 1.90 CI job has not been run locally.

@mbrubeck

Copy link
Copy Markdown
Collaborator

I'm slightly concerned about the silent behavior change here, though I can't think of a likely scenario where it causes a problem.

Is this a high priority fix for anyone? If not, perhaps it should wait a while to see if we have a chance to bundle it into a new major version.

@RanaPriyansh

Copy link
Copy Markdown
Contributor Author

Waiting for a major release makes sense. This PR addresses an equality consistency issue. I do not have evidence of urgent downstream impact.

@mbrubeck
mbrubeck merged commit b86fd8c into reem:master Oct 11, 2026
1 of 2 checks passed
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.

2 participants