`default_numeric_fallback` contains multiple false positives

Open
#13,036 2 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
5/5
Estimated time
Over a week
Newbie friendliness
25/100
Issue type
Bug
Clarity
Mostly clear
Activity status
Stale
Tech stack
rust
Domain
tooling

Research direction

Start with the linked rustc_hir_typeck fallback.rs function and the current default_numeric_fallback implementation, using the reproducer to identify the reported false positives. Check how a rustc change would be exposed to Clippy and how the compiler and Clippy would be updated together. Done means the reproducer no longer emits warnings for cases without meaningful numeric fallback, with appropriate coverage for the listed exceptions.

Written by the indexing model from the issue text.

Description

C-bug I-false-positive
Summary

There are a number of cases where the lint default_numeric_fallback is emitted, but no numeric fallback actually occurs due to type inference. I want to use this lint on my projects, but it causes far too many false positives.

Currently, the lint is emitted whenever a numeric literal contains no type suffix, is of the type i32 or f64, and is not a part of a large set of hardcoded cases where type inferences in known to occur. I believe that due to the complexity of type inference in Rust, the lint should instead directly check if rustc is actually doing numeric fallback.

I would like to help work on a PR implementing these changes, but the private rustc crates currently don't seem to provide any sort of interface to check if numeric fallback occurred. I think the ideal implementation should hook into this function and I could think of a few approaches, but I'm not sure what's best. This would also require updating rustc and then clippy, and I'm unsure what the process for that is like.

Furthermore, in my opinion, looking at RFC0212 as mentioned in the lint description, there are still a few common cases where the type fallen back to does not matter at all, and I think it's worth whitelisting those specific cases.

  • (0..100).map(|_| {}): This include all cases where a range in used as an iterator, but all values out of the iterator is discarded. This is often used to iterate a set number of times, where the type doesn't matter.
  • number << 8: While fallback does occur here, << and >> are implemented on all integer types with identical behavior.
  • 10 > 0 within a macro: I only include this because it was mentioned here. I have not yet ran into this case in my code.
Lint Name

default_numeric_fallback

Reproducer

I tried this code:

#![warn(clippy::default_numeric_fallback)]

fn id_f64(x: f64) -> f64 {
    x
}

struct A<T>(pub T);

fn main() {
    // inference due to binary operations
    let mut a: f64;
    a = 1.;
    a *= 2.;
    let b = 5i32;
    let _ = a > 2. || b == 3;
    let _ = 2. / 1f64;

    // inference due to if and match
    let x = 10f64;
    let _ = if true { x } else { 10. };
    let _ = match 3i32 {
        3 => x,
        _ => 10.,
    };
    let _ = matches!(5i32, 1..=10);

    // inference due to assignment
    let t = (4., 5);
    let _: (f64, i32) = t;
    let _: A<f64> = A(1.);

    // inference due to functions
    let z = 10.;
    let _ = id_f64(z) == 10.;
    let mut o = None::<f64>;
    let _ = &mut o;
    let _ = o.unwrap_or(1.);
    let mut v: Vec<i32> = vec![];
    let _ = &mut v;
    v.push(1);

    // inference due to array
    let a = [1., 2.];
    let _: f64 = *a.first().unwrap();

    // fallback doesn't have any real effect
    let _ = 1u32 << 8;
    for _ in 0..100 {}
}

I saw this happen:
There is a warning on every single unsuffixed numeric literal.

I expected to see this happen:
There should be no warning at all.

Version
rustc 1.81.0-nightly (6868c831a 2024-06-30)
binary: rustc
commit-hash: 6868c831a1eb45c5150ff623cef5e42a8b8946d0
commit-date: 2024-06-30
host: x86_64-unknown-linux-gnu
release: 1.81.0-nightly
LLVM version: 18.1.7
Additional Labels

No response

Dominant language
Rust
Stars
13.5k
Forks
2.2k
Avg merge
2d 10h
Merged PRs (30d)
32

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from rust-lang/rust-clippy

All issues in rust-lang/rust-clippy

Similar issues

More Rust issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.