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

Compiler suggests to add [closure@…] to source code when it can be inferred instead. #103705

Open
vi opened this issue Oct 28, 2022 · 4 comments
Labels
A-closures Area: Closures (`|…| { … }`) A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` C-bug Category: This is a bug. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. S-bug-has-test Status: This bug is tracked inside the repo by a `known-bug` test. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@vi
Copy link
Contributor

vi commented Oct 28, 2022

Unminimized example:

error[E0283]: type annotations needed
   --> crates/websocat-ioless/src/lib.rs:346:27
    |
346 |                 let w = w.sink_map_err(|e : std::io::Error |e.into());
    |                           ^^^^^^^^^^^^
    |
    = note: multiple `impl`s satisfying `BytesCodec: Encoder<_>` found in the `tokio_util` crate:
            - impl Encoder<BytesMut> for BytesCodec;
            - impl Encoder<websocat_api::bytes::Bytes> for BytesCodec;
    = note: required for `FramedWrite<Pin<Box<dyn tokio::io::AsyncWrite + std::marker::Send>>, BytesCodec>` to implement `websocat_api::futures::Sink<_>`
help: try using a fully qualified path to specify the expected types
    |
346 |                 let w = <FramedWrite<Pin<Box<dyn tokio::io::AsyncWrite + std::marker::Send>>, BytesCodec> as SinkExt<Item>>::sink_map_err::<websocat_api::anyhow::Error, [closure@crates/websocat-ioless/src/lib.rs:346:40: 346:61]>(w, |e : std::io::Error |e.into());
    |                         +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ ~

(in vi/websocat@9ef27b9, caused by upgrade from tokio-util 0.6 -> 0.7)

And I'm not sure how do I explicitly choose the impl I want when it is buried deep in third-party generics.

Aren't coherence rules designed to prevent this by the way?

The diagnostic is useful, but contains a lot of types that can be inferred instead, including the closure type. <_ as SinkExt<bytes::Bytes>>::sink_map_err makes it compile. It would be better if code suggestion preserved as much reliance on the type inference as possible.

rustc 1.66.0-nightly (0da281b60 2022-10-27).

@vi vi 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 Oct 28, 2022
@vi
Copy link
Contributor Author

vi commented Oct 28, 2022

Sorry, the got confused by the closure and not recognized the <... as ...>:: syntax and that it suggested me to switch to UFCS.

Would be simpler if error message was minimized by omitting details that are not relevant to the the ambiguity and can continue to be inferred instead.

@vi vi closed this as completed Oct 28, 2022
@fmease
Copy link
Member

fmease commented Oct 29, 2022

The compiler should never suggest to you to write [closure@…] since that's not valid syntax. Could you please re-open the issue, @vi :)

@rustbot label C-bug A-suggestion-diagnostics D-invalid-suggestion E-needs-mcve

@rustbot rustbot added C-bug Category: This is a bug. A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. labels Oct 29, 2022
@vi vi changed the title "multiple impls satisfying found ..." suggests to insert obviously broken code. Compiler suggests to add [closure@…] to source code when it can be inferred instead. Oct 29, 2022
@vi vi reopened this Oct 29, 2022
@rustbot rustbot added the E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example label Oct 29, 2022
@vi
Copy link
Contributor Author

vi commented Oct 29, 2022

Simpler example producing similar message:

trait MyTrait<T> {
   fn lol<F:FnOnce()>(&self, f:F) -> u16;
}

struct Qqq;

impl MyTrait<u32> for Qqq{
   fn lol<F:FnOnce()>(&self, _f:F) -> u16 { 5 }
}
impl MyTrait<u64> for Qqq{
   fn lol<F:FnOnce()>(&self, _f:F) -> u16 { 6 }
}

fn main() {
    let q = Qqq;
    q.lol(||());
}
error[E0282]: type annotations needed
  --> src/main.rs:16:7
   |
16 |     q.lol(||());
   |       ^^^
   |
help: try using a fully qualified path to specify the expected types
   |
16 |     <Qqq as MyTrait<T>>::lol::<[closure@src/main.rs:16:11: 16:13]>(&q, ||());
   |     ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ ~

error[E0283]: type annotations needed
  --> src/main.rs:16:7
   |
16 |     q.lol(||());
   |       ^^^
   |
note: multiple `impl`s satisfying `Qqq: MyTrait<_>` found
  --> src/main.rs:7:1
   |
7  | impl MyTrait<u32> for Qqq{
   | ^^^^^^^^^^^^^^^^^^^^^^^^^
...
10 | impl MyTrait<u64> for Qqq{
   | ^^^^^^^^^^^^^^^^^^^^^^^^^
help: try using a fully qualified path to specify the expected types
   |
16 |     <Qqq as MyTrait<T>>::lol::<[closure@src/main.rs:16:11: 16:13]>(&q, ||());
   |     ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ ~

@compiler-errors compiler-errors added S-has-mcve Status: A Minimal Complete and Verifiable Example has been found for this issue and removed E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example labels Oct 30, 2022
@estebank estebank added the A-closures Area: Closures (`|…| { … }`) label Nov 11, 2022
@estebank
Copy link
Contributor

We could create a new type folder that takes any closure type and replaces it with at minimum a _ and ideally a correct Fn* signature.

Manishearth added a commit to Manishearth/rust that referenced this issue Nov 14, 2022
…imulacrum

Add a few known-bug tests

The labels of these tests should be changed from `S-bug-has-mcve` to `S-bug-has-test` once this is merged.

cc:
rust-lang#101518
rust-lang#99492
rust-lang#90950
rust-lang#89196
rust-lang#104034
rust-lang#101350
rust-lang#103705
rust-lang#103899

I couldn't reproduce the failures in rust-lang#101962 and rust-lang#100772 (so either these have started passing, or I didn't repro properly), so leaving those out for now.

rust-lang#102065 was a bit more complicated, since it uses `rustc_private` and I didn't want to mess with that.
@jackh726 jackh726 added S-bug-has-test Status: This bug is tracked inside the repo by a `known-bug` test. and removed S-has-mcve Status: A Minimal Complete and Verifiable Example has been found for this issue labels Nov 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-closures Area: Closures (`|…| { … }`) A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` C-bug Category: This is a bug. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. S-bug-has-test Status: This bug is tracked inside the repo by a `known-bug` test. 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

6 participants