Skip to content

New lint: non_canonical_total_float_cmp#16817

Open
nik-rev wants to merge 1 commit intorust-lang:masterfrom
nik-contrib:partial-cmp-lint
Open

New lint: non_canonical_total_float_cmp#16817
nik-rev wants to merge 1 commit intorust-lang:masterfrom
nik-contrib:partial-cmp-lint

Conversation

@nik-rev
Copy link
Copy Markdown

@nik-rev nik-rev commented Apr 6, 2026

Zulip discussion: https://rust-lang.zulipchat.com/#narrow/channel/257328-clippy/topic/lint.20.60float.2Epartial_cmp.28float2.29.2Eunwrap_or.28Ordering.3A.3ALess.29.60/with/536651326

What it does

Checks for manual attempts to force a total ordering on floating-point numbers by
using .partial_cmp(..).unwrap_or(Ordering).

Why is this bad?

Floating-point numbers do not have a total order by default because of [NaN] values and the existence of both -0.0 and 0.0.

When developers use .unwrap_or(..) with [partial_cmp], they are usually trying to implement a total order manually, likely because they do not know about the existence of [total_cmp].

However, such an attempt can never produce a correct total order.

For example, if a comparison function is given (1.0, NaN) to compare, then, regardless of which [Ordering] it returns (other than Equal), it must return the opposite ordering for (NaN, 1.0), which use of unwrap_or() cannot do.

In contexts like sorting or searching (e.g., BTreeMap), such an inconsistent ordering function can lead to non-deterministic results, infinite loops, or panics.

While sorting is the most common place this pattern appears, it also affects any logic relying on a stable comparison, such as finding a minimum/maximum value in a collection.

Known problems

  • Performance: On some architectures (like x86), total_cmp may be
    slightly slower than hardware-accelerated float comparisons if the values
    are already in floating-point registers.

  • Semantic Change: total_cmp considers -0.0 to be strictly less than 0.0.
    If your logic relies on them being equal (the default behavior of ==),
    this lint's suggestion will change your program's behavior.

Example

use std::cmp;
 
let x = 0.0;
let y = -0.0;
let is_less = x.partial_cmp(&y).unwrap_or(cmp::Ordering::Greater);
 
vec.sort_by(|a, b| a.partial_cmp(b).unwrap_or(cmp::Ordering::Equal));

Use instead:

let x = 0.0;
let y = -0.0;
let is_less = x.total_cmp(&y);
 
vec.sort_by(|a, b| a.total_cmp(b));

AI Disclosure

I used gemini to help me write the "performance" section under "Known problems" in the lint's documentation.


changelog: [non_canonical_total_float_cmp]: add lint

@rustbot rustbot added needs-fcp PRs that add, remove, or rename lints and need an FCP S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Apr 6, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 6, 2026

r? @llogiq

rustbot has assigned @llogiq.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: 7 candidates
  • 7 candidates expanded to 7 candidates
  • Random selection from Jarcho, dswij, llogiq, samueltardieu

@nik-rev nik-rev force-pushed the partial-cmp-lint branch from 465994e to 68aabe2 Compare April 6, 2026 07:36
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 6, 2026

Lintcheck changes for 8438094

Lint Added Removed Changed
clippy::non_canonical_total_float_cmp 2 0 0

This comment will be updated if you push new changes

@nik-rev nik-rev force-pushed the partial-cmp-lint branch from 68aabe2 to 6d44767 Compare April 6, 2026 07:47
Comment thread clippy_lints/src/methods/mod.rs Outdated
Co-authored-by: Kevin Reid <kpreid@switchb.org>
@nik-rev nik-rev force-pushed the partial-cmp-lint branch from 1ff644b to 8438094 Compare April 6, 2026 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-fcp PRs that add, remove, or rename lints and need an FCP S-waiting-on-review Status: Awaiting review from the assignee but also interested parties

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants