Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Don't suggest awaiting in closure patterns #126915

Merged
merged 1 commit into from
Jun 26, 2024

Conversation

SparkyPotato
Copy link
Contributor

@SparkyPotato SparkyPotato commented Jun 24, 2024

Fixes #126903.

For

async fn do_async() {}

fn main() {
    Some(do_async()).map(|()| {});
}

the error is now

error[E0308]: mismatched types
 --> src/main.rs:4:27
  |
4 |     Some(do_async()).map(|()| {});
  |                           ^^
  |                           |
  |                           expected future, found `()`
  |                           expected due to this
  |
  = note: expected opaque type `impl Future<Output = ()>`
               found unit type `()`

Ideally, if main were to be async, it should be

error[E0308]: mismatched types
 --> src/main.rs:4:27
  |
4 |     Some(do_async()).map(|()| {});
  |                           ^^
  |                           |
  |                           expected future, found `()`
  |                           expected due to this
  |
  = note: expected opaque type `impl Future<Output = ()>`
               found unit type `()`
help: consider `await`ing on the `Future`
  |
4 |     Some(do_async().await).map(|()| {});
  |                    ++++++

However, this would mean FnCtx::check_pat_top would have to be called with an origin_expr in rustc_hir_typeck::check::check_fn, and that expr would have to be somehow plumbed through FnCtxt::check_expr_closure and closure signature deduction. I'm willing to work on the plumbing but unsure how to start.

@rustbot
Copy link
Collaborator

rustbot commented Jun 24, 2024

r? @oli-obk

rustbot has assigned @oli-obk.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 24, 2024
ObligationCauseCode::Pattern { span: Some(then_span), .. } => {
Some(ConsiderAddingAwait::FutureSugg { span: then_span.shrink_to_hi() })
ObligationCauseCode::Pattern { span: Some(then_span), origin_expr, .. } => {
origin_expr.then_some(ConsiderAddingAwait::FutureSugg {
Copy link
Member

Choose a reason for hiding this comment

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

Is there any test that exercises the case when origin_expr is Some?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, most of suggest-missing-await.rs does

@compiler-errors
Copy link
Member

Please squash this into one commit

@compiler-errors
Copy link
Member

thanks

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jun 24, 2024

📌 Commit 26677eb has been approved by compiler-errors

It is now in the queue for this repository.

@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 Jun 24, 2024
compiler-errors added a commit to compiler-errors/rust that referenced this pull request Jun 25, 2024
…r-errors

Don't suggest awaiting in closure patterns

Fixes rust-lang#126903.

For
```rust
async fn do_async() {}

fn main() {
    Some(do_async()).map(|()| {});
}
```
the error is now
```rust
error[E0308]: mismatched types
 --> src/main.rs:4:27
  |
4 |     Some(do_async()).map(|()| {});
  |                           ^^
  |                           |
  |                           expected future, found `()`
  |                           expected due to this
  |
  = note: expected opaque type `impl Future<Output = ()>`
               found unit type `()`
```

Ideally, if `main` were to be `async`, it should be
```rs
error[E0308]: mismatched types
 --> src/main.rs:4:27
  |
4 |     Some(do_async()).map(|()| {});
  |                           ^^
  |                           |
  |                           expected future, found `()`
  |                           expected due to this
  |
  = note: expected opaque type `impl Future<Output = ()>`
               found unit type `()`
help: consider `await`ing on the `Future`
  |
4 |     Some(do_async().await).map(|()| {});
  |                    ++++++
```
However, this would mean `FnCtx::check_pat_top` would have to be called with an `origin_expr` in `rustc_hir_typeck::check::check_fn`, and that expr would have to be somehow plumbed through `FnCtxt::check_expr_closure` and closure signature deduction. I'm willing to work on the plumbing but unsure how to start.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 25, 2024
…mpiler-errors

Rollup of 9 pull requests

Successful merges:

 - rust-lang#126326 (Un-unsafe the `StableOrd` trait)
 - rust-lang#126618 (Mark assoc tys live only if the corresponding trait is live)
 - rust-lang#126724 (Fix a span in `parse_ty_bare_fn`.)
 - rust-lang#126746 (Deny `use<>` for RPITITs)
 - rust-lang#126868 (not use offset when there is not ends with brace)
 - rust-lang#126884 (Do not ICE when suggesting dereferencing closure arg)
 - rust-lang#126915 (Don't suggest awaiting in closure patterns)
 - rust-lang#126916 (Specify target specific linker for `riscv64gc-gnu` job)
 - rust-lang#126926 (Tweak a confusing comment in `create_match_candidates`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 25, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#126618 (Mark assoc tys live only if the corresponding trait is live)
 - rust-lang#126746 (Deny `use<>` for RPITITs)
 - rust-lang#126868 (not use offset when there is not ends with brace)
 - rust-lang#126884 (Do not ICE when suggesting dereferencing closure arg)
 - rust-lang#126893 (Eliminate the distinction between PREC_POSTFIX and PREC_PAREN precedence level)
 - rust-lang#126915 (Don't suggest awaiting in closure patterns)
 - rust-lang#126943 (De-duplicate all consecutive native libs regardless of their options)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 25, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#126618 (Mark assoc tys live only if the corresponding trait is live)
 - rust-lang#126746 (Deny `use<>` for RPITITs)
 - rust-lang#126868 (not use offset when there is not ends with brace)
 - rust-lang#126884 (Do not ICE when suggesting dereferencing closure arg)
 - rust-lang#126893 (Eliminate the distinction between PREC_POSTFIX and PREC_PAREN precedence level)
 - rust-lang#126915 (Don't suggest awaiting in closure patterns)
 - rust-lang#126943 (De-duplicate all consecutive native libs regardless of their options)

r? `@ghost`
`@rustbot` modify labels: rollup
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Jun 25, 2024
…r-errors

Don't suggest awaiting in closure patterns

Fixes rust-lang#126903.

For
```rust
async fn do_async() {}

fn main() {
    Some(do_async()).map(|()| {});
}
```
the error is now
```rust
error[E0308]: mismatched types
 --> src/main.rs:4:27
  |
4 |     Some(do_async()).map(|()| {});
  |                           ^^
  |                           |
  |                           expected future, found `()`
  |                           expected due to this
  |
  = note: expected opaque type `impl Future<Output = ()>`
               found unit type `()`
```

Ideally, if `main` were to be `async`, it should be
```rs
error[E0308]: mismatched types
 --> src/main.rs:4:27
  |
4 |     Some(do_async()).map(|()| {});
  |                           ^^
  |                           |
  |                           expected future, found `()`
  |                           expected due to this
  |
  = note: expected opaque type `impl Future<Output = ()>`
               found unit type `()`
help: consider `await`ing on the `Future`
  |
4 |     Some(do_async().await).map(|()| {});
  |                    ++++++
```
However, this would mean `FnCtx::check_pat_top` would have to be called with an `origin_expr` in `rustc_hir_typeck::check::check_fn`, and that expr would have to be somehow plumbed through `FnCtxt::check_expr_closure` and closure signature deduction. I'm willing to work on the plumbing but unsure how to start.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 25, 2024
…kingjubilee

Rollup of 14 pull requests

Successful merges:

 - rust-lang#126618 (Mark assoc tys live only if the corresponding trait is live)
 - rust-lang#126746 (Deny `use<>` for RPITITs)
 - rust-lang#126868 (not use offset when there is not ends with brace)
 - rust-lang#126884 (Do not ICE when suggesting dereferencing closure arg)
 - rust-lang#126885 (Remove internal `PathBuf::as_mut_vec`)
 - rust-lang#126893 (Eliminate the distinction between PREC_POSTFIX and PREC_PAREN precedence level)
 - rust-lang#126915 (Don't suggest awaiting in closure patterns)
 - rust-lang#126916 (Specify target specific linker for `riscv64gc-gnu` job)
 - rust-lang#126926 (Tweak a confusing comment in `create_match_candidates`)
 - rust-lang#126927 (core: VaArgSafe is an unsafe trait)
 - rust-lang#126932 (Tweak `FlatPat::new` to avoid a temporarily-invalid state)
 - rust-lang#126941 (Migrate `run-make/llvm-ident` to `rmake.rs`)
 - rust-lang#126946 (Add missing slash in `const_eval_select` doc comment)
 - rust-lang#126947 (Delegation: ast lowering refactor)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 25, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#126618 (Mark assoc tys live only if the corresponding trait is live)
 - rust-lang#126746 (Deny `use<>` for RPITITs)
 - rust-lang#126868 (not use offset when there is not ends with brace)
 - rust-lang#126884 (Do not ICE when suggesting dereferencing closure arg)
 - rust-lang#126893 (Eliminate the distinction between PREC_POSTFIX and PREC_PAREN precedence level)
 - rust-lang#126915 (Don't suggest awaiting in closure patterns)
 - rust-lang#126943 (De-duplicate all consecutive native libs regardless of their options)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit dad39e8 into rust-lang:master Jun 26, 2024
6 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jun 26, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jun 26, 2024
Rollup merge of rust-lang#126915 - SparkyPotato:fix-126903, r=compiler-errors

Don't suggest awaiting in closure patterns

Fixes rust-lang#126903.

For
```rust
async fn do_async() {}

fn main() {
    Some(do_async()).map(|()| {});
}
```
the error is now
```rust
error[E0308]: mismatched types
 --> src/main.rs:4:27
  |
4 |     Some(do_async()).map(|()| {});
  |                           ^^
  |                           |
  |                           expected future, found `()`
  |                           expected due to this
  |
  = note: expected opaque type `impl Future<Output = ()>`
               found unit type `()`
```

Ideally, if `main` were to be `async`, it should be
```rs
error[E0308]: mismatched types
 --> src/main.rs:4:27
  |
4 |     Some(do_async()).map(|()| {});
  |                           ^^
  |                           |
  |                           expected future, found `()`
  |                           expected due to this
  |
  = note: expected opaque type `impl Future<Output = ()>`
               found unit type `()`
help: consider `await`ing on the `Future`
  |
4 |     Some(do_async().await).map(|()| {});
  |                    ++++++
```
However, this would mean `FnCtx::check_pat_top` would have to be called with an `origin_expr` in `rustc_hir_typeck::check::check_fn`, and that expr would have to be somehow plumbed through `FnCtxt::check_expr_closure` and closure signature deduction. I'm willing to work on the plumbing but unsure how to start.
@SparkyPotato SparkyPotato deleted the fix-126903 branch September 11, 2024 15:50
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid suggestion to .await on a function argument pattern
5 participants