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

Error when using debug_assert! and async function #5105

Closed
gliderkite opened this issue Jan 29, 2020 · 3 comments · Fixed by #5106
Closed

Error when using debug_assert! and async function #5105

gliderkite opened this issue Jan 29, 2020 · 3 comments · Fixed by #5106
Labels
C-bug Category: Clippy is not doing the correct thing good-first-issue These issues are a good way to get started with Clippy L-correctness Lint: Belongs in the correctness lint group T-async-await Type: Issues related to async/await

Comments

@gliderkite
Copy link

I'm getting an error with the following code:

use std::path::{Path, PathBuf};
use tokio::fs;

async fn exists(path: impl AsRef<Path>) -> bool {
    fs::metadata(path).await.is_ok()
}

#[tokio::main]
async fn main() {
    let path = PathBuf::from("");
    debug_assert!(!exists(&path).await);
}
error: do not call a function with mutable arguments inside of `debug_assert!`
  --> src/main.rs:11:20
   |
11 |     debug_assert!(!exists(&path).await);
   |                    ^^^^^^^^^^^^^^^^^^^
   |
   = note: `#[deny(clippy::debug_assert_with_mut_call)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#debug_assert_with_mut_call

error: aborting due to previous error

Note that this doesn't occur with the equivalent blocking version:

use std::path::{Path, PathBuf};

fn exists(path: impl AsRef<Path>) -> bool {
    path.as_ref().exists()
}

fn main() {
    let path = PathBuf::from("");
    debug_assert!(!exists(&path));
}

Is this expected behaviour?

@flip1995
Copy link
Member

This is kind of expected, since the behavior of your code will differ in debug and release mode. While the async version will block, until exists is ready in debug mode, it will not block on this in release mode.

But since this cannot lead to bugs, the lint is intended to detect, we should special case .await and bail out.

@flip1995 flip1995 added L-correctness Lint: Belongs in the correctness lint group good-first-issue These issues are a good way to get started with Clippy C-bug Category: Clippy is not doing the correct thing T-async-await Type: Issues related to async/await labels Jan 29, 2020
@gliderkite
Copy link
Author

If I understood you correctly, I'm not sure that bailing out whenever we encounter .await, independently if the method is called with a mutable parameter, is the way to go. The reason being that in a situation like:

use std::path::PathBuf;

async fn pop_mut(path: &mut PathBuf) -> bool {
    path.pop()
}

#[tokio::main]
async fn main() {
    let mut path = PathBuf::from("");
    debug_assert!(!pop_mut(&mut path).await);
}

I'd still want the warning to be raised.

@flip1995
Copy link
Member

flip1995 commented Jan 30, 2020

Ah good point! I'll fix my fix 👍

Edit: Done.

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 good-first-issue These issues are a good way to get started with Clippy L-correctness Lint: Belongs in the correctness lint group T-async-await Type: Issues related to async/await
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants