Skip to content

impl manual_noop_waker lint#16687

Open
jaroslawroszyk wants to merge 1 commit intorust-lang:masterfrom
jaroslawroszyk:waker_noop
Open

impl manual_noop_waker lint#16687
jaroslawroszyk wants to merge 1 commit intorust-lang:masterfrom
jaroslawroszyk:waker_noop

Conversation

@jaroslawroszyk
Copy link

@jaroslawroszyk jaroslawroszyk commented Mar 7, 2026

Fixes #16639

Description

This PR introduces a new lint manual_noop_waker that detects manual, empty implementations of the std::task::Wake trait. These can be replaced with std::task::Waker::noop(), which is more performant (avoids Arc allocation) and cleaner.

Example

struct MyWaker;
impl Wake for MyWaker {
    fn wake(self: Arc<Self>) {}
    fn wake_by_ref(self: &Arc<Self>) {}
}

Can be replaced with:

let waker = Waker::noop();

changelog: [manual_noop_waker]: Added a lint to detect manual no-op Wake implementations.

@rustbot rustbot added needs-fcp PRs that add, remove, or rename lints and need an FCP S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Mar 7, 2026
@rustbot
Copy link
Collaborator

rustbot commented Mar 7, 2026

r? @dswij

rustbot has assigned @dswij.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: 7 candidates
  • 7 candidates expanded to 7 candidates
  • Random selection from Jarcho, dswij, llogiq, samueltardieu

@jaroslawroszyk jaroslawroszyk force-pushed the waker_noop branch 4 times, most recently from 9ff1a2c to a64fe9c Compare March 7, 2026 23:35
Copy link
Member

@samueltardieu samueltardieu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will falsely trigger the lint:

trait Wake {
    fn wake(self);
}

impl Wake for () {
    fn wake(self) {}
}

See the Clippy book for good practices. Sorry if this is not the case, but the lint looks vibe-coded by a LLM which is not very efficient.

View changes since this review

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Mar 8, 2026
@rustbot
Copy link
Collaborator

rustbot commented Mar 8, 2026

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@jaroslawroszyk
Copy link
Author

This will falsely trigger the lint:

trait Wake {
    fn wake(self);
}

impl Wake for () {
    fn wake(self) {}
}

See the Clippy book for good practices. Sorry if this is not the case, but the lint looks vibe-coded by a LLM which is not very efficient.

View changes since this review

It's not vibe coded. I followed example and on that I'll do implementation. It's my first pr in this repo.

@jaroslawroszyk jaroslawroszyk force-pushed the waker_noop branch 4 times, most recently from 31dfdc6 to 9e5f983 Compare March 8, 2026 12:59
@jaroslawroszyk
Copy link
Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Mar 8, 2026
Copy link
Member

@samueltardieu samueltardieu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You haven't addressed the false positives (see #16687 (review))

View changes since this review

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Mar 9, 2026
@jaroslawroszyk
Copy link
Author

You haven't addressed the false positives (see #16687 (review))

View changes since this review

Did you see?
tests/ui/manual_noop_waker.rs
and stderr of that?

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Mar 9, 2026
@jaroslawroszyk
Copy link
Author

@rustbot ready

@jaroslawroszyk jaroslawroszyk reopened this Mar 9, 2026
@samueltardieu
Copy link
Member

Did you see? tests/ui/manual_noop_waker.rs and stderr of that?

It doesn't test the same thing. Did you even try to run Clippy on the exact code I gave?

trait Wake {
    fn wake(self);
}

impl Wake for () {
    fn wake(self) {}
}
warning: manual implementation of a no-op waker
 --> /tmp/t.rs:5:1
  |
5 | / impl Wake for () {
6 | |     fn wake(self) {}
7 | | }
  | |_^
  |
  = help: use `std::task::Waker::noop()` instead

This is wrong.

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Mar 9, 2026
@samueltardieu
Copy link
Member

The proper way to check that this is the right trait is to use a diagnostic item when available (it is not the case here), or a type path. All this is described in the Clippy book in the trait checking chapter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-fcp PRs that add, remove, or rename lints and need an FCP S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Lint on no-op implementations of core::task::Wake

4 participants