Skip to content

Commit

Permalink
Unrolled build for rust-lang#127058
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#127058 - compiler-errors:tighten-async-spans, r=oli-obk

Tighten `fn_decl_span` for async blocks

Tightens the span of `async {}` blocks in diagnostics, and subsequently async closures and async fns, by actually setting the `fn_decl_span` correctly. This is kinda a follow-up on rust-lang#125078, but it fixes the problem in a more general way.

I think the diagnostics are significantly improved, since we no longer have a bunch of overlapping spans. I'll point out one caveat where I think the diagnostic may get a bit more confusing, but where I don't think it matters.

r? ````@estebank```` or ````@oli-obk```` or someone else on wg-diag or compiler i dont really care lol
  • Loading branch information
rust-timer authored Jun 28, 2024
2 parents 42add88 + 789ee88 commit ee34dfc
Show file tree
Hide file tree
Showing 43 changed files with 173 additions and 204 deletions.
5 changes: 4 additions & 1 deletion compiler/rustc_ast/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1454,7 +1454,10 @@ pub enum ExprKind {
Block(P<Block>, Option<Label>),
/// An `async` block (`async move { ... }`),
/// or a `gen` block (`gen move { ... }`)
Gen(CaptureBy, P<Block>, GenBlockKind),
///
/// The span is the "decl", which is the header before the body `{ }`
/// including the `asyng`/`gen` keywords and possibly `move`.
Gen(CaptureBy, P<Block>, GenBlockKind, Span),
/// An await expression (`my_future.await`). Span is of await keyword.
Await(P<Expr>, Span),

Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_ast/src/mut_visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1528,8 +1528,9 @@ pub fn noop_visit_expr<T: MutVisitor>(
visit_opt(label, |label| vis.visit_label(label));
vis.visit_block(blk);
}
ExprKind::Gen(_capture_by, body, _kind) => {
ExprKind::Gen(_capture_by, body, _kind, decl_span) => {
vis.visit_block(body);
vis.visit_span(decl_span);
}
ExprKind::Await(expr, await_kw_span) => {
vis.visit_expr(expr);
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 @@ -1122,7 +1122,7 @@ pub fn walk_expr<'a, V: Visitor<'a>>(visitor: &mut V, expression: &'a Expr) -> V
visit_opt!(visitor, visit_label, opt_label);
try_visit!(visitor.visit_block(block));
}
ExprKind::Gen(_capt, body, _kind) => try_visit!(visitor.visit_block(body)),
ExprKind::Gen(_capt, body, _kind, _decl_span) => try_visit!(visitor.visit_block(body)),
ExprKind::Await(expr, _span) => try_visit!(visitor.visit_expr(expr)),
ExprKind::Assign(lhs, rhs, _span) => {
try_visit!(visitor.visit_expr(lhs));
Expand Down
7 changes: 5 additions & 2 deletions compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
*fn_arg_span,
),
},
ExprKind::Gen(capture_clause, block, genblock_kind) => {
ExprKind::Gen(capture_clause, block, genblock_kind, decl_span) => {
let desugaring_kind = match genblock_kind {
GenBlockKind::Async => hir::CoroutineDesugaring::Async,
GenBlockKind::Gen => hir::CoroutineDesugaring::Gen,
Expand All @@ -237,6 +237,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
*capture_clause,
e.id,
None,
*decl_span,
e.span,
desugaring_kind,
hir::CoroutineSource::Block,
Expand Down Expand Up @@ -616,6 +617,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
capture_clause: CaptureBy,
closure_node_id: NodeId,
return_ty: Option<hir::FnRetTy<'hir>>,
fn_decl_span: Span,
span: Span,
desugaring_kind: hir::CoroutineDesugaring,
coroutine_source: hir::CoroutineSource,
Expand Down Expand Up @@ -692,7 +694,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
bound_generic_params: &[],
fn_decl,
body,
fn_decl_span: self.lower_span(span),
fn_decl_span: self.lower_span(fn_decl_span),
fn_arg_span: None,
kind: hir::ClosureKind::Coroutine(coroutine_kind),
constness: hir::Constness::NotConst,
Expand Down Expand Up @@ -1083,6 +1085,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
let (parameters, expr) = this.lower_coroutine_body_with_moved_arguments(
&inner_decl,
|this| this.with_new_scopes(fn_decl_span, |this| this.lower_expr_mut(body)),
fn_decl_span,
body.span,
coroutine_kind,
hir::CoroutineSource::Closure,
Expand Down
16 changes: 8 additions & 8 deletions compiler/rustc_ast_lowering/src/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
// declaration (decl), not the return types.
let coroutine_kind = header.coroutine_kind;
let body_id = this.lower_maybe_coroutine_body(
*fn_sig_span,
span,
hir_id,
decl,
Expand Down Expand Up @@ -799,6 +800,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
}
AssocItemKind::Fn(box Fn { sig, generics, body: Some(body), .. }) => {
let body_id = self.lower_maybe_coroutine_body(
sig.span,
i.span,
hir_id,
&sig.decl,
Expand Down Expand Up @@ -915,6 +917,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
),
AssocItemKind::Fn(box Fn { sig, generics, body, .. }) => {
let body_id = self.lower_maybe_coroutine_body(
sig.span,
i.span,
hir_id,
&sig.decl,
Expand Down Expand Up @@ -1111,6 +1114,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
/// `gen {}` block as appropriate.
fn lower_maybe_coroutine_body(
&mut self,
fn_decl_span: Span,
span: Span,
fn_id: hir::HirId,
decl: &FnDecl,
Expand All @@ -1124,6 +1128,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
let (parameters, expr) = this.lower_coroutine_body_with_moved_arguments(
decl,
|this| this.lower_block_expr(body),
fn_decl_span,
body.span,
coroutine_kind,
hir::CoroutineSource::Fn,
Expand All @@ -1145,6 +1150,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
&mut self,
decl: &FnDecl,
lower_body: impl FnOnce(&mut LoweringContext<'_, 'hir>) -> hir::Expr<'hir>,
fn_decl_span: Span,
body_span: Span,
coroutine_kind: CoroutineKind,
coroutine_source: hir::CoroutineSource,
Expand Down Expand Up @@ -1315,13 +1321,6 @@ impl<'hir> LoweringContext<'_, 'hir> {
};
let closure_id = coroutine_kind.closure_id();

let span = if let FnRetTy::Default(span) = decl.output
&& matches!(coroutine_source, rustc_hir::CoroutineSource::Closure)
{
body_span.with_lo(span.lo())
} else {
body_span
};
let coroutine_expr = self.make_desugared_coroutine_expr(
// The default capture mode here is by-ref. Later on during upvar analysis,
// we will force the captured arguments to by-move, but for async closures,
Expand All @@ -1330,7 +1329,8 @@ impl<'hir> LoweringContext<'_, 'hir> {
CaptureBy::Ref,
closure_id,
None,
span,
fn_decl_span,
body_span,
desugaring_kind,
coroutine_source,
mkbody,
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 @@ -540,7 +540,7 @@ impl<'a> State<'a> {
self.ibox(0);
self.print_block_with_attrs(blk, attrs);
}
ast::ExprKind::Gen(capture_clause, blk, kind) => {
ast::ExprKind::Gen(capture_clause, blk, kind, _decl_span) => {
self.word_nbsp(kind.modifier());
self.print_capture_clause(*capture_clause);
// cbox/ibox in analogy to the `ExprKind::Block` arm above
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 @@ -298,7 +298,7 @@ impl<'cx, 'a> Context<'cx, 'a> {
// sync with the `rfc-2011-nicer-assert-messages/all-expr-kinds.rs` test.
ExprKind::Assign(_, _, _)
| ExprKind::AssignOp(_, _, _)
| ExprKind::Gen(_, _, _)
| ExprKind::Gen(_, _, _, _)
| ExprKind::Await(_, _)
| ExprKind::Block(_, _)
| ExprKind::Break(_, _)
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3432,8 +3432,9 @@ impl<'a> Parser<'a> {
}
}
let capture_clause = self.parse_capture_clause()?;
let decl_span = lo.to(self.prev_token.span);
let (attrs, body) = self.parse_inner_attrs_and_block()?;
let kind = ExprKind::Gen(capture_clause, body, kind);
let kind = ExprKind::Gen(capture_clause, body, kind, decl_span);
Ok(self.mk_expr_with_attrs(lo.to(self.prev_token.span), kind, attrs))
}

Expand Down Expand Up @@ -4022,7 +4023,7 @@ impl MutVisitor for CondChecker<'_> {
| ExprKind::Match(_, _, _)
| ExprKind::Closure(_)
| ExprKind::Block(_, _)
| ExprKind::Gen(_, _, _)
| ExprKind::Gen(_, _, _, _)
| ExprKind::TryBlock(_)
| ExprKind::Underscore
| ExprKind::Path(_, _)
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_resolve/src/def_collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ impl<'a, 'b, 'tcx> visit::Visitor<'a> for DefCollector<'a, 'b, 'tcx> {
None => closure_def,
}
}
ExprKind::Gen(_, _, _) => {
ExprKind::Gen(_, _, _, _) => {
self.create_def(expr.id, kw::Empty, DefKind::Closure, expr.span)
}
ExprKind::ConstBlock(ref constant) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,7 @@ fn ident_difference_expr_with_base_location(
| (Assign(_, _, _), Assign(_, _, _))
| (TryBlock(_), TryBlock(_))
| (Await(_, _), Await(_, _))
| (Gen(_, _, _), Gen(_, _, _))
| (Gen(_, _, _, _), Gen(_, _, _, _))
| (Block(_, _), Block(_, _))
| (Closure(_), Closure(_))
| (Match(_, _, _), Match(_, _, _))
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 @@ -226,7 +226,7 @@ pub fn eq_expr(l: &Expr, r: &Expr) -> bool {
&& eq_fn_decl(lf, rf)
&& eq_expr(le, re)
},
(Gen(lc, lb, lk), Gen(rc, rb, rk)) => lc == rc && eq_block(lb, rb) && lk == rk,
(Gen(lc, lb, lk, _), Gen(rc, rb, rk, _)) => lc == rc && eq_block(lb, rb) && lk == rk,
(Range(lf, lt, ll), Range(rf, rt, rl)) => ll == rl && eq_expr_opt(lf, rf) && eq_expr_opt(lt, rt),
(AddrOf(lbk, lm, le), AddrOf(rbk, rm, re)) => lbk == rbk && lm == rm && eq_expr(le, re),
(Path(lq, lp), Path(rq, rp)) => both(lq, rq, eq_qself) && eq_path(lp, rp),
Expand Down
2 changes: 1 addition & 1 deletion src/tools/rustfmt/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ pub(crate) fn format_expr(
))
}
}
ast::ExprKind::Gen(capture_by, ref block, ref kind) => {
ast::ExprKind::Gen(capture_by, ref block, ref kind, _) => {
let mover = if matches!(capture_by, ast::CaptureBy::Value { .. }) {
"move "
} else {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,20 +1,18 @@
error[E0267]: `break` inside `async` block
--> $DIR/async-block-control-flow-static-semantics.rs:32:9
|
LL | / async {
LL | | break 0u8;
| | ^^^^^^^^^ cannot `break` inside `async` block
LL | | };
| |_____- enclosing `async` block
LL | async {
| ----- enclosing `async` block
LL | break 0u8;
| ^^^^^^^^^ cannot `break` inside `async` block

error[E0267]: `break` inside `async` block
--> $DIR/async-block-control-flow-static-semantics.rs:39:13
|
LL | / async {
LL | | break 0u8;
| | ^^^^^^^^^ cannot `break` inside `async` block
LL | | };
| |_________- enclosing `async` block
LL | async {
| ----- enclosing `async` block
LL | break 0u8;
| ^^^^^^^^^ cannot `break` inside `async` block

error[E0308]: mismatched types
--> $DIR/async-block-control-flow-static-semantics.rs:21:58
Expand All @@ -29,13 +27,13 @@ LL | |
LL | | }
| |_^ expected `u8`, found `()`

error[E0271]: expected `{async block@$DIR/async-block-control-flow-static-semantics.rs:23:17: 25:6}` to be a future that resolves to `()`, but it resolves to `u8`
error[E0271]: expected `{async block@$DIR/async-block-control-flow-static-semantics.rs:23:17: 23:22}` to be a future that resolves to `()`, but it resolves to `u8`
--> $DIR/async-block-control-flow-static-semantics.rs:26:39
|
LL | let _: &dyn Future<Output = ()> = &block;
| ^^^^^^ expected `()`, found `u8`
|
= note: required for the cast from `&{async block@$DIR/async-block-control-flow-static-semantics.rs:23:17: 25:6}` to `&dyn Future<Output = ()>`
= note: required for the cast from `&{async block@$DIR/async-block-control-flow-static-semantics.rs:23:17: 23:22}` to `&dyn Future<Output = ()>`

error[E0308]: mismatched types
--> $DIR/async-block-control-flow-static-semantics.rs:12:43
Expand All @@ -45,13 +43,13 @@ LL | fn return_targets_async_block_not_fn() -> u8 {
| |
| implicitly returns `()` as its body has no tail or `return` expression

error[E0271]: expected `{async block@$DIR/async-block-control-flow-static-semantics.rs:14:17: 16:6}` to be a future that resolves to `()`, but it resolves to `u8`
error[E0271]: expected `{async block@$DIR/async-block-control-flow-static-semantics.rs:14:17: 14:22}` to be a future that resolves to `()`, but it resolves to `u8`
--> $DIR/async-block-control-flow-static-semantics.rs:17:39
|
LL | let _: &dyn Future<Output = ()> = &block;
| ^^^^^^ expected `()`, found `u8`
|
= note: required for the cast from `&{async block@$DIR/async-block-control-flow-static-semantics.rs:14:17: 16:6}` to `&dyn Future<Output = ()>`
= note: required for the cast from `&{async block@$DIR/async-block-control-flow-static-semantics.rs:14:17: 14:22}` to `&dyn Future<Output = ()>`

error[E0308]: mismatched types
--> $DIR/async-block-control-flow-static-semantics.rs:49:44
Expand Down
10 changes: 4 additions & 6 deletions tests/ui/async-await/async-borrowck-escaping-block-error.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@ error[E0373]: async block may outlive the current function, but it borrows `x`,
--> $DIR/async-borrowck-escaping-block-error.rs:6:14
|
LL | Box::new(async { x } )
| ^^^^^^^^-^^
| | |
| | `x` is borrowed here
| ^^^^^ - `x` is borrowed here
| |
| may outlive borrowed value `x`
|
note: async block is returned here
Expand All @@ -21,9 +20,8 @@ error[E0373]: async block may outlive the current function, but it borrows `x`,
--> $DIR/async-borrowck-escaping-block-error.rs:11:5
|
LL | async { *x }
| ^^^^^^^^--^^
| | |
| | `x` is borrowed here
| ^^^^^ -- `x` is borrowed here
| |
| may outlive borrowed value `x`
|
note: async block is returned here
Expand Down
24 changes: 11 additions & 13 deletions tests/ui/async-await/async-closures/wrong-fn-kind.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -22,20 +22,18 @@ LL | fn needs_async_fn(_: impl async Fn()) {}
error[E0596]: cannot borrow `x` as mutable, as it is a captured variable in a `Fn` closure
--> $DIR/wrong-fn-kind.rs:9:20
|
LL | fn needs_async_fn(_: impl async Fn()) {}
| --------------- change this to accept `FnMut` instead of `Fn`
LL | fn needs_async_fn(_: impl async Fn()) {}
| --------------- change this to accept `FnMut` instead of `Fn`
...
LL | needs_async_fn(async || {
| -------------- ^-------
| | |
| _____|______________in this closure
| | |
| | expects `Fn` instead of `FnMut`
LL | |
LL | | x += 1;
| | - mutable borrow occurs due to use of `x` in closure
LL | | });
| |_____^ cannot borrow as mutable
LL | needs_async_fn(async || {
| -------------- ^^^^^^^^
| | |
| | cannot borrow as mutable
| | in this closure
| expects `Fn` instead of `FnMut`
LL |
LL | x += 1;
| - mutable borrow occurs due to use of `x` in closure

error: aborting due to 2 previous errors

Expand Down
25 changes: 12 additions & 13 deletions tests/ui/async-await/async-is-unwindsafe.stderr
Original file line number Diff line number Diff line change
@@ -1,20 +1,19 @@
error[E0277]: the type `&mut Context<'_>` may not be safely transferred across an unwind boundary
--> $DIR/async-is-unwindsafe.rs:12:5
|
LL | is_unwindsafe(async {
| ______^ -
| | ___________________|
LL | ||
LL | || use std::ptr::null;
LL | || use std::task::{Context, RawWaker, RawWakerVTable, Waker};
... ||
LL | || drop(cx_ref);
LL | || });
| ||_____-^ `&mut Context<'_>` may not be safely transferred across an unwind boundary
| |_____|
| within this `{async block@$DIR/async-is-unwindsafe.rs:12:19: 29:6}`
LL | is_unwindsafe(async {
| ^ ----- within this `{async block@$DIR/async-is-unwindsafe.rs:12:19: 12:24}`
| _____|
| |
LL | |
LL | | use std::ptr::null;
LL | | use std::task::{Context, RawWaker, RawWakerVTable, Waker};
... |
LL | | drop(cx_ref);
LL | | });
| |______^ `&mut Context<'_>` may not be safely transferred across an unwind boundary
|
= help: within `{async block@$DIR/async-is-unwindsafe.rs:12:19: 29:6}`, the trait `UnwindSafe` is not implemented for `&mut Context<'_>`, which is required by `{async block@$DIR/async-is-unwindsafe.rs:12:19: 29:6}: UnwindSafe`
= help: within `{async block@$DIR/async-is-unwindsafe.rs:12:19: 12:24}`, the trait `UnwindSafe` is not implemented for `&mut Context<'_>`, which is required by `{async block@$DIR/async-is-unwindsafe.rs:12:19: 12:24}: UnwindSafe`
= note: `UnwindSafe` is implemented for `&Context<'_>`, but not for `&mut Context<'_>`
note: future does not implement `UnwindSafe` as this value is used across an await
--> $DIR/async-is-unwindsafe.rs:25:18
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/async-await/coroutine-desc.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ LL | fun(async {}, async {});
| | expected all arguments to be this `async` block type because they need to match the type of this parameter
| arguments to this function are incorrect
|
= note: expected `async` block `{async block@$DIR/coroutine-desc.rs:10:9: 10:17}`
found `async` block `{async block@$DIR/coroutine-desc.rs:10:19: 10:27}`
= note: expected `async` block `{async block@$DIR/coroutine-desc.rs:10:9: 10:14}`
found `async` block `{async block@$DIR/coroutine-desc.rs:10:19: 10:24}`
= note: no two async blocks, even if identical, have the same type
= help: consider pinning your async block and casting it to a trait object
note: function defined here
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/async-await/coroutine-not-future.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ note: required by a bound in `takes_coroutine`
LL | fn takes_coroutine<ResumeTy>(_g: impl Coroutine<ResumeTy, Yield = (), Return = ()>) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `takes_coroutine`

error[E0277]: the trait bound `{async block@$DIR/coroutine-not-future.rs:39:21: 39:29}: Coroutine<_>` is not satisfied
error[E0277]: the trait bound `{async block@$DIR/coroutine-not-future.rs:39:21: 39:26}: Coroutine<_>` is not satisfied
--> $DIR/coroutine-not-future.rs:39:21
|
LL | takes_coroutine(async {});
| --------------- ^^^^^^^^ the trait `Coroutine<_>` is not implemented for `{async block@$DIR/coroutine-not-future.rs:39:21: 39:29}`
| --------------- ^^^^^^^^ the trait `Coroutine<_>` is not implemented for `{async block@$DIR/coroutine-not-future.rs:39:21: 39:26}`
| |
| required by a bound introduced by this call
|
Expand Down
Loading

0 comments on commit ee34dfc

Please sign in to comment.