Skip to content

Tweak await span to not contain dot #110823

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

Merged
merged 6 commits into from
May 1, 2023
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
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)))
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems somewhat unexpected to have this under CallKind::Normal. The docs say CallKind::Normal corresponds to method calls. I mean this is consistent with previous usage, but maybe we should introduce something like CallKind::Other?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm... this just feels like uplifting the desugaring part of the Normal variant up to its own variant. I don't see how this makes things much cleaner 😅

Copy link
Contributor

Choose a reason for hiding this comment

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

I just meant that this would be more consistent with the docs of CallKind::Normal. Feel free to ignore since CallKind::Normal was also used for other CallDesugaringKinds before for which that variant doesn't really match the docs afaict. The rest of the PR looks good to me, so r=me.

} 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 @@ -3256,7 +3256,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 @@ -208,6 +208,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