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

rustc suggests adding Send trait to the wrong item #127555

Closed
zabackary opened this issue Jul 10, 2024 · 2 comments
Closed

rustc suggests adding Send trait to the wrong item #127555

zabackary opened this issue Jul 10, 2024 · 2 comments
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@zabackary
Copy link

Code

pub trait Foo {
    fn bar<F>(&mut self, func: F) -> impl std::future::Future<Output = ()> + Send
    where
        F: FnMut();
}

struct Baz {}

impl Foo for Baz {
    async fn bar<F>(&mut self, _func: F) -> ()
    where
        F: FnMut() + Send,
    {
        ()
    }
}

Current output

$ cargo check
   Checking playground v0.0.1 (/playground)
error[E0277]: `F` cannot be sent between threads safely
  --> src/lib.rs:10:5
   |
10 | /     async fn bar<F>(&mut self, _func: F) -> ()
11 | |     where
12 | |         F: FnMut() + Send,
   | |__________________________^ `F` cannot be sent between threads safely
   |
note: required by a bound in `<Baz as Foo>::bar`
  --> src/lib.rs:12:22
   |
10 |     async fn bar<F>(&mut self, _func: F) -> ()
   |              --- required by a bound in this associated function
11 |     where
12 |         F: FnMut() + Send,
   |                      ^^^^ required by this bound in `<Baz as Foo>::bar`
help: consider further restricting this bound
   |
12 |         F: FnMut() + Send + std::marker::Send,
   |                           +++++++++++++++++++

error[E0276]: impl has stricter requirements than trait
  --> src/lib.rs:12:22
   |
2  | /     fn bar<F>(&mut self, func: F) -> impl std::future::Future<Output = ()> + Send
3  | |     where
4  | |         F: FnMut();
   | |___________________- definition of `bar` from trait
...
12 |           F: FnMut() + Send,
   |                        ^^^^ impl has extra requirement `F: Send`

Some errors have detailed explanations: E0276, E0277.
For more information about an error, try `rustc --explain E0276`.
error: could not compile `playground` (lib) due to 2 previous errors

Desired output

$ cargo check
   Checking playground v0.0.1 (/playground)
error[E0277]: `F` cannot be sent between threads safely
  --> src/lib.rs:10:5
   |
10 | /     async fn bar<F>(&mut self, _func: F) -> ()
11 | |     where
12 | |         F: FnMut() + Send,
   | |__________________________^ `F` cannot be sent between threads safely
   |
note: required by a bound in `<Baz as Foo>::bar`
  --> src/lib.rs:12:22
   |
10 |     async fn bar<F>(&mut self, _func: F) -> ()
   |              --- required by a bound in this associated function
11 |     where
12 |         F: FnMut() + Send,
   |                      ^^^^ required by this bound in `<Baz as Foo>::bar`
help: consider further restricting this bound
   |
 4 |         F: FnMut() + std::marker::Send,
   |                      +++++++++++++++++++

error[E0276]: impl has stricter requirements than trait
  --> src/lib.rs:12:22
   |
2  | /     fn bar<F>(&mut self, func: F) -> impl std::future::Future<Output = ()> + Send
3  | |     where
4  | |         F: FnMut();
   | |___________________- definition of `bar` from trait
...
12 |           F: FnMut() + Send,
   |                        ^^^^ impl has extra requirement `F: Send`

Some errors have detailed explanations: E0276, E0277.
For more information about an error, try `rustc --explain E0276`.
error: could not compile `playground` (lib) due to 2 previous errors

Rationale and extra context

rustc's suggestion to add the std::marker::Send trait to the impl Foo for Baz impl block is incorrect -- the function in that block already has the marker trait. Instead, it should suggest to add the trait in the where F: FnMut() in the trait definition instead, or raise the error on the trait definition instead, because the real error is made there.

If the suggestion is applied, it will result in a second Send bound added to line 12, which is definitely not correct.

Other cases

No response

Rust Version

$ rustc --version --verbose
rustc 1.79.0 (129f3b996 2024-06-10)
binary: rustc
commit-hash: 129f3b9964af4d4a709d1383930ade12dfe7c081
commit-date: 2024-06-10
host: x86_64-pc-windows-msvc
release: 1.79.0
LLVM version: 18.1.7

Anything else?

This is my first bug on the Rust issue tracker, so apologies if I'm confusing or did something wrong. I've been using Rust for around a year now on and off and it's really such a good language!

@zabackary zabackary added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 10, 2024
@chenyukang chenyukang self-assigned this Jul 17, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jul 17, 2024
…27555, r=jieyouxu

Remove invalid further restricting suggestion for type bound

This PR partially addresses rust-lang#127555, it will remove the obvious error suggestion:

```console
   |                      ^^^^ required by this bound in `<Baz as Foo>::bar`
help: consider further restricting this bound
   |
12 |         F: FnMut() + Send + std::marker::Send,
   |                           +++++++++++++++++++
```

I may create another PR to get a better diagnostic for `impl has stricter requirements than trait` scenario.
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jul 18, 2024
Rollup merge of rust-lang#127844 - chenyukang:yukang-fix-type-bound-127555, r=jieyouxu

Remove invalid further restricting suggestion for type bound

This PR partially addresses rust-lang#127555, it will remove the obvious error suggestion:

```console
   |                      ^^^^ required by this bound in `<Baz as Foo>::bar`
help: consider further restricting this bound
   |
12 |         F: FnMut() + Send + std::marker::Send,
   |                           +++++++++++++++++++
```

I may create another PR to get a better diagnostic for `impl has stricter requirements than trait` scenario.
@chenyukang
Copy link
Member

With #127844 is merged, now the incorrect suggestion is removed, the current output is:

error[E0277]: `F` cannot be sent between threads safely
  --> ./p/tr.rs:10:5
   |
10 | /     async fn bar<F>(&mut self, _func: F) -> ()
11 | |     where
12 | |         F: FnMut() + Send,
   | |__________________________^ `F` cannot be sent between threads safely
   |
note: required by a bound in `<Baz as Foo>::bar`
  --> ./p/tr.rs:12:22
   |
10 |     async fn bar<F>(&mut self, _func: F) -> ()
   |              --- required by a bound in this associated function
11 |     where
12 |         F: FnMut() + Send,
   |                      ^^^^ required by this bound in `<Baz as Foo>::bar`

error[E0276]: impl has stricter requirements than trait
  --> ./p/tr.rs:12:22
   |
2  | /     fn bar<F>(&mut self, func: F) -> impl std::future::Future<Output = ()> + Send
3  | |     where
4  | |         F: FnMut();
   | |___________________- definition of `bar` from trait
...
12 |           F: FnMut() + Send,
   |                        ^^^^ impl has extra requirement `F: Send`

I think the second error message is clear enough for the root cause?

@zabackary
Copy link
Author

I guess I should close this, thank you so much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

2 participants