Skip to content

Commit

Permalink
When .await is called on a non-Future expression, suggest removal
Browse files Browse the repository at this point in the history
Keep track of the origin of a `T: Future` obligation when caused by an
`.await` expression.

Address #66731.
  • Loading branch information
estebank committed Dec 13, 2021
1 parent 06a6674 commit 7227a87
Show file tree
Hide file tree
Showing 36 changed files with 190 additions and 121 deletions.
5 changes: 3 additions & 2 deletions compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -607,6 +607,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
/// }
/// ```
fn lower_expr_await(&mut self, await_span: Span, expr: &Expr) -> hir::ExprKind<'hir> {
let dot_await_span = expr.span.shrink_to_hi().to(await_span);
match self.generator_kind {
Some(hir::GeneratorKind::Async(_)) => {}
Some(hir::GeneratorKind::Gen) | None => {
Expand All @@ -623,7 +624,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
err.emit();
}
}
let span = self.mark_span_with_reason(DesugaringKind::Await, await_span, None);
let span = self.mark_span_with_reason(DesugaringKind::Await, dot_await_span, None);
let gen_future_span = self.mark_span_with_reason(
DesugaringKind::Await,
await_span,
Expand Down Expand Up @@ -682,7 +683,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
let break_x = self.with_loop_scope(loop_node_id, move |this| {
let expr_break =
hir::ExprKind::Break(this.lower_loop_destination(None), Some(x_expr));
this.arena.alloc(this.expr(await_span, expr_break, ThinVec::new()))
this.arena.alloc(this.expr(span, expr_break, ThinVec::new()))
});
self.arm(ready_pat, break_x)
};
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_middle/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,8 @@ pub enum ObligationCauseCode<'tcx> {
/// If `X` is the concrete type of an opaque type `impl Y`, then `X` must implement `Y`
OpaqueType,

AwaitableExpr,

/// Well-formed checking. If a `WellFormedLoc` is provided,
/// then it will be used to eprform HIR-based wf checking
/// after an error occurs, in order to generate a more precise error span.
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2838,8 +2838,8 @@ impl<'a> Parser<'a> {
ExprKind::Call(f, args)
}

fn mk_await_expr(&mut self, self_arg: P<Expr>, lo: Span) -> P<Expr> {
let span = lo.to(self.prev_token.span);
fn mk_await_expr(&mut self, self_arg: P<Expr>, _lo: Span) -> P<Expr> {
let span = self.prev_token.span;
let await_expr = self.mk_expr(span, ExprKind::Await(self_arg), AttrVec::new());
self.recover_from_await_method_call();
await_expr
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
self.suggest_remove_reference(&obligation, &mut err, trait_ref);
self.suggest_semicolon_removal(&obligation, &mut err, span, trait_ref);
self.note_version_mismatch(&mut err, &trait_ref);
self.suggest_remove_await(&obligation, &mut err);

if Some(trait_ref.def_id()) == tcx.lang_items().try_trait() {
self.suggest_await_before_try(&mut err, &obligation, trait_ref, span);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,12 @@ pub trait InferCtxtExt<'tcx> {
trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>,
);

fn suggest_remove_await(
&self,
obligation: &PredicateObligation<'tcx>,
err: &mut DiagnosticBuilder<'_>,
);

fn suggest_change_mut(
&self,
obligation: &PredicateObligation<'tcx>,
Expand Down Expand Up @@ -873,6 +879,25 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
}
}

fn suggest_remove_await(
&self,
obligation: &PredicateObligation<'tcx>,
err: &mut DiagnosticBuilder<'_>,
) {
let span = obligation.cause.span;

if let ObligationCauseCode::AwaitableExpr = obligation.cause.code {
// FIXME: use `trait_ref.self_ty().no_bound_vars()` to typecheck if `()` and if not
// maybe suggest returning instead?
err.span_suggestion_verbose(
span,
"do not `.await` the expression",
String::new(),
Applicability::MachineApplicable,
);
}
}

/// Check if the trait bound is implemented for a different mutability and note it in the
/// final error.
fn suggest_change_mut(
Expand Down Expand Up @@ -1935,6 +1960,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
| ObligationCauseCode::ReturnType
| ObligationCauseCode::ReturnValue(_)
| ObligationCauseCode::BlockTailExpression(_)
| ObligationCauseCode::AwaitableExpr
| ObligationCauseCode::LetElse => {}
ObligationCauseCode::SliceOrArrayElem => {
err.note("slice and array elements must have `Sized` type");
Expand Down
31 changes: 28 additions & 3 deletions compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -804,7 +804,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let ty = item_ty.subst(self.tcx, substs);

self.write_resolution(hir_id, Ok((def_kind, def_id)));
self.add_required_obligations(span, def_id, &substs);
self.add_required_obligations_with_code(
span,
def_id,
&substs,
match lang_item {
hir::LangItem::FuturePoll => ObligationCauseCode::AwaitableExpr,
// FIXME: see if there are other obligation specializations we could do here beyond
// what we do above for `.await`.
_ => traits::ItemObligation(def_id),
},
);
(Res::Def(def_kind, def_id), ty)
}

Expand Down Expand Up @@ -1486,12 +1496,27 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}

/// Add all the obligations that are required, substituting and normalized appropriately.
#[tracing::instrument(level = "debug", skip(self, span, def_id, substs))]
crate fn add_required_obligations(&self, span: Span, def_id: DefId, substs: &SubstsRef<'tcx>) {
self.add_required_obligations_with_code(
span,
def_id,
substs,
traits::ItemObligation(def_id),
)
}

#[tracing::instrument(level = "debug", skip(self, span, def_id, substs))]
fn add_required_obligations_with_code(
&self,
span: Span,
def_id: DefId,
substs: &SubstsRef<'tcx>,
code: ObligationCauseCode<'tcx>,
) {
let (bounds, _) = self.instantiate_bounds(span, def_id, &substs);

for obligation in traits::predicates_for_generics(
traits::ObligationCause::new(span, self.body_id, traits::ItemObligation(def_id)),
traits::ObligationCause::new(span, self.body_id, code),
self.param_env,
bounds,
) {
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/async-await/async-error-span.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ LL | let a;
| ^ cannot infer type
|
note: the type is part of the `async fn` body because of this `await`
--> $DIR/async-error-span.rs:14:5
--> $DIR/async-error-span.rs:14:17
|
LL | get_future().await;
| ^^^^^^^^^^^^^^^^^^
| ^^^^^^

error: aborting due to 2 previous errors

Expand Down
12 changes: 6 additions & 6 deletions src/test/ui/async-await/async-fn-nonsend.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ LL | assert_send(local_dropped_before_await());
|
= help: within `impl Future<Output = ()>`, the trait `Send` is not implemented for `Rc<()>`
note: future is not `Send` as this value is used across an await
--> $DIR/async-fn-nonsend.rs:24:5
--> $DIR/async-fn-nonsend.rs:24:10
|
LL | let x = non_send();
| - has type `impl Debug` which is not `Send`
LL | drop(x);
LL | fut().await;
| ^^^^^^^^^^^ await occurs here, with `x` maybe used later
| ^^^^^^ await occurs here, with `x` maybe used later
LL | }
| - `x` is later dropped here
note: required by a bound in `assert_send`
Expand All @@ -29,12 +29,12 @@ LL | assert_send(non_send_temporary_in_match());
|
= help: within `impl Future<Output = ()>`, the trait `Send` is not implemented for `Rc<()>`
note: future is not `Send` as this value is used across an await
--> $DIR/async-fn-nonsend.rs:33:20
--> $DIR/async-fn-nonsend.rs:33:25
|
LL | match Some(non_send()) {
| ---------- has type `impl Debug` which is not `Send`
LL | Some(_) => fut().await,
| ^^^^^^^^^^^ await occurs here, with `non_send()` maybe used later
| ^^^^^^ await occurs here, with `non_send()` maybe used later
...
LL | }
| - `non_send()` is later dropped here
Expand All @@ -52,13 +52,13 @@ LL | assert_send(non_sync_with_method_call());
|
= help: the trait `Send` is not implemented for `dyn std::fmt::Write`
note: future is not `Send` as this value is used across an await
--> $DIR/async-fn-nonsend.rs:42:9
--> $DIR/async-fn-nonsend.rs:42:14
|
LL | let f: &mut std::fmt::Formatter = panic!();
| - has type `&mut Formatter<'_>` which is not `Send`
LL | if non_sync().fmt(f).unwrap() == () {
LL | fut().await;
| ^^^^^^^^^^^ await occurs here, with `f` maybe used later
| ^^^^^^ await occurs here, with `f` maybe used later
LL | }
LL | }
| - `f` is later dropped here
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,52 +162,52 @@ LL | let _ = (await bar())?;
| ^^^^^^^^^^^ only allowed inside `async` functions and blocks

error[E0728]: `await` is only allowed inside `async` functions and blocks
--> $DIR/incorrect-syntax-suggestions.rs:71:13
--> $DIR/incorrect-syntax-suggestions.rs:71:19
|
LL | fn foo13() -> Result<(), ()> {
| ----- this is not `async`
LL | let _ = bar().await();
| ^^^^^^^^^^^ only allowed inside `async` functions and blocks
| ^^^^^ only allowed inside `async` functions and blocks

error[E0728]: `await` is only allowed inside `async` functions and blocks
--> $DIR/incorrect-syntax-suggestions.rs:76:13
--> $DIR/incorrect-syntax-suggestions.rs:76:19
|
LL | fn foo14() -> Result<(), ()> {
| ----- this is not `async`
LL | let _ = bar().await()?;
| ^^^^^^^^^^^ only allowed inside `async` functions and blocks
| ^^^^^ only allowed inside `async` functions and blocks

error[E0728]: `await` is only allowed inside `async` functions and blocks
--> $DIR/incorrect-syntax-suggestions.rs:81:13
--> $DIR/incorrect-syntax-suggestions.rs:81:19
|
LL | fn foo15() -> Result<(), ()> {
| ----- this is not `async`
LL | let _ = bar().await;
| ^^^^^^^^^^^ only allowed inside `async` functions and blocks
| ^^^^^ only allowed inside `async` functions and blocks

error[E0728]: `await` is only allowed inside `async` functions and blocks
--> $DIR/incorrect-syntax-suggestions.rs:85:13
--> $DIR/incorrect-syntax-suggestions.rs:85:19
|
LL | fn foo16() -> Result<(), ()> {
| ----- this is not `async`
LL | let _ = bar().await?;
| ^^^^^^^^^^^ only allowed inside `async` functions and blocks
| ^^^^^ only allowed inside `async` functions and blocks

error[E0728]: `await` is only allowed inside `async` functions and blocks
--> $DIR/incorrect-syntax-suggestions.rs:90:17
--> $DIR/incorrect-syntax-suggestions.rs:90:23
|
LL | fn foo() -> Result<(), ()> {
| --- this is not `async`
LL | let _ = bar().await?;
| ^^^^^^^^^^^ only allowed inside `async` functions and blocks
| ^^^^^ only allowed inside `async` functions and blocks

error[E0728]: `await` is only allowed inside `async` functions and blocks
--> $DIR/incorrect-syntax-suggestions.rs:97:17
--> $DIR/incorrect-syntax-suggestions.rs:97:23
|
LL | let foo = || {
| -- this is not `async`
LL | let _ = bar().await?;
| ^^^^^^^^^^^ only allowed inside `async` functions and blocks
| ^^^^^ only allowed inside `async` functions and blocks

error[E0728]: `await` is only allowed inside `async` functions and blocks
--> $DIR/incorrect-syntax-suggestions.rs:113:17
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/async-await/issue-64130-1-sync.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ LL | is_sync(bar());
|
= help: within `impl Future<Output = ()>`, the trait `Sync` is not implemented for `Foo`
note: future is not `Sync` as this value is used across an await
--> $DIR/issue-64130-1-sync.rs:15:5
--> $DIR/issue-64130-1-sync.rs:15:10
|
LL | let x = Foo;
| - has type `Foo` which is not `Sync`
LL | baz().await;
| ^^^^^^^^^^^ await occurs here, with `x` maybe used later
| ^^^^^^ await occurs here, with `x` maybe used later
LL | }
| - `x` is later dropped here
note: required by a bound in `is_sync`
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/async-await/issue-64130-2-send.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ LL | is_send(bar());
|
= help: within `impl Future<Output = ()>`, the trait `Send` is not implemented for `Foo`
note: future is not `Send` as this value is used across an await
--> $DIR/issue-64130-2-send.rs:15:5
--> $DIR/issue-64130-2-send.rs:15:10
|
LL | let x = Foo;
| - has type `Foo` which is not `Send`
LL | baz().await;
| ^^^^^^^^^^^ await occurs here, with `x` maybe used later
| ^^^^^^ await occurs here, with `x` maybe used later
LL | }
| - `x` is later dropped here
note: required by a bound in `is_send`
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/async-await/issue-64130-3-other.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ LL | is_qux(bar());
| ^^^^^ within `impl Future<Output = ()>`, the trait `Qux` is not implemented for `Foo`
|
note: future does not implement `Qux` as this value is used across an await
--> $DIR/issue-64130-3-other.rs:18:5
--> $DIR/issue-64130-3-other.rs:18:10
|
LL | let x = Foo;
| - has type `Foo` which does not implement `Qux`
LL | baz().await;
| ^^^^^^^^^^^ await occurs here, with `x` maybe used later
| ^^^^^^ await occurs here, with `x` maybe used later
LL | }
| - `x` is later dropped here
note: required by a bound in `is_qux`
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/async-await/issue-64130-4-async-move.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ LL | pub fn foo() -> impl Future + Send {
|
= help: the trait `Sync` is not implemented for `(dyn Any + Send + 'static)`
note: future is not `Send` as this value is used across an await
--> $DIR/issue-64130-4-async-move.rs:21:26
--> $DIR/issue-64130-4-async-move.rs:21:31
|
LL | match client.status() {
| ------ has type `&Client` which is not `Send`
LL | 200 => {
LL | let _x = get().await;
| ^^^^^^^^^^^ await occurs here, with `client` maybe used later
| ^^^^^^ await occurs here, with `client` maybe used later
...
LL | }
| - `client` is later dropped here
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ LL | is_send(foo());
|
= help: within `impl Future<Output = ()>`, the trait `Send` is not implemented for `MutexGuard<'_, u32>`
note: future is not `Send` as this value is used across an await
--> $DIR/issue-64130-non-send-future-diags.rs:17:5
--> $DIR/issue-64130-non-send-future-diags.rs:17:10
|
LL | let g = x.lock().unwrap();
| - has type `MutexGuard<'_, u32>` which is not `Send`
LL | baz().await;
| ^^^^^^^^^^^ await occurs here, with `g` maybe used later
| ^^^^^^ await occurs here, with `g` maybe used later
LL | }
| - `g` is later dropped here
note: required by a bound in `is_send`
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/async-await/issue-67252-unnamed-future.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ LL | spawn(async {
|
= help: within `impl Future<Output = [async output]>`, the trait `Send` is not implemented for `*mut ()`
note: future is not `Send` as this value is used across an await
--> $DIR/issue-67252-unnamed-future.rs:20:9
--> $DIR/issue-67252-unnamed-future.rs:20:16
|
LL | let _a = std::ptr::null_mut::<()>(); // `*mut ()` is not `Send`
| -- has type `*mut ()` which is not `Send`
LL | AFuture.await;
| ^^^^^^^^^^^^^ await occurs here, with `_a` maybe used later
| ^^^^^^ await occurs here, with `_a` maybe used later
LL | });
| - `_a` is later dropped here
note: required by a bound in `spawn`
Expand Down
1 change: 1 addition & 0 deletions src/test/ui/async-await/issue-70594.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ async fn fun() {
//~| error: `.await` is not allowed in a `const`
//~| error: `.await` is not allowed in a `const`
//~| error: `()` is not a future
//~| error: `()` is not a future
}

fn main() {}
Loading

0 comments on commit 7227a87

Please sign in to comment.