Skip to content

Fix mixed OrderedFloat equality for NaN values - #185

Open
RanaPriyansh wants to merge 1 commit into
reem:masterfrom
RanaPriyansh:fix/mixed-equality-143
Open

RanaPriyansh wants to merge 1 commit into
reem:masterfrom
RanaPriyansh:fix/mixed-equality-143

Conversation

@RanaPriyansh

Copy link
Copy Markdown

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
Author

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

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