Skip to content

Commit

Permalink
Rollup merge of rust-lang#110823 - compiler-errors:tweak-await-span, …
Browse files Browse the repository at this point in the history
…r=b-naber

Tweak await span to not contain dot

Fixes a discrepancy between method calls and await expressions where the latter are desugared to have a span that *contains* the dot (i.e. `.await`) but method call identifiers don't contain the dot. This leads to weird suggestions suggestions in borrowck -- see linked issue.

Fixes rust-lang#110761

This mostly touches a bunch of tests to tighten their `await` span.
  • Loading branch information
matthiaskrgr authored Apr 30, 2023
2 parents 9ecda8d + 6d6c904 commit 1b262b8
Show file tree
Hide file tree
Showing 113 changed files with 593 additions and 806 deletions.
4 changes: 2 additions & 2 deletions compiler/rustc_ast/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1430,8 +1430,8 @@ pub enum ExprKind {
/// The async block used to have a `NodeId`, which was removed in favor of
/// using the parent `NodeId` of the parent `Expr`.
Async(CaptureBy, P<Block>),
/// An await expression (`my_future.await`).
Await(P<Expr>),
/// An await expression (`my_future.await`). Span is of await keyword.
Await(P<Expr>, Span),

/// A try block (`try { ... }`).
TryBlock(P<Block>),
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_ast/src/mut_visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1415,7 +1415,10 @@ pub fn noop_visit_expr<T: MutVisitor>(
ExprKind::Async(_capture_by, body) => {
vis.visit_block(body);
}
ExprKind::Await(expr) => vis.visit_expr(expr),
ExprKind::Await(expr, await_kw_span) => {
vis.visit_expr(expr);
vis.visit_span(await_kw_span);
}
ExprKind::Assign(el, er, _) => {
vis.visit_expr(el);
vis.visit_expr(er);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ast/src/util/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ pub fn contains_exterior_struct_lit(value: &ast::Expr) -> bool {
// X { y: 1 } + X { y: 2 }
contains_exterior_struct_lit(lhs) || contains_exterior_struct_lit(rhs)
}
ast::ExprKind::Await(x)
ast::ExprKind::Await(x, _)
| ast::ExprKind::Unary(_, x)
| ast::ExprKind::Cast(x, _)
| ast::ExprKind::Type(x, _)
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ast/src/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -864,7 +864,7 @@ pub fn walk_expr<'a, V: Visitor<'a>>(visitor: &mut V, expression: &'a Expr) {
ExprKind::Async(_, body) => {
visitor.visit_block(body);
}
ExprKind::Await(expr) => visitor.visit_expr(expr),
ExprKind::Await(expr, _) => visitor.visit_expr(expr),
ExprKind::Assign(lhs, rhs, _) => {
visitor.visit_expr(lhs);
visitor.visit_expr(rhs);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ast_lowering/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ pub struct BaseExpressionDoubleDot {
pub struct AwaitOnlyInAsyncFnAndBlocks {
#[primary_span]
#[label]
pub dot_await_span: Span,
pub await_kw_span: Span,
#[label(ast_lowering_this_not_async)]
pub item_span: Option<Span>,
}
Expand Down
24 changes: 5 additions & 19 deletions compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,21 +185,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
hir::AsyncGeneratorKind::Block,
|this| this.with_new_scopes(|this| this.lower_block_expr(block)),
),
ExprKind::Await(expr) => {
let dot_await_span = if expr.span.hi() < e.span.hi() {
let span_with_whitespace = self
.tcx
.sess
.source_map()
.span_extend_while(expr.span, char::is_whitespace)
.unwrap_or(expr.span);
span_with_whitespace.shrink_to_hi().with_hi(e.span.hi())
} else {
// this is a recovered `await expr`
e.span
};
self.lower_expr_await(dot_await_span, expr)
}
ExprKind::Await(expr, await_kw_span) => self.lower_expr_await(*await_kw_span, expr),
ExprKind::Closure(box Closure {
binder,
capture_clause,
Expand Down Expand Up @@ -710,18 +696,18 @@ impl<'hir> LoweringContext<'_, 'hir> {
/// }
/// }
/// ```
fn lower_expr_await(&mut self, dot_await_span: Span, expr: &Expr) -> hir::ExprKind<'hir> {
let full_span = expr.span.to(dot_await_span);
fn lower_expr_await(&mut self, await_kw_span: Span, expr: &Expr) -> hir::ExprKind<'hir> {
let full_span = expr.span.to(await_kw_span);
match self.generator_kind {
Some(hir::GeneratorKind::Async(_)) => {}
Some(hir::GeneratorKind::Gen) | None => {
self.tcx.sess.emit_err(AwaitOnlyInAsyncFnAndBlocks {
dot_await_span,
await_kw_span,
item_span: self.current_item,
});
}
}
let span = self.mark_span_with_reason(DesugaringKind::Await, dot_await_span, None);
let span = self.mark_span_with_reason(DesugaringKind::Await, await_kw_span, None);
let gen_future_span = self.mark_span_with_reason(
DesugaringKind::Await,
full_span,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ast_lowering/src/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,7 @@ fn may_contain_yield_point(e: &ast::Expr) -> bool {

impl Visitor<'_> for MayContainYieldPoint {
fn visit_expr(&mut self, e: &ast::Expr) {
if let ast::ExprKind::Await(_) | ast::ExprKind::Yield(_) = e.kind {
if let ast::ExprKind::Await(_, _) | ast::ExprKind::Yield(_) = e.kind {
self.0 = true;
} else {
visit::walk_expr(self, e);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ast_pretty/src/pprust/state/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ impl<'a> State<'a> {
self.ibox(0);
self.print_block_with_attrs(blk, attrs);
}
ast::ExprKind::Await(expr) => {
ast::ExprKind::Await(expr, _) => {
self.print_expr_maybe_paren(expr, parser::PREC_POSTFIX);
self.word(".await");
}
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_borrowck/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,15 @@ borrowck_moved_due_to_method_call =
*[false] call
}
borrowck_moved_due_to_await =
{$place_name} {$is_partial ->
[true] partially moved
*[false] moved
} due to this {$is_loop_message ->
[true] await, in previous iteration of loop
*[false] await
}
borrowck_value_moved_here =
value {$is_partial ->
[true] partially moved
Expand Down
21 changes: 15 additions & 6 deletions compiler/rustc_borrowck/src/diagnostics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1085,12 +1085,21 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}
}
} else {
err.subdiagnostic(CaptureReasonLabel::MethodCall {
fn_call_span,
place_name: &place_name,
is_partial,
is_loop_message,
});
if let Some((CallDesugaringKind::Await, _)) = desugaring {
err.subdiagnostic(CaptureReasonLabel::Await {
fn_call_span,
place_name: &place_name,
is_partial,
is_loop_message,
});
} else {
err.subdiagnostic(CaptureReasonLabel::MethodCall {
fn_call_span,
place_name: &place_name,
is_partial,
is_loop_message,
});
}
// Erase and shadow everything that could be passed to the new infcx.
let ty = moved_place.ty(self.body, tcx).ty;

Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_borrowck/src/session_diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,14 @@ pub(crate) enum CaptureReasonLabel<'a> {
is_partial: bool,
is_loop_message: bool,
},
#[label(borrowck_moved_due_to_await)]
Await {
#[primary_span]
fn_call_span: Span,
place_name: &'a str,
is_partial: bool,
is_loop_message: bool,
},
#[label(borrowck_value_moved_here)]
MovedHere {
#[primary_span]
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_builtin_macros/src/assert/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ impl<'cx, 'a> Context<'cx, 'a> {
ExprKind::Assign(_, _, _)
| ExprKind::AssignOp(_, _, _)
| ExprKind::Async(_, _)
| ExprKind::Await(_)
| ExprKind::Await(_, _)
| ExprKind::Block(_, _)
| ExprKind::Break(_, _)
| ExprKind::Closure(_)
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_const_eval/src/transform/check_consts/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,9 @@ impl<'tcx> NonConstOp<'tcx> for FnCallNonConst<'tcx> {
CallDesugaringKind::TryBlockFromOutput => {
error!("`try` block cannot convert `{}` to the result in {}s")
}
CallDesugaringKind::Await => {
error!("cannot convert `{}` into a future in {}s")
}
};

diag_trait(&mut err, self_ty, kind.trait_def_id(tcx));
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_middle/src/util/call_kind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ pub enum CallDesugaringKind {
QuestionFromResidual,
/// try { ..; x } calls type_of(x)::from_output(x)
TryBlockFromOutput,
/// `.await` calls `IntoFuture::into_future`
Await,
}

impl CallDesugaringKind {
Expand All @@ -29,6 +31,7 @@ impl CallDesugaringKind {
tcx.require_lang_item(LangItem::Try, None)
}
Self::QuestionFromResidual => tcx.get_diagnostic_item(sym::FromResidual).unwrap(),
Self::Await => tcx.get_diagnostic_item(sym::IntoFuture).unwrap(),
}
}
}
Expand Down Expand Up @@ -129,6 +132,8 @@ pub fn call_kind<'tcx>(
&& fn_call_span.desugaring_kind() == Some(DesugaringKind::TryBlock)
{
Some((CallDesugaringKind::TryBlockFromOutput, method_substs.type_at(0)))
} else if fn_call_span.is_desugaring(DesugaringKind::Await) {
Some((CallDesugaringKind::Await, method_substs.type_at(0)))
} else {
None
};
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_parse/src/parser/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1646,7 +1646,7 @@ impl<'a> Parser<'a> {
// Avoid knock-down errors as we don't know whether to interpret this as `foo().await?`
// or `foo()?.await` (the very reason we went with postfix syntax 😅).
ExprKind::Try(_) => ExprKind::Err,
_ => ExprKind::Await(expr),
_ => ExprKind::Await(expr, await_sp),
};
let expr = self.mk_expr(lo.to(sp), kind);
self.maybe_recover_from_bad_qpath(expr)
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 @@ -859,7 +859,7 @@ impl<'a> Parser<'a> {
ExprKind::Field(_, _) => "a field access",
ExprKind::MethodCall(_) => "a method call",
ExprKind::Call(_, _) => "a function call",
ExprKind::Await(_) => "`.await`",
ExprKind::Await(_, _) => "`.await`",
ExprKind::Err => return Ok(with_postfix),
_ => unreachable!("parse_dot_or_call_expr_with_ shouldn't produce this"),
}
Expand Down Expand Up @@ -3252,7 +3252,7 @@ impl<'a> Parser<'a> {

fn mk_await_expr(&mut self, self_arg: P<Expr>, lo: Span) -> P<Expr> {
let span = lo.to(self.prev_token.span);
let await_expr = self.mk_expr(span, ExprKind::Await(self_arg));
let await_expr = self.mk_expr(span, ExprKind::Await(self_arg, self.prev_token.span));
self.recover_from_await_method_call();
await_expr
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ symbols! {
Input,
Into,
IntoDiagnostic,
IntoFuture,
IntoIterator,
IoRead,
IoWrite,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1583,55 +1583,68 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
}

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

if let ObligationCauseCode::AwaitableExpr(hir_id) = obligation.cause.code().peel_derives() {
let hir = self.tcx.hir();
if let Some(hir::Node::Expr(expr)) = hir_id.and_then(|hir_id| hir.find(hir_id)) {
// FIXME: use `obligation.predicate.kind()...trait_ref.self_ty()` to see if we have `()`
// and if not maybe suggest doing something else? If we kept the expression around we
// could also check if it is an fn call (very likely) and suggest changing *that*, if
// it is from the local crate.
let hir = self.tcx.hir();
if let ObligationCauseCode::AwaitableExpr(Some(hir_id)) = obligation.cause.code().peel_derives()
&& let hir::Node::Expr(expr) = hir.get(*hir_id)
{
// FIXME: use `obligation.predicate.kind()...trait_ref.self_ty()` to see if we have `()`
// and if not maybe suggest doing something else? If we kept the expression around we
// could also check if it is an fn call (very likely) and suggest changing *that*, if
// it is from the local crate.

// use nth(1) to skip one layer of desugaring from `IntoIter::into_iter`
if let Some((_, hir::Node::Expr(await_expr))) = hir.parent_iter(*hir_id).nth(1)
&& let Some(expr_span) = expr.span.find_ancestor_inside(await_expr.span)
{
let removal_span = self.tcx
.sess
.source_map()
.span_extend_while(expr_span, char::is_whitespace)
.unwrap_or(expr_span)
.shrink_to_hi()
.to(await_expr.span.shrink_to_hi());
err.span_suggestion(
span,
removal_span,
"remove the `.await`",
"",
Applicability::MachineApplicable,
);
// FIXME: account for associated `async fn`s.
if let hir::Expr { span, kind: hir::ExprKind::Call(base, _), .. } = expr {
if let ty::PredicateKind::Clause(ty::Clause::Trait(pred)) =
obligation.predicate.kind().skip_binder()
} else {
err.span_label(obligation.cause.span, "remove the `.await`");
}
// FIXME: account for associated `async fn`s.
if let hir::Expr { span, kind: hir::ExprKind::Call(base, _), .. } = expr {
if let ty::PredicateKind::Clause(ty::Clause::Trait(pred)) =
obligation.predicate.kind().skip_binder()
{
err.span_label(*span, &format!("this call returns `{}`", pred.self_ty()));
}
if let Some(typeck_results) = &self.typeck_results
&& let ty = typeck_results.expr_ty_adjusted(base)
&& let ty::FnDef(def_id, _substs) = ty.kind()
&& let Some(hir::Node::Item(hir::Item { ident, span, vis_span, .. })) =
hir.get_if_local(*def_id)
{
err.span_label(*span, &format!("this call returns `{}`", pred.self_ty()));
}
if let Some(typeck_results) = &self.typeck_results
&& let ty = typeck_results.expr_ty_adjusted(base)
&& let ty::FnDef(def_id, _substs) = ty.kind()
&& let Some(hir::Node::Item(hir::Item { ident, span, vis_span, .. })) =
hir.get_if_local(*def_id)
{
let msg = format!(
"alternatively, consider making `fn {}` asynchronous",
ident
let msg = format!(
"alternatively, consider making `fn {}` asynchronous",
ident
);
if vis_span.is_empty() {
err.span_suggestion_verbose(
span.shrink_to_lo(),
&msg,
"async ",
Applicability::MaybeIncorrect,
);
} else {
err.span_suggestion_verbose(
vis_span.shrink_to_hi(),
&msg,
" async",
Applicability::MaybeIncorrect,
);
if vis_span.is_empty() {
err.span_suggestion_verbose(
span.shrink_to_lo(),
&msg,
"async ",
Applicability::MaybeIncorrect,
);
} else {
err.span_suggestion_verbose(
vis_span.shrink_to_hi(),
&msg,
" async",
Applicability::MaybeIncorrect,
);
}
}
}
}
}
}
}
Expand Down
1 change: 1 addition & 0 deletions library/core/src/future/into_future.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ use crate::future::Future;
/// }
/// ```
#[stable(feature = "into_future", since = "1.64.0")]
#[rustc_diagnostic_item = "IntoFuture"]
pub trait IntoFuture {
/// The output that the future will produce on completion.
#[stable(feature = "into_future", since = "1.64.0")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,7 @@ fn ident_difference_expr_with_base_location(
| (AssignOp(_, _, _), AssignOp(_, _, _))
| (Assign(_, _, _), Assign(_, _, _))
| (TryBlock(_), TryBlock(_))
| (Await(_), Await(_))
| (Await(_, _), Await(_, _))
| (Async(_, _), Async(_, _))
| (Block(_, _), Block(_, _))
| (Closure(_), Closure(_))
Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy/clippy_utils/src/ast_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ pub fn eq_expr(l: &Expr, r: &Expr) -> bool {
(Paren(l), _) => eq_expr(l, r),
(_, Paren(r)) => eq_expr(l, r),
(Err, Err) => true,
(Try(l), Try(r)) | (Await(l), Await(r)) => eq_expr(l, r),
(Try(l), Try(r)) | (Await(l, _), Await(r, _)) => eq_expr(l, r),
(Array(l), Array(r)) => over(l, r, |l, r| eq_expr(l, r)),
(Tup(l), Tup(r)) => over(l, r, |l, r| eq_expr(l, r)),
(Repeat(le, ls), Repeat(re, rs)) => eq_expr(le, re) && eq_expr(&ls.value, &rs.value),
Expand Down
Loading

0 comments on commit 1b262b8

Please sign in to comment.