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

False positive: or_fun_call with const function #5886

Closed
lopopolo opened this issue Aug 10, 2020 · 2 comments · Fixed by #5889
Closed

False positive: or_fun_call with const function #5886

lopopolo opened this issue Aug 10, 2020 · 2 comments · Fixed by #5889
Labels
C-bug Category: Clippy is not doing the correct thing

Comments

@lopopolo
Copy link

I tried this code:

#![warn(clippy::all)]

#[derive(Default, Debug, Clone, Copy, Hash, PartialEq, Eq, PartialOrd, Ord)]
pub struct Error {
    _private: (),
}

impl Error {
    pub const fn new() -> Self {
        Self { _private: () }
    }
}

pub fn main() {
    if let Err(err) = try_main() {
        eprintln!("{:?}", err);
    }
}

pub fn try_main() -> Result<(), Error> {
    let cell = None::<String>;

    let data = cell.ok_or(Error::new())?;
    println!("data: {}", data);
    Ok(())
}

I expected to see this happen: *no clippy warning for using a function call in Option::ok_or position because the funcall is const.

Instead, this happened: Clippy emits clippy::or_fun_call warning. This code has worse performance since it prevents const propagation and defers construction of the error to runtime.

    Checking clippy-false-positive v0.1.0 (/Users/lopopolo/dev/repos/clippy-false-positive)
warning: use of `ok_or` followed by a function call
  --> src/lib.rs:33:21
   |
33 |     let data = cell.ok_or(Error::new())?;
   |                     ^^^^^^^^^^^^^^^^^^^ help: try this: `ok_or_else(Error::new)`
   |
note: the lint level is defined here
  --> src/lib.rs:11:9
   |
11 | #![warn(clippy::all)]
   |         ^^^^^^^^^^^
   = note: `#[warn(clippy::or_fun_call)]` implied by `#[warn(clippy::all)]`
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#or_fun_call

warning: 1 warning emitted

    Finished dev [unoptimized + debuginfo] target(s) in 0.07s

Meta

  • cargo clippy -V: clippy 0.0.212 (5c1f21c3b 2020-07-13)
  • rustc -Vv:
    rustc 1.45.0 (5c1f21c3b 2020-07-13)
    binary: rustc
    commit-hash: 5c1f21c3b82297671ad3ae1e8c942d2ca92e84f2
    commit-date: 2020-07-13
    host: x86_64-apple-darwin
    release: 1.45.0
    LLVM version: 10.0
    
Backtrace

<backtrace>

@lopopolo lopopolo added the C-bug Category: Clippy is not doing the correct thing label Aug 10, 2020
@ebroto
Copy link
Member

ebroto commented Aug 10, 2020

Looks like #5658

@ebroto
Copy link
Member

ebroto commented Aug 10, 2020

Actually, this case may be simpler than the issue I linked before. See this comment which summarizes it very well: #5682 (comment)

TL;DR if a const fn has arguments those may not be constant and the function will be evaluated at runtime, so the suggestion would make sense. In this case though, it seems clippy should not suggest ok_or_else.

(A const fn is only guaranteed to be evaluated at compile time if it's called inside a const context, see here for details)

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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants