Skip to content

fix unused_async with macros and sub async block #13205

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

Closed
wants to merge 6 commits into from

Conversation

hlf20010508
Copy link

changelog: [unused_async]:

  • check after pro_macro_attribute
  • check inside async block

fixes #13199

I've run the test, but failed with the error that I didn't understand.

error: actual output differed from expected
Execute `cargo uibless` to update `tests/ui/unused_async.stderr` to the actual output
--- tests/ui/unused_async.stderr
+++ <stderr output>
 error: unused `async` for function with no await statements
   --> tests/ui/unused_async.rs:12:5
... 46 lines skipped ...
    = help: consider removing the `async` from this function

-error: aborting due to 4 previous errors
+error: unused `async` for function with no await statements

+error: aborting due to 5 previous errors
+

@rustbot
Copy link
Collaborator

rustbot commented Aug 2, 2024

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Manishearth (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Aug 2, 2024
@hlf20010508
Copy link
Author

hlf20010508 commented Aug 2, 2024

I think there's one more problem.
The function can be warned if I don't use it. But once I use it in another file, the warning would be gone, I don't even need to call it.

@hlf20010508
Copy link
Author

I'm not sure if it's is_node_func_call's problem.

@hlf20010508
Copy link
Author

I'm not sure if it's is_node_func_call's problem.

Confirmed.

@Alexendoo
Copy link
Member

I would say the macro detection is working as intended - #13199 (comment)

@hlf20010508
Copy link
Author

I would say the macro detection is working as intended - #13199 (comment)

Thanks, I didn't know this before.

@hlf20010508
Copy link
Author

I think there's one more problem. The function can be warned if I don't use it. But once I use it in another file, the warning would be gone, I don't even need to call it.

I need help. I've fixed this, but didn't pass the issue9695

mod issue9695 {
    use std::future::Future;

    async fn f() {}
    async fn f2() {}
    async fn f3() {}
    //~^ ERROR: unused `async` for function with no await statements

    fn needs_async_fn<F: Future<Output = ()>>(_: fn() -> F) {}

    fn test() {
        let x = f;
        needs_async_fn(x); // async needed in f
        needs_async_fn(f2); // async needed in f2
        f3(); // async not needed in f3
    }
}

f would be warned.

@Manishearth
Copy link
Member

r? @Alexendoo

coming off of vacation and will be on planes for a while

@rustbot rustbot assigned Alexendoo and unassigned Manishearth Aug 5, 2024
@Alexendoo
Copy link
Member

@oli-obk any idea what's happening with this diff? When I run it locally I get a more reasonable output of

--- tests/ui/unused_async.stderr
+++ <stderr output>
 error: unused `async` for function with no await statements
   --> tests/ui/unused_async.rs:12:5
... 17 lines skipped ...
 
 error: unused `async` for function with no await statements
+  --> tests/ui/unused_async.rs:43:5
+   |
+LL |     async fn f() {}
+   |     ^^^^^^^^^^^^^^^
+   |
+   = help: consider removing the `async` from this function
+
+error: unused `async` for function with no await statements
   --> tests/ui/unused_async.rs:45:5
    |
... 25 lines skipped ...
    = help: consider removing the `async` from this function
 
-error: aborting due to 4 previous errors
+error: aborting due to 5 previous errors

@oli-obk
Copy link
Contributor

oli-obk commented Aug 8, 2024

Weird that it's only happening in CI... the full stderr doesn't really fit the diff you posted either. If it's a recurring thing, open an issue

@Alexendoo
Copy link
Member

Oops, I was looking at the wrong run, that one is for an earlier commit. False alarm

@bors
Copy link
Contributor

bors commented Sep 28, 2024

☔ The latest upstream changes (presumably #13471) made this pull request unmergeable. Please resolve the merge conflicts.

@hlf20010508 hlf20010508 closed this Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

unused_async doesn't work if an async fn without await has attribute
6 participants