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

Nellshamrell/improve async error #86705

Closed
wants to merge 6 commits into from
Closed

Nellshamrell/improve async error #86705

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
93ef6b3
slight improvement to error message - indicates that a future was exp…
nellshamrell Jun 4, 2021
0ec98df
removes unnecessary trait
nellshamrell Jun 28, 2021
686eaf4
slight modification to error message
nellshamrell Jul 23, 2021
0f05538
fixes formatting error
nellshamrell Jul 23, 2021
4eddf27
combines if else statements
nellshamrell Aug 1, 2021
11dccad
renames variables to be more descriptive
nellshamrell Aug 1, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 18 additions & 27 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1481,35 +1481,27 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
target: &str,
types: &FxHashMap<TyCategory, FxHashSet<Span>>,
) {
for (key, values) in types.iter() {
let count = values.len();
let kind = key.descr();
for (ty_category, spans) in types.iter() {
let count = spans.len();
let kind = ty_category.descr();
let mut returned_async_output_error = false;
for sp in values {
err.span_label(
*sp,
format!(
"{}{}{} {}{}",
if sp.is_desugaring(DesugaringKind::Async)
&& !returned_async_output_error
{
"checked the `Output` of this `async fn`, "
} else if count == 1 {
"the "
} else {
""
},
if count > 1 { "one of the " } else { "" },
target,
kind,
pluralize!(count),
),
);
if sp.is_desugaring(DesugaringKind::Async)
&& returned_async_output_error == false
{
for sp in spans {
if sp.is_desugaring(DesugaringKind::Async) && !returned_async_output_error {
err.span_label(*sp, format!("{}", "async functions return futures"));
err.note("while checking the return type of the `async fn`");
returned_async_output_error = true;
} else {
err.span_label(
*sp,
format!(
"{}{}{} {}{}",
if count == 1 { "the " } else { "" },
if count > 1 { "one of the " } else { "" },
target,
kind,
pluralize!(count),
),
);
}
}
}
Expand Down Expand Up @@ -1768,7 +1760,6 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
if let ObligationCauseCode::CompareImplMethodObligation { .. } = &cause.code {
return;
}

match (
self.get_impl_future_output_ty(exp_found.expected),
self.get_impl_future_output_ty(exp_found.found),
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/async-await/dont-suggest-missing-await.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ 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
| --- async functions return futures
Copy link
Contributor

Choose a reason for hiding this comment

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

Love the new output! It is certainly better.

I wonder if we should (maybe in the future?) give output similar to "async function make_u32 returns an impl Future<Output = u32>". I know I've pushed for that in the past and not everyone is happy with that, but I fear that we are not taking advantage of teaching people about what "futures" are at the type level. Having said that, we could reserve a more "accurate" phrasing for specific cases (like when we suggest appending .await to a function call), and maybe make it part of a note.

Copy link
Contributor

@estebank estebank Aug 2, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I'd forgotten I had #87673 open that touches the same general place, mainly moving the span_label to a span_note using a MultiSpan to have more text. I wonder if instead of landing my PR you could incorporate some of the changes I made?

I just spent a few minutes trying to accomplish what you mentioned (use the name of the method) and this is what I landed at, which I feel looks "pretty neat", but I want us to go for "easy to understand by newcomers":

error[E0308]: mismatched types
  --> src/test/ui/async-await/dont-suggest-missing-await.rs:14:18
   |
14 |         take_u32(x)
   |                  ^ expected `u32`, found opaque type
   |
note: while checking the return type of the async function `make_u32`
  --> src/test/ui/async-await/dont-suggest-missing-await.rs:7:24
   |
7  | async fn make_u32() -> u32 {
   |          --------      ^^^ the desugared version of `make_u32` returns `impl Future<Output = u32>`
   |          |
   |          async functions return futures
   = note:     expected type `u32`
           found opaque type `impl Future`
help: consider `await`ing on the `Future`
   |
14 |         take_u32(x.await)
   |                   ^^^^^^

Screen Shot 2021-08-02 at 7 36 59 AM

I'll push the code to #87673 in a minute and you can tell me if you can pick from it so you can run with it and make it your own if there are more things we want to do here :) (because I'm not satisfied with the wording I threw in there, but the code to get the spans and names is still valid).

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh! While I was getting this working, the PR got approved and selected in a rollup (otherwise I'd kill it). We'll have to rebase this PR after that lands, sorry about that :-/

The follow up code the above screenshot came from is at bfcaf8b. Would you have the time/desire to take that and make it the best output it can be?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure! I likely won't have time until next weekend, but I would be happy to! I'll likely open a new PR with that change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And ty so much for the help!

...
LL | take_u32(x)
| ^ expected `u32`, found opaque type
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/async-await/generator-desc.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ error[E0308]: mismatched types
--> $DIR/generator-desc.rs:12:16
|
LL | async fn one() {}
| - checked the `Output` of this `async fn`, expected opaque type
| - async functions return futures
LL | async fn two() {}
| - checked the `Output` of this `async fn`, found opaque type
| - async functions return futures
...
LL | fun(one(), two());
| ^^^^^ expected opaque type, found a different opaque type
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/async-await/issue-61076.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ async fn struct_() -> Struct {
}

async fn tuple() -> Tuple {
//~^ NOTE checked the `Output` of this `async fn`, expected opaque type
//~^ NOTE async functions return futures
Tuple(1i32)
}

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/async-await/issue-61076.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ error[E0308]: mismatched types
--> $DIR/issue-61076.rs:92:9
|
LL | async fn tuple() -> Tuple {
| ----- checked the `Output` of this `async fn`, expected opaque type
| ----- async functions return futures
...
LL | Tuple(_) => {}
| ^^^^^^^^ expected opaque type, found struct `Tuple`
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/async-await/suggest-missing-await-closure.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error[E0308]: mismatched types
--> $DIR/suggest-missing-await-closure.rs:16:18
|
LL | async fn make_u32() -> u32 {
| --- checked the `Output` of this `async fn`, found opaque type
| --- async functions return futures
...
LL | take_u32(x)
| ^ expected `u32`, found opaque type
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/async-await/suggest-missing-await.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error[E0308]: mismatched types
--> $DIR/suggest-missing-await.rs:12:14
|
LL | async fn make_u32() -> u32 {
| --- checked the `Output` of this `async fn`, found opaque type
| --- async functions return futures
...
LL | take_u32(x)
| ^ expected `u32`, found opaque type
Expand All @@ -19,7 +19,7 @@ error[E0308]: mismatched types
--> $DIR/suggest-missing-await.rs:22:5
|
LL | async fn dummy() {}
| - checked the `Output` of this `async fn`, found opaque type
| - async functions return futures
...
LL | dummy()
| ^^^^^^^ expected `()`, found opaque type
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/parser/fn-header-semantic-fail.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ LL | async fn ft1();
LL | async fn ft1() {}
| ^
| |
| checked the `Output` of this `async fn`, found opaque type
| async functions return futures
| expected `()`, found opaque type
|
= note: while checking the return type of the `async fn`
Expand All @@ -205,7 +205,7 @@ LL | const async unsafe extern "C" fn ft5();
LL | const async unsafe extern "C" fn ft5() {}
| ^
| |
| checked the `Output` of this `async fn`, found opaque type
| async functions return futures
| expected `()`, found opaque type
|
= note: while checking the return type of the `async fn`
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/resolve/issue-70736-async-fn-no-body-def-collector.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ LL | async fn associated();
LL | async fn associated();
| ^
| |
| checked the `Output` of this `async fn`, found opaque type
| async functions return futures
| expected `()`, found opaque type
|
= note: while checking the return type of the `async fn`
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/suggestions/issue-81839.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ LL | | }
::: $DIR/auxiliary/issue-81839.rs:6:49
|
LL | pub async fn answer_str(&self, _s: &str) -> Test {
| ---- checked the `Output` of this `async fn`, found opaque type
| ---- async functions return futures
|
= note: while checking the return type of the `async fn`
= note: expected type `()`
Expand Down
6 changes: 3 additions & 3 deletions src/test/ui/suggestions/match-prev-arm-needing-semi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ fn extra_semicolon() {
};
}

async fn async_dummy() {} //~ NOTE checked the `Output` of this `async fn`, found opaque type
async fn async_dummy2() {} //~ NOTE checked the `Output` of this `async fn`, found opaque type
//~| NOTE checked the `Output` of this `async fn`, found opaque type
async fn async_dummy() {} //~ NOTE async functions return futures
async fn async_dummy2() {} //~ NOTE async functions return futures
//~| NOTE async functions return futures

async fn async_extra_semicolon_same() {
let _ = match true { //~ NOTE `match` arms have incompatible types
Expand Down
6 changes: 3 additions & 3 deletions src/test/ui/suggestions/match-prev-arm-needing-semi.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error[E0308]: `match` arms have incompatible types
--> $DIR/match-prev-arm-needing-semi.rs:26:18
|
LL | async fn async_dummy() {}
| - checked the `Output` of this `async fn`, found opaque type
| - async functions return futures
...
LL | let _ = match true {
| _____________-
Expand Down Expand Up @@ -34,7 +34,7 @@ error[E0308]: `match` arms have incompatible types
--> $DIR/match-prev-arm-needing-semi.rs:40:18
|
LL | async fn async_dummy2() {}
| - checked the `Output` of this `async fn`, found opaque type
| - async functions return futures
...
LL | let _ = match true {
| _____________-
Expand Down Expand Up @@ -69,7 +69,7 @@ error[E0308]: `match` arms have incompatible types
--> $DIR/match-prev-arm-needing-semi.rs:52:18
|
LL | async fn async_dummy2() {}
| - checked the `Output` of this `async fn`, found opaque type
| - async functions return futures
...
LL | let _ = match true {
| _____________-
Expand Down