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.

Sign up for GitHub

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

Jump to bottom

Tweak errors coming from for-loop, ? and .await desugaring #90939

Merged
merged 16 commits into from
Dec 15, 2021
Merged

Tweak errors coming from for-loop, ? and .await desugaring #90939

merged 16 commits into from
Dec 15, 2021

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Nov 16, 2021 •
edited
Loading

  • Suggest removal of .await on non-Future expression
  • Keep track of obligations introduced by desugaring
  • Remove span pointing at method for obligation errors coming from desugaring
  • Point at called local sync fn and suggest making it async
error[E0277]: `()` is not a future
  --> $DIR/unnecessary-await.rs:9:10
   |
LL |     boo().await;
   |     -----^^^^^^ `()` is not a future
   |     |
   |     this call returns `()`
   |
   = help: the trait `Future` is not implemented for `()`
help: do not `.await` the expression
   |
LL -     boo().await;
LL +     boo();
   | 
help: alternatively, consider making `fn boo` asynchronous
   |
LL | async fn boo () {}
   | +++++

Fix #66731.

@rust-highfive
Copy link
Collaborator

r? @michaelwoerister

(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 Nov 16, 2021
@estebank estebank mentioned this pull request Nov 16, 2021
Closed
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@estebank
Copy link
Contributor Author

r? @tmandry

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@estebank estebank force-pushed the wg-af-polish branch 2 times, most recently from dc8ee04 to 7489b2a Compare November 16, 2021 20:17
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@estebank estebank force-pushed the wg-af-polish branch 2 times, most recently from bd5d07b to 4ed0001 Compare November 16, 2021 22:06
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@tmandry tmandry mentioned this pull request Nov 17, 2021
Merged
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@apiraino apiraino 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. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 18, 2021
@bors

This comment has been minimized.

Sign in to view
@bors

This comment has been minimized.

Sign in to view
tmandry
tmandry reviewed Nov 30, 2021
View reviewed changes
src/test/ui/async-await/issues/issue-65436-raw-ptr-not-send.stderr Outdated Show resolved Hide resolved
src/test/ui/async-await/unnecessary-await.rs Outdated Show resolved Hide resolved
compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs Outdated Show resolved Hide resolved
library/core/src/ops/try_trait.rs
@@ -115,15 +115,14 @@ use crate::ops::ControlFlow;
#[unstable(feature = "try_trait_v2", issue = "84277")]
#[rustc_on_unimplemented(
on(
all(from_method = "from_output", from_desugaring = "TryBlock"),
all(from_desugaring = "TryBlock"),
Copy link
Member

Choose a reason for hiding this comment

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

Why are the changes in this file needed?

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 originally constrained these a lot to avoid false-positives, but given how the desugaring is applied, it wasn't necessary. But with the changes to the desugaring there were now some cases where we had TryBlock, but weren't coming from the method call (IIRC, we perform deduplication and a separate obligation ended up being selected for display). Removing the overspicificity fixed those regressions.

@bors
Copy link
Contributor

bors commented Dec 3, 2021

☔ The latest upstream changes (presumably #90737) made this pull request unmergeable. Please resolve the merge conflicts.

@estebank
Copy link
Contributor Author

@bors r=tmandry

@bors
Copy link
Contributor

bors commented Dec 13, 2021

📌 Commit f2fc84f has been approved by tmandry

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 13, 2021
@bors
Copy link
Contributor

bors commented Dec 14, 2021

⌛ Testing commit f2fc84f with merge f8b36190bdb8713600f13cc60289beffcfceeffc...

@bors
Copy link
Contributor

bors commented Dec 14, 2021

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 14, 2021
@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-tools failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot) 
.......... (50/53)
..        (53/53)


/checkout/src/test/rustdoc-gui/search-filter.goml search-filter... FAILED
[ERROR] (line 17) TimeoutError: waiting for selector "#titles" failed: timeout 30000ms exceeded: for command `// Waiting for the search results to appear...
wait-for: "#titles"`



command did not execute successfully: "/node-v14.4.0-linux-x64/bin/node" "/checkout/src/tools/rustdoc-gui/tester.js" "--jobs" "16" "--doc-folder" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-gui/doc" "--tests-folder" "/checkout/src/test/rustdoc-gui"


Build completed unsuccessfully in 0:00:46

@tmandry
Copy link
Member

tmandry commented Dec 14, 2021

Failed due to a timeout. I guess..

@bors r+ rollup=sometimes

@bors
Copy link
Contributor

bors commented Dec 14, 2021

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Contributor

bors commented Dec 14, 2021

📌 Commit f2fc84f has been approved by tmandry

@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 14, 2021
@tmandry
Copy link
Member

tmandry commented Dec 14, 2021

@bors retry rollup=maybe

@tmandry
Copy link
Member

tmandry commented Dec 14, 2021

@bors rollup=iffy

@matthiaskrgr matthiaskrgr mentioned this pull request Dec 14, 2021
Closed
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 14, 2021
@matthiaskrgr
Rollup merge of rust-lang#90939 - estebank:wg-af-polish, r=tmandry
2666841 
Tweak errors coming from `for`-loop, `?` and `.await` desugaring

 * Suggest removal of `.await` on non-`Future` expression
 * Keep track of obligations introduced by desugaring
 * Remove span pointing at method for obligation errors coming from desugaring
 * Point at called local sync `fn` and suggest making it `async`

```
error[E0277]: `()` is not a future
  --> $DIR/unnecessary-await.rs:9:10
   |
LL |     boo().await;
   |     -----^^^^^^ `()` is not a future
   |     |
   |     this call returns `()`
   |
   = help: the trait `Future` is not implemented for `()`
help: do not `.await` the expression
   |
LL -     boo().await;
LL +     boo();
   |
help: alternatively, consider making `fn boo` asynchronous
   |
LL | async fn boo () {}
   | +++++
```

Fix rust-lang#66731.
@matthiaskrgr matthiaskrgr mentioned this pull request Dec 15, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 15, 2021
@bors
Auto merge of rust-lang#91945 - matthiaskrgr:rollup-jszf9zp, r=matthi…
195e931 
…askrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#90939 (Tweak errors coming from `for`-loop, `?` and `.await` desugaring)
 - rust-lang#91859 (Iterator::cycle() — document empty iterator special case)
 - rust-lang#91868 (Use `OutputFilenames` to generate output file for `-Zllvm-time-trace`)
 - rust-lang#91870 (Revert setting a default for the MACOSX_DEPLOYMENT_TARGET env var for linking)
 - rust-lang#91881 (Stabilize `iter::zip`)
 - rust-lang#91882 (Remove `in_band_lifetimes` from `rustc_typeck`)
 - rust-lang#91940 (Update cargo)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 272188e into rust-lang:master Dec 15, 2021
@rustbot rustbot added this to the 1.59.0 milestone Dec 15, 2021
flip1995 pushed a commit to flip1995/rust that referenced this pull request Dec 17, 2021
@matthiaskrgr
Rollup merge of rust-lang#90939 - estebank:wg-af-polish, r=tmandry
b166642 
Tweak errors coming from `for`-loop, `?` and `.await` desugaring

 * Suggest removal of `.await` on non-`Future` expression
 * Keep track of obligations introduced by desugaring
 * Remove span pointing at method for obligation errors coming from desugaring
 * Point at called local sync `fn` and suggest making it `async`

```
error[E0277]: `()` is not a future
  --> $DIR/unnecessary-await.rs:9:10
   |
LL |     boo().await;
   |     -----^^^^^^ `()` is not a future
   |     |
   |     this call returns `()`
   |
   = help: the trait `Future` is not implemented for `()`
help: do not `.await` the expression
   |
LL -     boo().await;
LL +     boo();
   |
help: alternatively, consider making `fn boo` asynchronous
   |
LL | async fn boo () {}
   | +++++
```

Fix rust-lang#66731.
@compiler-errors compiler-errors mentioned this pull request Jan 19, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@camelid camelid camelid left review comments

@tmandry tmandry tmandry left review comments

Assignees

@tmandry tmandry

Labels
A-diagnostics Area: Messages for errors, warnings, and lints 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
Milestone
1.59.0
Development

Successfully merging this pull request may close these issues.

More friendly error msg when await on NONE ASYNC fn/block or return a obj that implements deprecated Future
10 participants
@estebank @rust-highfive @rust-log-analyzer @bors @tmandry @matthiaskrgr @camelid @michaelwoerister @apiraino @rustbot