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

Suggest calling await on method call and field access #78297

Merged
merged 1 commit into from
Oct 28, 2020
Merged

Suggest calling await on method call and field access #78297

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion 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 @@ -1669,7 +1669,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
self.note_error_origin(diag, cause, exp_found);
}

fn get_impl_future_output_ty(&self, ty: Ty<'tcx>) -> Option<Ty<'tcx>> {
pub fn get_impl_future_output_ty(&self, ty: Ty<'tcx>) -> Option<Ty<'tcx>> {
Copy link
Contributor

Choose a reason for hiding this comment

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

https://github.com/rust-lang/rust/pull/76765/files#diff-b5c71ca5f71c77ba51507d5ba5cfe4afa15578aaa8f2f0385ef88423cb69744fR88 has a similar fn but it operates on rustc_*hir*::Ty<'_> instead of this, which I think is rustc_middle::Ty<'_>

Should they be merged? In that PR, we specifically need to check the HIR because we need to know if its an async fn, but after that we could switch to using rustc_middle (can you get an rustc_middle::Ty with a local def id?)

That fn also just checks the bound against the symbol Output, this seems to find the future trait def and compare against that, that may be better (but I am not sure if rustc_hir::Ty makes that easy)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whenever you have a ty::Ty<'tcx> it is easier to do it this way, when you have access to the hir::Ty that comes from the return type of the hir fn return type it easier to use that directly.

We might be able to unify these two methods, but I've found in the past that I've had to have the same logic twice, one for the hir and another for ty, so I wouldn't be too torn about the "duplication".

Copy link
Contributor

Choose a reason for hiding this comment

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

perhaps we can merge these separately and then if I find time I can see how hard it is to merge!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That sounds good.

if let ty::Opaque(def_id, substs) = ty.kind() {
let future_trait = self.tcx.require_lang_item(LangItem::Future, None);
// Future::Output
Expand Down
71 changes: 27 additions & 44 deletions compiler/rustc_typeck/src/check/expr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ use rustc_middle::ty::{AdtKind, Visibility};
use rustc_span::hygiene::DesugaringKind;
use rustc_span::source_map::Span;
use rustc_span::symbol::{kw, sym, Ident, Symbol};
use rustc_trait_selection::traits::{self, ObligationCauseCode, SelectionContext};
use rustc_trait_selection::traits::{self, ObligationCauseCode};

use std::fmt::Display;

Expand Down Expand Up @@ -1580,51 +1580,34 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
err: &mut DiagnosticBuilder<'_>,
field_ident: Ident,
base: &'tcx hir::Expr<'tcx>,
expr: &'tcx hir::Expr<'tcx>,
def_id: DefId,
ty: Ty<'tcx>,
) {
let param_env = self.tcx().param_env(def_id);
let future_trait = self.tcx.require_lang_item(LangItem::Future, None);
// Future::Output
let item_def_id =
self.tcx.associated_items(future_trait).in_definition_order().next().unwrap().def_id;

let projection_ty = self.tcx.projection_ty_from_predicates((def_id, item_def_id));
debug!("suggest_await_on_field_access: projection_ty={:?}", projection_ty);

let cause = self.misc(expr.span);
let mut selcx = SelectionContext::new(&self.infcx);

let mut obligations = vec![];
if let Some(projection_ty) = projection_ty {
let normalized_ty = rustc_trait_selection::traits::normalize_projection_type(
&mut selcx,
param_env,
projection_ty,
cause,
0,
&mut obligations,
);
debug!(
"suggest_await_on_field_access: normalized_ty={:?}, ty_kind={:?}",
self.resolve_vars_if_possible(&normalized_ty),
normalized_ty.kind(),
);
if let ty::Adt(def, _) = normalized_ty.kind() {
// no field access on enum type
if !def.is_enum() {
if def.non_enum_variant().fields.iter().any(|field| field.ident == field_ident)
{
err.span_suggestion_verbose(
base.span.shrink_to_hi(),
"consider awaiting before field access",
".await".to_string(),
Applicability::MaybeIncorrect,
);
}
let output_ty = match self.infcx.get_impl_future_output_ty(ty) {
Some(output_ty) => self.resolve_vars_if_possible(&output_ty),
_ => return,
};
let mut add_label = true;
if let ty::Adt(def, _) = output_ty.kind() {
// no field access on enum type
if !def.is_enum() {
if def.non_enum_variant().fields.iter().any(|field| field.ident == field_ident) {
add_label = false;
err.span_label(
field_ident.span,
"field not available in `impl Future`, but it is available in its `Output`",
);
err.span_suggestion_verbose(
base.span.shrink_to_hi(),
"consider `await`ing on the `Future` and access the field of its `Output`",
".await".to_string(),
Applicability::MaybeIncorrect,
);
}
}
}
if add_label {
err.span_label(field_ident.span, &format!("field not found in `{}`", ty));
}
}

fn ban_nonexisting_field(
Expand Down Expand Up @@ -1653,8 +1636,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
ty::Param(param_ty) => {
self.point_at_param_definition(&mut err, param_ty);
}
ty::Opaque(def_id, _) => {
self.suggest_await_on_field_access(&mut err, field, base, expr, def_id);
ty::Opaque(_, _) => {
self.suggest_await_on_field_access(&mut err, field, base, expr_t.peel_refs());
}
_ => {}
}
Expand Down
54 changes: 13 additions & 41 deletions compiler/rustc_typeck/src/check/method/suggest.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ use rustc_span::symbol::{kw, sym, Ident};
use rustc_span::{source_map, FileName, Span};
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt;
use rustc_trait_selection::traits::Obligation;
use rustc_trait_selection::traits::SelectionContext;

use std::cmp::Ordering;

Expand Down Expand Up @@ -870,46 +869,19 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
call: &hir::Expr<'_>,
span: Span,
) {
if let ty::Opaque(def_id, _) = *ty.kind() {
let future_trait = self.tcx.require_lang_item(LangItem::Future, None);
// Future::Output
let item_def_id = self
.tcx
.associated_items(future_trait)
.in_definition_order()
.next()
.unwrap()
.def_id;

let projection_ty = self.tcx.projection_ty_from_predicates((def_id, item_def_id));
let cause = self.misc(span);
let mut selcx = SelectionContext::new(&self.infcx);
let mut obligations = vec![];
if let Some(projection_ty) = projection_ty {
let normalized_ty = rustc_trait_selection::traits::normalize_projection_type(
&mut selcx,
self.param_env,
projection_ty,
cause,
0,
&mut obligations,
);
debug!(
"suggest_await_before_method: normalized_ty={:?}, ty_kind={:?}",
self.resolve_vars_if_possible(&normalized_ty),
normalized_ty.kind(),
);
let method_exists = self.method_exists(item_name, normalized_ty, call.hir_id, true);
debug!("suggest_await_before_method: is_method_exist={}", method_exists);
if method_exists {
err.span_suggestion_verbose(
span.shrink_to_lo(),
"consider awaiting before this method call",
"await.".to_string(),
Applicability::MaybeIncorrect,
);
}
}
let output_ty = match self.infcx.get_impl_future_output_ty(ty) {
Some(output_ty) => self.resolve_vars_if_possible(&output_ty),
_ => return,
};
let method_exists = self.method_exists(item_name, output_ty, call.hir_id, true);
debug!("suggest_await_before_method: is_method_exist={}", method_exists);
if method_exists {
err.span_suggestion_verbose(
span.shrink_to_lo(),
"consider `await`ing on the `Future` and calling the method on its `Output`",
"await.".to_string(),
Applicability::MaybeIncorrect,
);
}
}

Expand Down
29 changes: 27 additions & 2 deletions 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 @@ -40,6 +40,14 @@ async fn foo() -> Result<(), ()> {

async fn bar() -> Result<(), ()> {
foo()?; //~ ERROR the `?` operator can only be applied to values that implement `Try`
//~^ NOTE the `?` operator cannot be applied to type `impl Future`
//~| HELP the trait `Try` is not implemented for `impl Future`
//~| NOTE required by `into_result`
//~| HELP consider `await`ing on the `Future`
//~| NOTE in this expansion of desugaring of operator `?`
//~| NOTE in this expansion of desugaring of operator `?`
//~| NOTE in this expansion of desugaring of operator `?`
//~| NOTE in this expansion of desugaring of operator `?`
Ok(())
}

Expand All @@ -48,25 +56,42 @@ async fn struct_() -> Struct {
}

async fn tuple() -> Tuple {
//~^ NOTE the `Output` of this `async fn`'s expected opaque type
Tuple(1i32)
}

async fn baz() -> Result<(), ()> {
let t = T;
t?; //~ ERROR the `?` operator can only be applied to values that implement `Try`
//~^ NOTE the `?` operator cannot be applied to type `T`
//~| HELP the trait `Try` is not implemented for `T`
//~| NOTE required by `into_result`
//~| HELP consider `await`ing on the `Future`
//~| NOTE in this expansion of desugaring of operator `?`
//~| NOTE in this expansion of desugaring of operator `?`
//~| NOTE in this expansion of desugaring of operator `?`
//~| NOTE in this expansion of desugaring of operator `?`


let _: i32 = tuple().0; //~ ERROR no field `0`
//~^ HELP consider `await`ing on the `Future`
//~| NOTE field not available in `impl Future`

let _: i32 = struct_().a; //~ ERROR no field `a`
//~^ HELP consider `await`ing on the `Future`
//~| NOTE field not available in `impl Future`

struct_().method(); //~ ERROR no method named

//~^ NOTE method not found in `impl Future`
//~| HELP consider `await`ing on the `Future`
Ok(())
}

async fn match_() {
match tuple() {
match tuple() { //~ HELP consider `await`ing on the `Future`
Tuple(_) => {} //~ ERROR mismatched types
//~^ NOTE expected opaque type, found struct `Tuple`
//~| NOTE expected opaque type `impl Future`
}
}

Expand Down
29 changes: 22 additions & 7 deletions 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 @@ -12,7 +12,7 @@ LL | foo().await?;
| ^^^^^^

error[E0277]: the `?` operator can only be applied to values that implement `Try`
--> $DIR/issue-61076.rs:56:5
--> $DIR/issue-61076.rs:65:5
|
LL | t?;
| ^^ the `?` operator cannot be applied to type `T`
Expand All @@ -25,25 +25,40 @@ LL | t.await?;
| ^^^^^^

error[E0609]: no field `0` on type `impl Future`
--> $DIR/issue-61076.rs:58:26
--> $DIR/issue-61076.rs:76:26
|
LL | let _: i32 = tuple().0;
| ^
| ^ field not available in `impl Future`, but it is available in its `Output`
|
help: consider `await`ing on the `Future` and access the field of its `Output`
|
LL | let _: i32 = tuple().await.0;
| ^^^^^^

error[E0609]: no field `a` on type `impl Future`
--> $DIR/issue-61076.rs:60:28
--> $DIR/issue-61076.rs:80:28
|
LL | let _: i32 = struct_().a;
| ^
| ^ field not available in `impl Future`, but it is available in its `Output`
|
help: consider `await`ing on the `Future` and access the field of its `Output`
|
LL | let _: i32 = struct_().await.a;
| ^^^^^^

error[E0599]: no method named `method` found for opaque type `impl Future` in the current scope
--> $DIR/issue-61076.rs:62:15
--> $DIR/issue-61076.rs:84:15
|
LL | struct_().method();
| ^^^^^^ method not found in `impl Future`
|
help: consider `await`ing on the `Future` and calling the method on its `Output`
|
LL | struct_().await.method();
| ^^^^^^

error[E0308]: mismatched types
--> $DIR/issue-61076.rs:69:9
--> $DIR/issue-61076.rs:92:9
|
LL | async fn tuple() -> Tuple {
| ----- the `Output` of this `async fn`'s expected opaque type
Expand Down