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

Poor error for manual implementations of function traits #39259

Closed
jdm opened this issue Jan 23, 2017 · 10 comments · Fixed by #108930
Closed

Poor error for manual implementations of function traits #39259

jdm opened this issue Jan 23, 2017 · 10 comments · Fixed by #108930
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. D-confusing Diagnostics: Confusing error or lint that should be reworked. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. D-terse Diagnostics: An error or lint that doesn't give enough information about the problem at hand. requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jdm
Copy link
Contributor

jdm commented Jan 23, 2017

#![feature(fn_traits)]

struct S;

impl Fn(u32) -> u32 for S {
    fn call(&self) -> u32 {
        5
    }
}

fn main() {
}

This yields:

error[E0229]: associated type bindings are not allowed here
 --> foo2.rs:5:17
  |
5 | impl Fn(u32) -> u32 for S {
  |                 ^^^ associate type not allowed here

error: aborting due to previous error

Which is extremely confusing and non-actionable.

@jdm jdm added the A-diagnostics Area: Messages for errors, warnings, and lints label Jan 23, 2017
@jdm
Copy link
Contributor Author

jdm commented Jan 23, 2017

cc @jrmuizel

@durka
Copy link
Contributor

durka commented Jan 23, 2017

It is sort of actionable if you know that Fn(u32) -> u32 really means Fn<(u32,), Output=u32>. Let's put that expansion in a note on the error message I guess?

@steveklabnik steveklabnik added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 9, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jul 26, 2017
@BGR360
Copy link
Contributor

BGR360 commented Dec 29, 2021

This code now produces two confusing errors:

#![feature(fn_traits)]
#![feature(unboxed_closures)]

struct S;

impl Fn(u32) -> u32 for S {
    fn call(&self) -> u32 {
        5
    }
}

fn main() {}
error[E0229]: associated type bindings are not allowed here
 --> src/main.rs:6:17
  |
6 | impl Fn(u32) -> u32 for S {
  |                 ^^^ associated type not allowed here

error[E0277]: expected a `FnMut<(u32,)>` closure, found `S`
  --> src/main.rs:6:6
   |
6  | impl Fn(u32) -> u32 for S {
   |      ^^^^^^^^^^^^^^ expected an `FnMut<(u32,)>` closure, found `S`
   |
   = help: the trait `FnMut<(u32,)>` is not implemented for `S`
note: required by a bound in `Fn`

In the first one, the erroneous span has moved inside of the parentheses, so I feel it's no longer as correct as it used to be, and no more helpful.

And the second one just seems... incorrect? Idk, I'm not intimately familiar with this nightly feature.

@rustbot label +D-confusing +D-terse +requires-nightly +D-incorrect

@rustbot rustbot added D-confusing Diagnostics: Confusing error or lint that should be reworked. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. D-terse Diagnostics: An error or lint that doesn't give enough information about the problem at hand. requires-nightly This issue requires a nightly compiler in some way. labels Dec 29, 2021
@estebank
Copy link
Contributor

estebank commented Jan 8, 2023

Current output:

error[E0229]: associated type bindings are not allowed here
 --> src/main.rs:8:17
  |
8 | impl Fn(u32) -> u32 for S {
  |                 ^^^ associated type not allowed here

@Ezrashaw
Copy link
Contributor

Ezrashaw commented Jan 9, 2023

My understanding of this issue is that the impl Fn(u32) -> u32 is expanded to impl Fn<(u32, ), Output = u32> where Output is not allowed and must be moved elsewhere.

Two issues currently seem to exist:

  1. That the expansion (Fn(u32) -> u32 -> Fn<(u32, ), Output = u32>) for fn_traits is not shown. This can be addressed by adding a note to the error message.
  2. That E0229 doesn't suggest a moving the associated type, this needs a proper suggestion and should be discussed more.

I'll work on 1. 2 seems out of scope of this issue.

@rustbot claim

@estebank
Copy link
Contributor

estebank commented Jan 9, 2023

@Ezrashaw if you're able to detect that you've got Fn trait syntax in the Trait part of the imp item, you should be able to print the type directly (to get the desugared version) and mention in passing "include type Output = u32 in the impl body instead" or something like that.

@Ezrashaw
Copy link
Contributor

Ezrashaw commented Jan 9, 2023

@estebank Isn't that suggestion generic to all "assoc type not allowed here" error?

@estebank
Copy link
Contributor

Come to think of it, It would be.

@Ezrashaw
Copy link
Contributor

Hmm, @estebank I think I've run into a problem: I cannot print the type. The error occurs is emitted from rustc_hir_analysis and so ype checking hasn't been done yet.

@estebank
Copy link
Contributor

estebank commented Jan 11, 2023

Because this is a nightly feature, I would side step the issue by not mentioning the type for now and only mention the "represent the closure with the regular trait syntax instead and move the output type to the body this way", but it should be easy to come up with a mechanism to translate from one syntax to the other (if it doesn't exist already!), while not having to rely on ty::*, only on the AST and resolve.

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 C-enhancement Category: An issue proposing an enhancement or a PR with one. D-confusing Diagnostics: Confusing error or lint that should be reworked. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. D-terse Diagnostics: An error or lint that doesn't give enough information about the problem at hand. requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants