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 await for future-related type errors #61076

Closed
nikomatsakis opened this issue May 23, 2019 · 22 comments
Closed

suggest await for future-related type errors #61076

nikomatsakis opened this issue May 23, 2019 · 22 comments

Comments

@nikomatsakis
Copy link
Contributor

@nikomatsakis nikomatsakis commented May 23, 2019

A common problem when using async-await is to forget to invoke .await. This often shows up as a type error, e.g. in a case like this (playground):

#![feature(async_await)]

async fn make_u32() -> u32 {
    22
}

fn take_u32(x: u32) {

}

async fn foo() {
    let x = make_u32();
    take_u32(x);
}

fn main() { }

currently we give a rather generic error:

error[E0308]: mismatched types
  --> src/main.rs:13:14
   |
13 |     take_u32(x);
   |              ^ expected u32, found opaque type
   |
   = note: expected type `u32`
              found type `impl std::future::Future`

it'd be nice if we could suggest adding .await somewhere.

@nikomatsakis
Copy link
Contributor Author

@nikomatsakis nikomatsakis commented May 23, 2019

@nikomatsakis
Copy link
Contributor Author

@nikomatsakis nikomatsakis commented May 23, 2019

@wycats was reporting a much worse error, but I'm quite sure what he did to encounter it:

image

Probably has something to do with streams?

@estebank
Copy link
Contributor

@estebank estebank commented May 23, 2019

This will be harder to do for chained expessions future.field_or_resolved_future.execute_new_future()? that should have been future.await.field_or_resolved_future.execute_new_future().await?.

@cramertj
Copy link
Member

@cramertj cramertj commented May 23, 2019

@nikomatsakis Looks like @wycats's error was the same, it's just that the output type of the future is more complicated (Pin<Box<Stream<Item = Value> + Send + 'static>>).

@doctorn
Copy link
Contributor

@doctorn doctorn commented Jun 17, 2019

Hi, I'd be really interested in working on this

@davidtwco
Copy link
Member

@davidtwco davidtwco commented Jun 17, 2019

@doctorn awesome! Sending @rustbot claim should assign you to the issue and feel free to start a topic in the #wg-async-foundations stream if you need a hand.

@doctorn
Copy link
Contributor

@doctorn doctorn commented Jun 17, 2019

@rustbot claim

@rustbot rustbot self-assigned this Jun 17, 2019
Centril added a commit to Centril/rust that referenced this issue Jun 27, 2019
…jasper

Add suggestion for missing `.await` keyword

This commit adds a suggestion diagnostic for missing `.await`. In order to do this, the trait `Future` is promoted to a lang item.

Compiling code of the form:

```rust
#![feature(async_await)]

fn take_u32(x: u32) {}

async fn make_u32() -> u32 {
    22
}

async fn foo() {
    let x = make_u32();
    take_u32(x)
}

fn main() {}
```

Will now result in the suggestion:

```
error[E0308]: mismatched types
  --> src/main.rs:11:18
   |
11 |         take_u32(x)
   |                  ^
   |                  |
   |                  expected u32, found opaque type
   |                  help: consider using `.await` here: `x.await`
   |
   = note: expected type `u32`
              found type `impl std::future::Future`
```

This commit does not cover chained expressions and therefore only covers the case originally pointed out in rust-lang#61076. Cases I can think of that still need to be covered:

- [ ] Return places for functions
- [ ] Field access
- [ ] Method invocation

I'm planning to submit PRs for each of these separately as and when I have figured them out.
Centril added a commit to Centril/rust that referenced this issue Jun 27, 2019
…jasper

Add suggestion for missing `.await` keyword

This commit adds a suggestion diagnostic for missing `.await`. In order to do this, the trait `Future` is promoted to a lang item.

Compiling code of the form:

```rust
#![feature(async_await)]

fn take_u32(x: u32) {}

async fn make_u32() -> u32 {
    22
}

async fn foo() {
    let x = make_u32();
    take_u32(x)
}

fn main() {}
```

Will now result in the suggestion:

```
error[E0308]: mismatched types
  --> src/main.rs:11:18
   |
11 |         take_u32(x)
   |                  ^
   |                  |
   |                  expected u32, found opaque type
   |                  help: consider using `.await` here: `x.await`
   |
   = note: expected type `u32`
              found type `impl std::future::Future`
```

This commit does not cover chained expressions and therefore only covers the case originally pointed out in rust-lang#61076. Cases I can think of that still need to be covered:

- [ ] Return places for functions
- [ ] Field access
- [ ] Method invocation

I'm planning to submit PRs for each of these separately as and when I have figured them out.
Centril added a commit to Centril/rust that referenced this issue Jun 27, 2019
…jasper

Add suggestion for missing `.await` keyword

This commit adds a suggestion diagnostic for missing `.await`. In order to do this, the trait `Future` is promoted to a lang item.

Compiling code of the form:

```rust
#![feature(async_await)]

fn take_u32(x: u32) {}

async fn make_u32() -> u32 {
    22
}

async fn foo() {
    let x = make_u32();
    take_u32(x)
}

fn main() {}
```

Will now result in the suggestion:

```
error[E0308]: mismatched types
  --> src/main.rs:11:18
   |
11 |         take_u32(x)
   |                  ^
   |                  |
   |                  expected u32, found opaque type
   |                  help: consider using `.await` here: `x.await`
   |
   = note: expected type `u32`
              found type `impl std::future::Future`
```

This commit does not cover chained expressions and therefore only covers the case originally pointed out in rust-lang#61076. Cases I can think of that still need to be covered:

- [ ] Return places for functions
- [ ] Field access
- [ ] Method invocation

I'm planning to submit PRs for each of these separately as and when I have figured them out.
@gilescope
Copy link
Contributor

@gilescope gilescope commented Jul 30, 2019

Can we close this now @doctorn ? - seems like this is now done.

@doctorn
Copy link
Contributor

@doctorn doctorn commented Jul 30, 2019

Unfortunately not... I haven't had a lot of time for the past few weeks, but basically all cases similar to this one that @estebank pointed out still aren't covered.

future.field_or_resolved_future.execute_new_future()? that should have been
future.await.field_or_resolved_future.execute_new_future().await?.

I've made some progress towards this, but there are some complicated trait predicates involved in solving the ? case. I brought this up in #t-compiler/help but I'm not aware of anyone who knows how to solve this at the moment.

@estebank
Copy link
Contributor

@estebank estebank commented Jul 30, 2019

I fear that that case will remain open for a while, as it'll be hard to get right.

@gilescope
Copy link
Contributor

@gilescope gilescope commented Jul 31, 2019

I was thinking that this issue was fairly similar to #63100.

@doctorn
Copy link
Contributor

@doctorn doctorn commented Jul 31, 2019

It is essentially the same issue. It boils down to altering the trait inference code to give diagnostics when certain necessary trait predicates don’t hold. For example if you consider the question mark case:

async fn foo() -> Result<(), ()> {
    Ok(())
}

async fn bar() -> Result<(), ()> {
    foo()?;
    Ok(())
}

The error here arises because Future<Output=Result<(), ()>> does not implement std::ops::Try; however, it’s obvious to us that the Output type does, so we want to await here.

What we’d want to do is say that any time type T can’t be shown to implement an arbitrary trait (say Baz), if we can show that T: Future<Output=U> where U: Baz suggest .await. Similarly, for functions rather than futures if we can show that T: Fn() -> U where U: Baz suggest calling it.

Long story short, we need to verify if a trait bound holds for a projected type; however, I don’t think you can do this easily.

@doctorn
Copy link
Contributor

@doctorn doctorn commented Jul 31, 2019

#63100 only points out cases where we have a concrete expected type which are the same cases I covered with #62067. Covering expected trait bounds is where it gets sticky.

@estebank
Copy link
Contributor

@estebank estebank commented Aug 28, 2019

From #62067, these are the cases still missing:

  • Return places for functions
  • Field access
  • Method invocation

In bodies that aren't async, .await is not suggested, but maybe we should mention that we're not in an async block and that the future could have been awaited. I guess this is likely to be a common wip error during development.

@nikomatsakis
Copy link
Contributor Author

@nikomatsakis nikomatsakis commented May 5, 2020

@rustbot release-assignment

@doctorn mentioned they were pretty busy

@rustbot rustbot removed their assignment May 5, 2020
@tmandry tmandry added this to On deck in wg-async-foundations work via automation May 5, 2020
@csmoe csmoe moved this from On deck to In progress in wg-async-foundations work May 6, 2020
@csmoe csmoe self-assigned this May 6, 2020
@csmoe csmoe moved this from In progress to Claimed in wg-async-foundations work May 6, 2020
@csmoe csmoe moved this from Claimed to In progress in wg-async-foundations work May 10, 2020
@jebrosen
Copy link
Contributor

@jebrosen jebrosen commented May 11, 2020

Here's another example where suggesting .await would be helpful (playground):

async fn a() -> Result<(), ()> {
    Ok(())
}

fn main() {
    match a() {
        Ok(()) => (),
        Err(()) => (),
    }
}

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue May 15, 2020
Suggest to await future before ? operator

Closes rust-lang#71811
cc rust-lang#61076
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue May 16, 2020
Suggest to await future before ? operator

Closes rust-lang#71811
cc rust-lang#61076
flip1995 pushed a commit to flip1995/rust that referenced this issue May 17, 2020
@tmandry
Copy link
Contributor

@tmandry tmandry commented May 19, 2020

@csmoe Thanks for improving on this with #71948. Do you want to continue working on this, or release the assignment for now?

@csmoe
Copy link
Member

@csmoe csmoe commented May 20, 2020

@tmandry I'll try to fix them all, more commits pending.

@csmoe
Copy link
Member

@csmoe csmoe commented May 20, 2020

From #62067, these are the cases still missing:

* [ ]  Return places for functions

* [ ]  Field access

* [ ]  Method invocation

In bodies that aren't async, .await is not suggested, but maybe we should mention that we're not in an async block and that the future could have been awaited. I guess this is likely to be a common wip error during development.

@estebank could you write a case of return-place-for-functions? I'm not sure about this one, since await seems already suggested

@estebank
Copy link
Contributor

@estebank estebank commented May 20, 2020

Indeed. I'm not sure if it is due to an improvement since august last year, or just a mistake on my part. Either way, we are in a good place today :)

@tmandry
Copy link
Contributor

@tmandry tmandry commented Jul 14, 2020

Ping from wg-async-foundations triage. @csmoe are you still working on this?

@csmoe
Copy link
Member

@csmoe csmoe commented Jul 15, 2020

@tmandry yes, in slow progress since time is tough these days.

@bors bors closed this in f7cbb7a Aug 27, 2020
wg-async-foundations work automation moved this from In progress to Done Aug 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked pull requests

Successfully merging a pull request may close this issue.