Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unnecessary_lazy_evaluations fix causes a panic when used with Range::contains() #11930

Closed
rezzubs opened this issue Dec 5, 2023 · 2 comments
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@rezzubs
Copy link

rezzubs commented Dec 5, 2023

Summary

I encountered this error during Advent of Code where Clippy suggests to change an Option::then to then_some. I used std::ops::Range::contains() to make sure the closure passed to then is safe to call. It can panic when the number is not within the range and should not be evaluated eagerly.

I'm not sure if a lint like this could possibly take outside context into account but it's a false positive nonetheless.

Lint Name

unnecessary_lazy_evaluations

Reproducer

The version with then

use std::ops::Range;

fn main() {
    let map = Mapping {
        range: 10..20,
        offset: 10,
    };
    
    println!("{:?}", map.get(10));
    println!("{:?}", map.get(9));
}

struct Mapping {
    range: Range<u64>,
    offset: u64,
}

impl Mapping {
    fn get(&self, seed: u64) -> Option<u64> {
        // This can panic when replaced with then_some().
        self.range.contains(&seed).then(|| {
            seed - self.offset
        })
    }
}

works as expected.

The version with then_some

use std::ops::Range;

fn main() {
    let map = Mapping {
        range: 10..20,
        offset: 10,
    };
    
    println!("{:?}", map.get(10));
    println!("{:?}", map.get(9)); // panics here
}

struct Mapping {
    range: Range<u64>,
    offset: u64,
}

impl Mapping {
    fn get(&self, seed: u64) -> Option<u64> {
        self.range.contains(&seed).then_some({
            seed - self.offset
        })
    }
}

Version

rustc 1.74.0 (79e9716c9 2023-11-13)
binary: rustc
commit-hash: 79e9716c980570bfd1f666e3b16ac583f0168962
commit-date: 2023-11-13
host: x86_64-unknown-linux-gnu
release: 1.74.0
LLVM version: 17.0.4

Additional Labels

No response

@rezzubs rezzubs added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Dec 5, 2023
@y21
Copy link
Member

y21 commented Dec 5, 2023

#11002 should have fixed this. Can you try updating to the latest nightly and see if that fixes it?

@y21
Copy link
Member

y21 commented Feb 9, 2024

Closing as fixed, the reproducer in the description no longer emits a warning. The fix should be in all channels now (stable, beta, nightly).

@y21 y21 closed this as completed Feb 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
Development

No branches or pull requests

2 participants