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

E0121 suggests the use of the unique type of a returned function/closure rather than a fn pointer or a Fn trait #80179

Closed
PatchMixolydic opened this issue Dec 19, 2020 · 17 comments · Fixed by #80284
Labels
A-diagnostics Area: Messages for errors, warnings, and lints D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information.

Comments

@PatchMixolydic
Copy link
Contributor

PatchMixolydic commented Dec 19, 2020

I tried this code:

fn f() -> i32 {
    0
}

fn returns_fn_pointer() -> _ {
    f
}

The resulting diagnostic suggests replacing _ with fn() -> i32 {f}, the unique type of f():

error[E0121]: the type placeholder `_` is not allowed within types on item signatures
 --> src/lib.rs:5:28
  |
5 | fn returns_fn_pointer() -> _ {
  |                            ^
  |                            |
  |                            not allowed in type signatures
  |                            help: replace with the correct return type: `fn() -> i32 {f}`

This, of course, doesn't work:

error: expected item, found `{`
 --> src/lib.rs:5:44
  |
5 | fn returns_fn_pointer() -> fn() -> i32 {f} {
  |                                            ^ expected item

I'd expect the diagnostic to suggest replacing _ with fn() -> i32 or impl Fn() -> i32, both of which compile successfully.

Meta

rustc --version --verbose:

rustc 1.50.0-nightly (2225ee1b6 2020-12-11)
binary: rustc
commit-hash: 2225ee1b62ff089917434aefd9b2bf509cfa087f
commit-date: 2020-12-11
host: x86_64-unknown-linux-gnu
release: 1.50.0-nightly
@PatchMixolydic
Copy link
Contributor Author

@rustbot modify labels: +A-diagnostics +D-incorrect

@rustbot rustbot added A-diagnostics Area: Messages for errors, warnings, and lints D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. labels Dec 19, 2020
@PatchMixolydic
Copy link
Contributor Author

This also seems to happen with closures:

fn returns_closure() -> _ {
    || 5
}
error[E0121]: the type placeholder `_` is not allowed within types on item signatures
 --> src/lib.rs:1:25
  |
1 | fn returns_closure() -> _ {
  |                         ^
  |                         |
  |                         not allowed in type signatures
  |                         help: replace with the correct return type: `[closure@src/lib.rs:2:5: 2:9]`

@PatchMixolydic PatchMixolydic changed the title E0121 suggests the use of the unique type of a function rather than a fn pointer or a Fn trait E0121 suggests the use of the unique type of functions and closures rather than a fn pointer or a Fn trait Dec 19, 2020
@PatchMixolydic PatchMixolydic changed the title E0121 suggests the use of the unique type of functions and closures rather than a fn pointer or a Fn trait E0121 suggests the use of the unique type of a returned function/closure rather than a fn pointer or a Fn trait Dec 19, 2020
@ThePuzzlemaker
Copy link
Contributor

@rustbot claim

@ThePuzzlemaker
Copy link
Contributor

Function pointers are now working, but it looks like closures will be a bit harder of a task.

fn f() -> i32 { 0 }
fn g() -> _ { f } 
error[E0121]: the type placeholder `_` is not allowed within types on item signatures
 --> <anon>:2:11
  |
2 | fn g() -> _ { f }
  |           ^
  |           |
  |           not allowed in type signatures
  |           help: replace with the correct return type: `fn() -> i32`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0121`.

@ThePuzzlemaker
Copy link
Contributor

This brings up some questions though, unfortunately. For example, this code has the same type of issue:

fn f() -> i32 { 0 }
fn g() -> () { f }

This code brings up this diagnostic:

error[E0308]: mismatched types
 --> <anon>:2:16
  |
2 | fn g() -> () { f }
  |           --   ^ expected `()`, found fn item
  |           |
  |           expected `()` because of return type
  |
  = note: expected unit type `()`
               found fn item `fn() -> i32 {f}`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.

And to be honest, these diagnostics aren't entirely incorrect—that is the return type, it's just not valid syntax. Using a function pointer would be taking that unique type and coercing it into a function pointer or non-unique closure type.

Arguably, it should have something like help: this looks like a fn item, try using a function pointer: `fn() -> i32`, but doing this would end up meaning changing a lot of typeck code. I'm not really sure how to go through with this considering these things (as I'm not really too experienced with rustc code and therefore might mess up a bit of things).

@PatchMixolydic
Copy link
Contributor Author

PatchMixolydic commented Dec 21, 2020

For errors like E0308, it would make sense to state the actual type since that is the root cause of the issue (functions cannot be coerced to ()). E0121, however, suggests replacing _ with the type it gives you, which falls apart in the face of these unnameable types. This can be a papercut to people who don't know how to return a function pointer/closure and become confused when the suggestion fails to compile.

This is probably problematic, since changing the Display impl of types (it seems like that's what's being used per rustc_typeck::collect::fn_sig) would likely mess up E0308 and other similar diagnostics.

I haven't looked into the underlying issue personally, but I can see a few options:

  • Don't emit a suggestion for closures or functions. This is the simplest solution and would preserve the other typecheck-related diagnostics, but may be unhelpful to those who rely on E0121 as a simulacrum of typed holes.
  • Add the fix to suggest using a fn pointer if the returned value is a function, but don't emit a suggestion for closures. I'm not sure of the depth of changes necessary for just this, and again, I'm not sure if this meshes well with other diagnostics.
  • Emit a different suggestion for closures and possibly functions along the lines of consider using a Fn, FnOnce, or FnMut trait bound. For more information on Fn traits, see....

@ThePuzzlemaker
Copy link
Contributor

I think that I'll probably do this I guess:

  • If it's a function, suggest a function pointer, but also add that suggestion about a Fn/FnOnce/FnMut trait bound
  • If it's a closure, suggest a Fn/FnOnce/FnMut trait bound as well

@ThePuzzlemaker
Copy link
Contributor

Should I link to the Rust by Example on closure output types or maybe the std documentation for Fn?

@PatchMixolydic
Copy link
Contributor Author

Rust by Example or The Book would probably be preferable since they explain Fn, FnMut, and FnOnce in one place, but I'm not certain if there's any precedent or policy about linking to them in diagnostics.

@ThePuzzlemaker
Copy link
Contributor

Alright. Does this look good?
Function definition:

fn f() -> i32 { 0 }
fn g() -> _ { f }
error[E0121]: the type placeholder `_` is not allowed within types on item signatures
 --> <anon>:2:11
  |
2 | fn g() -> _ { f }
  |           ^
  |           |
  |           not allowed in type signatures
  |           help: replace with the correct return type: `fn() -> i32`
  |
  = help: also consider using a `Fn`, `FnMut`, or `FnOnce` trait bound
  = note: For more information on using `Fn` traits with function outputs, see https://doc.rust-lang.org/rust-by-example/fn/closures/output_parameters.html

error: aborting due to previous error

For more information about this error, try `rustc --explain E0121`.

I'll get closures in a bit, cause my rustc dev build decided to die on me and I have to completely rebuild it 🙁

@PatchMixolydic
Copy link
Contributor Author

👍 Looks good to me, but I'd like to emphasize that I'm not ready a contributor, so take this with a grain of salt.

@ThePuzzlemaker
Copy link
Contributor

Closures:

fn f() -> _ { || 0 }
error[E0121]: the type placeholder `_` is not allowed within types on item signatures
 --> <anon>:1:11
  |
1 | fn f() -> _ { || 0 }
  |           ^ not allowed in type signatures
  |
  = help: consider using an `Fn`, `FnMut`, or `FnOnce` trait bound
  = note: For more information on using `Fn` traits with function outputs, see https://doc.rust-lang.org/rust-by-example/fn/closures/output_parameters.html

error: aborting due to previous error

For more information about this error, try `rustc --explain E0121`.

@ThePuzzlemaker
Copy link
Contributor

I think I'm going to remove the Fn(Mut/Once) from function definitions as it's kind of not strictly necessary as it's a return type, not an input param

@PatchMixolydic
Copy link
Contributor Author

Thank you so much for all of your hard work!

Just a nitpick: it seems like the note should start with a lowercase letter.

@ThePuzzlemaker
Copy link
Contributor

Yeah, camelid noted that in a Zulip thread related to this so I fixed that.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Dec 28, 2020
…arkor

Suggest fn ptr rather than fn item and suggest to use `Fn` trait bounds rather than the unique closure type in E0121

Previously, using `_` as a return type in a function that returned a function/closure would provide a diagnostic that would cause a papercut. For example:
```rust
fn f() -> i32 { 0 }
fn fn_ptr() -> _ { f }
fn closure() -> _ { || 0 }
```
would result in this diagnostic:
```rust
error[E0121]: the type placeholder `_` is not allowed within types on item signatures
 --> <anon>:2:16
  |
2 | fn fn_ptr() -> _ { f }
  |                ^
  |                |
  |                not allowed in type signatures
  |                help: replace with the correct return type: `fn() -> i32 {f}`

error[E0121]: the type placeholder `_` is not allowed within types on item signatures
 --> <anon>:3:17
  |
3 | fn closure() -> _ { || 0 }
  |                 ^
  |                 |
  |                 not allowed in type signatures
  |                 help: replace with the correct return type: `[closure@<anon>:3:21: 3:25]`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0121`.
```
As can be seen, it was suggested to use the function definition return type `fn() -> i32 { f }` which is not valid syntax as a return type. Additionally, closures cause a papercut as unique closure types (notated in this case as `[closure@<anon>:3:21: 3:25]`) are not valid syntax either.

Instead, this PR implements this version of the diagnostic (this example is for the same code featured above):
```rust
error[E0121]: the type placeholder `_` is not allowed within types on item signatures
 --> <anon>:2:16
  |
2 | fn fn_ptr() -> _ { f }
  |                ^
  |                |
  |                not allowed in type signatures
  |                help: replace with the correct return type: `fn() -> i32`

error[E0121]: the type placeholder `_` is not allowed within types on item signatures
 --> <anon>:3:17
  |
3 | fn closure() -> _ { || 0 }
  |                 ^ not allowed in type signatures
  |
  = help: consider using an `Fn`, `FnMut`, or `FnOnce` trait bound
  = note: for more information on `Fn` traits and closure types, see https://doc.rust-lang.org/book/ch13-01-closures.html

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0121`.
```
As can be seen in this diagnostic, the papercut for returning a function item is fixed by suggesting the usage of a function pointer as the return type. As for closures, it's suggested to use an `Fn`, `FnMut`, or `FnOnce` trait bound (with further reading on closures and `Fn` traits in *The Book* for beginners). I did not implement a suggestion to use `impl Fn() -> i32` syntax as that was out-of-scope for my abilities at the moment, therefore someone in the future may want to implement that. Also, it's possible to use either `impl Trait` syntax, generics, or generics with a `where` clause, and some users may not want to use `impl Trait` syntax for their own reasons.

This PR fixes rust-lang#80179.
@bors bors closed this as completed in 12ac312 Dec 28, 2020
@PatchMixolydic
Copy link
Contributor Author

This still fails on generators (playground):

#![feature(generators)]

fn foo() -> _ {
    || yield i32
}
error[E0121]: the type placeholder `_` is not allowed within types on item signatures
 --> src/lib.rs:3:13
  |
3 | fn foo() -> _ {
  |             ^
  |             |
  |             not allowed in type signatures
  |             help: replace with the correct return type: [generator@src/lib.rs:4:5: 4:18 {i32, ()}]

I'm working on a PR to fix this as well as making this suggestion and E0308's try adding a return type suggestion use the same mechanism for checking if a type is suggestable directly.

@rustbot claim

@PatchMixolydic
Copy link
Contributor Author

Moving this to #80844

@rustbot release-assignment

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 D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information.
Projects
None yet
3 participants