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

Reword labels on E0308 involving async fn return type #82165

Merged
merged 1 commit into from
Feb 26, 2021
Merged

Reword labels on E0308 involving async fn return type #82165

merged 1 commit into from
Feb 26, 2021

Conversation

nellshamrell
Copy link
Contributor

Fix for #80658.

When someone writes code like this:

fn foo() -> u8 {
    async fn async_fn() -> () {}

    async_fn()
}

And they try to compile it, they will see an error that looks like this:

error[E0308]: mismatched types
 --> test.rs:4:5
  |
1 | fn foo() -> u8 {
  |             -- expected `u8` because of return type
2 |     async fn async_fn() -> () {}
  |                            -- checked the `Output` of this `async fn`, found opaque type
3 |
4 |     async_fn()
  |     ^^^^^^^^^^ expected `u8`, found opaque type
  |
  = note: while checking the return type of this `async fn`
  = note:     expected type `u8`
          found opaque type `impl Future`

@rust-highfive
Copy link
Collaborator

r? @estebank

(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 Feb 16, 2021
@guswynn
Copy link
Contributor

guswynn commented Feb 16, 2021

I think I noticed this weird wording on #81496 and I think this will fix it! Nice!

@estebank estebank changed the title Nell/fix 80658 b Reword labels on E0308 involving async fn return type Feb 16, 2021
@@ -1502,6 +1502,9 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
pluralize!(count),
),
);
if sp.is_desugaring(DesugaringKind::Async) {
err.note("while checking the return type of this `async fn`");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how cumbersome it might be to procure, but if we had the Span for make_u32 we could have the following output:

error[E0308]: mismatched types
  --> $DIR/dont-suggest-missing-await.rs:14:18
   |
LL | async fn make_u32() -> u32 {
   |          --------      --- checked the `Output` of this `async fn`, found opaque type
   |          |
   |          while checking the return type of this `async fn`
...
LL |         take_u32(x)
   |                  ^ expected `u32`, found opaque type
   |
   = note:     expected type `u32`
           found opaque type `impl Future`
help: consider `await`ing on the `Future`
   |
LL |         take_u32(x.await)
   |                   ^^^^^^

If getting it is too difficult, then we should reword the note to be "while checking the return type of the async fn", because the note has no span and the output on both the CLI and third-party tools might be ambiguous.

@estebank
Copy link
Contributor

After doing either of the options, could you squash your commits? With those nitpicks addressed, this can be landed :)

@rust-log-analyzer

This comment has been minimized.

Comment on lines 1505 to 1511
if sp.is_desugaring(DesugaringKind::Async) {
err.note("while checking the return type of the `async fn`");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you'll have to keep a local that checks if you have noted this in this loop already
In the rebase on my pr, this message shows up twice in 1 diagnostic

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will experiment with this (likely tomorrow), but is there any chance you could point me to a place in the code base where it does a similar local check in the loop?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Experimented and I think I've got what you suggested working. Would you mind trying it out when you have a moment?

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@estebank
Copy link
Contributor

It seems like you'll have to ./x.py fmt after fetching the changes from github, sadly. After that r=me.

@nellshamrell
Copy link
Contributor Author

@estebank I think this is ready to go now!

@estebank
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Feb 25, 2021

📌 Commit 356beb3 has been approved by estebank

@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 Feb 25, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 26, 2021
…tebank

Reword labels on E0308 involving async fn return type

Fix for rust-lang#80658.

When someone writes code like this:

```rust
fn foo() -> u8 {
    async fn async_fn() -> () {}

    async_fn()
}
```

And they try to compile it, they will see an error that looks like this:

```bash
error[E0308]: mismatched types
 --> test.rs:4:5
  |
1 | fn foo() -> u8 {
  |             -- expected `u8` because of return type
2 |     async fn async_fn() -> () {}
  |                            -- checked the `Output` of this `async fn`, found opaque type
3 |
4 |     async_fn()
  |     ^^^^^^^^^^ expected `u8`, found opaque type
  |
  = note: while checking the return type of this `async fn`
  = note:     expected type `u8`
          found opaque type `impl Future`
```
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Feb 26, 2021
…tebank

Reword labels on E0308 involving async fn return type

Fix for rust-lang#80658.

When someone writes code like this:

```rust
fn foo() -> u8 {
    async fn async_fn() -> () {}

    async_fn()
}
```

And they try to compile it, they will see an error that looks like this:

```bash
error[E0308]: mismatched types
 --> test.rs:4:5
  |
1 | fn foo() -> u8 {
  |             -- expected `u8` because of return type
2 |     async fn async_fn() -> () {}
  |                            -- checked the `Output` of this `async fn`, found opaque type
3 |
4 |     async_fn()
  |     ^^^^^^^^^^ expected `u8`, found opaque type
  |
  = note: while checking the return type of this `async fn`
  = note:     expected type `u8`
          found opaque type `impl Future`
```
This was referenced Feb 26, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 26, 2021
…laumeGomez

Rollup of 8 pull requests

Successful merges:

 - rust-lang#81940 (Stabilize str_split_once)
 - rust-lang#82165 (Reword labels on E0308 involving async fn return type)
 - rust-lang#82456 (Replaced some unwrap_or and map_or with lazy variants)
 - rust-lang#82491 (Consider inexpensive inlining criteria first)
 - rust-lang#82506 (Properly account for non-shorthand pattern field in unused variable lint)
 - rust-lang#82535 (Set codegen thread names)
 - rust-lang#82545 (rustdoc: add optional woff2 versions of FiraSans.)
 - rust-lang#82549 (Revert "Update normalize.css to 8.0.1")

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a56bbb1 into rust-lang:master Feb 26, 2021
@rustbot rustbot added this to the 1.52.0 milestone Feb 26, 2021
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
Development

Successfully merging this pull request may close these issues.

7 participants