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 in more situations where infer types are involved #91022

Merged
merged 2 commits into from
Nov 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
34 changes: 31 additions & 3 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,34 @@ pub fn unexpected_hidden_region_diagnostic(
err
}

/// Structurally compares two types, modulo any inference variables.
///
/// Returns `true` if two types are equal, or if one type is an inference variable compatible
/// with the other type. A TyVar inference type is compatible with any type, and an IntVar or
/// FloatVar inference type are compatible with themselves or their concrete types (Int and
/// Float types, respectively). When comparing two ADTs, these rules apply recursively.
pub fn same_type_modulo_infer(a: Ty<'tcx>, b: Ty<'ctx>) -> bool {
match (&a.kind(), &b.kind()) {
(&ty::Adt(did_a, substs_a), &ty::Adt(did_b, substs_b)) => {
if did_a != did_b {
return false;
}

substs_a.types().zip(substs_b.types()).all(|(a, b)| same_type_modulo_infer(a, b))
}
(&ty::Int(_), &ty::Infer(ty::InferTy::IntVar(_)))
| (&ty::Infer(ty::InferTy::IntVar(_)), &ty::Int(_) | &ty::Infer(ty::InferTy::IntVar(_)))
| (&ty::Float(_), &ty::Infer(ty::InferTy::FloatVar(_)))
| (
&ty::Infer(ty::InferTy::FloatVar(_)),
&ty::Float(_) | &ty::Infer(ty::InferTy::FloatVar(_)),
)
| (&ty::Infer(ty::InferTy::TyVar(_)), _)
| (_, &ty::Infer(ty::InferTy::TyVar(_))) => true,
Comment on lines +328 to +336
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 pretty sure there's code like this somewhere.

Copy link
Member Author

@compiler-errors compiler-errors Nov 19, 2021

Choose a reason for hiding this comment

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

Yeah, this is from fn equals(Ty, Ty) -> bool.

We could possibly unify them by adding an enum Mode { None, NumInfer, Infer } or something. The complication is that this equals fn cares about the non-Ty substs in Adt, where TyS::same_ty (and the function I added) doesn't...

Thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. fn equals has to make sure the whole type are the same because this is used to determine whether it can be replaced with _ in the output. It's fine.

_ => a == b,
}
}

impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
pub fn report_region_errors(&self, errors: &Vec<RegionResolutionError<'tcx>>) {
debug!("report_region_errors(): {} errors to start", errors.len());
Expand Down Expand Up @@ -1761,7 +1789,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
self.get_impl_future_output_ty(exp_found.expected),
self.get_impl_future_output_ty(exp_found.found),
) {
(Some(exp), Some(found)) if ty::TyS::same_type(exp, found) => match &cause.code {
(Some(exp), Some(found)) if same_type_modulo_infer(exp, found) => match &cause.code {
ObligationCauseCode::IfExpression(box IfExpressionCause { then, .. }) => {
diag.multipart_suggestion(
"consider `await`ing on both `Future`s",
Expand Down Expand Up @@ -1793,15 +1821,15 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
diag.help("consider `await`ing on both `Future`s");
}
},
(_, Some(ty)) if ty::TyS::same_type(exp_found.expected, ty) => {
(_, Some(ty)) if same_type_modulo_infer(exp_found.expected, ty) => {
diag.span_suggestion_verbose(
exp_span.shrink_to_hi(),
"consider `await`ing on the `Future`",
".await".to_string(),
Applicability::MaybeIncorrect,
);
}
(Some(ty), _) if ty::TyS::same_type(ty, exp_found.found) => match cause.code {
(Some(ty), _) if same_type_modulo_infer(ty, exp_found.found) => match cause.code {
ObligationCauseCode::Pattern { span: Some(span), .. }
| ObligationCauseCode::IfExpression(box IfExpressionCause { then: span, .. }) => {
diag.span_suggestion_verbose(
Expand Down
17 changes: 17 additions & 0 deletions src/test/ui/async-await/suggest-missing-await.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,4 +54,21 @@ async fn suggest_await_on_match_expr() {
};
}

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

#[allow(unused)]
async fn suggest_await_in_generic_pattern() {
match dummy_result() {
//~^ HELP consider `await`ing on the `Future`
//~| HELP consider `await`ing on the `Future`
//~| SUGGESTION .await
Ok(_) => {}
//~^ ERROR mismatched types [E0308]
Err(_) => {}
//~^ ERROR mismatched types [E0308]
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder, can this test be marked as // run-rustfix?

Copy link
Member Author

Choose a reason for hiding this comment

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

// run-rustfix seems to produce broken output.

  1. In suggest_await_in_async_fn_return, the "add a semicolon" and "add .await" suggestions get applied out of order to produce dummy();.await
  2. In suggest_await_in_generic_pattern (the test I added), it adds .await twice, because there are two type errors due to the Ok and Err branches.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I see. Fair enough, leave as is.

fn main() {}
38 changes: 37 additions & 1 deletion src/test/ui/async-await/suggest-missing-await.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,42 @@ help: consider `await`ing on the `Future`
LL | let _x = match dummy().await {
| ++++++

error: aborting due to 5 previous errors
error[E0308]: mismatched types
--> $DIR/suggest-missing-await.rs:67:9
|
LL | Ok(_) => {}
| ^^^^^ expected opaque type, found enum `Result`
|
note: while checking the return type of the `async fn`
--> $DIR/suggest-missing-await.rs:57:28
|
LL | async fn dummy_result() -> Result<(), ()> {
| ^^^^^^^^^^^^^^ checked the `Output` of this `async fn`, expected opaque type
= note: expected opaque type `impl Future<Output = Result<(), ()>>`
found enum `Result<_, _>`
help: consider `await`ing on the `Future`
|
LL | match dummy_result().await {
| ++++++

error[E0308]: mismatched types
--> $DIR/suggest-missing-await.rs:69:9
|
LL | Err(_) => {}
| ^^^^^^ expected opaque type, found enum `Result`
|
note: while checking the return type of the `async fn`
--> $DIR/suggest-missing-await.rs:57:28
|
LL | async fn dummy_result() -> Result<(), ()> {
| ^^^^^^^^^^^^^^ checked the `Output` of this `async fn`, expected opaque type
= note: expected opaque type `impl Future<Output = Result<(), ()>>`
found enum `Result<_, _>`
help: consider `await`ing on the `Future`
|
LL | match dummy_result().await {
| ++++++

error: aborting due to 7 previous errors

For more information about this error, try `rustc --explain E0308`.