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

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

Merged
merged 2 commits into from
Dec 28, 2020

Conversation

ThePuzzlemaker
Copy link
Contributor

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:

fn f() -> i32 { 0 }
fn fn_ptr() -> _ { f }
fn closure() -> _ { || 0 }

would result in this diagnostic:

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):

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 #80179.

@rust-highfive
Copy link
Collaborator

r? @varkor

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 22, 2020
@varkor
Copy link
Member

varkor commented Dec 22, 2020

Could you add a couple of tests for the two new cases?

…ds rather than the unique closure type in E0121

This is a squash of the titular commit along with these minor commits:
- Improve note
- Improve note pt2
@ThePuzzlemaker
Copy link
Contributor Author

@varkor Alright, added a regression test containing the two cases.

@varkor
Copy link
Member

varkor commented Dec 26, 2020

Great, thanks!

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Dec 26, 2020

📌 Commit 5e6dc92 has been approved by varkor

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 26, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request 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 added a commit to rust-lang-ci/rust that referenced this pull request Dec 28, 2020
Rollup of 11 pull requests

Successful merges:

 - rust-lang#79662 (Move some more code out of CodegenBackend::{codegen_crate,link})
 - rust-lang#79815 (Update RELEASES.md for 1.49.0)
 - rust-lang#80284 (Suggest fn ptr rather than fn item and suggest to use `Fn` trait bounds rather than the unique closure type in E0121)
 - rust-lang#80331 (Add more comments to trait queries)
 - rust-lang#80344 (use matches!() macro in more places)
 - rust-lang#80353 (BTreeMap: test split_off (and append) more thoroughly)
 - rust-lang#80362 (Document rustc_macros on nightly-rustc)
 - rust-lang#80399 (Remove FIXME in rustc_privacy)
 - rust-lang#80408 (Sync rustc_codegen_cranelift)
 - rust-lang#80411 (rustc_span: Remove `Symbol::with`)
 - rust-lang#80434 (bootstrap: put the component name in the tarball temp dir path)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 12ac312 into rust-lang:master Dec 28, 2020
@rustbot rustbot added this to the 1.51.0 milestone Dec 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
5 participants