Skip to content

Commit f305466

Browse files
matthiaskrgrmatthiaskrgr
authored
Rollup merge of #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 #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
2 parents a66f2c2 + 789ee88 commit f305466

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

43 files changed

+173
-204
lines changed

‎compiler/rustc_ast/src/ast.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -1454,7 +1454,10 @@ pub enum ExprKind {
14541454
Block(P<Block>, Option<Label>),
14551455
/// An `async` block (`async move { ... }`),
14561456
/// or a `gen` block (`gen move { ... }`)
1457-
Gen(CaptureBy, P<Block>, GenBlockKind),
1457+
///
1458+
/// The span is the "decl", which is the header before the body `{ }`
1459+
/// including the `asyng`/`gen` keywords and possibly `move`.
1460+
Gen(CaptureBy, P<Block>, GenBlockKind, Span),
14581461
/// An await expression (`my_future.await`). Span is of await keyword.
14591462
Await(P<Expr>, Span),
14601463

‎compiler/rustc_ast/src/mut_visit.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -1528,8 +1528,9 @@ pub fn noop_visit_expr<T: MutVisitor>(
15281528
visit_opt(label, |label| vis.visit_label(label));
15291529
vis.visit_block(blk);
15301530
}
1531-
ExprKind::Gen(_capture_by, body, _kind) => {
1531+
ExprKind::Gen(_capture_by, body, _kind, decl_span) => {
15321532
vis.visit_block(body);
1533+
vis.visit_span(decl_span);
15331534
}
15341535
ExprKind::Await(expr, await_kw_span) => {
15351536
vis.visit_expr(expr);

‎compiler/rustc_ast/src/visit.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1122,7 +1122,7 @@ pub fn walk_expr<'a, V: Visitor<'a>>(visitor: &mut V, expression: &'a Expr) -> V
11221122
visit_opt!(visitor, visit_label, opt_label);
11231123
try_visit!(visitor.visit_block(block));
11241124
}
1125-
ExprKind::Gen(_capt, body, _kind) => try_visit!(visitor.visit_block(body)),
1125+
ExprKind::Gen(_capt, body, _kind, _decl_span) => try_visit!(visitor.visit_block(body)),
11261126
ExprKind::Await(expr, _span) => try_visit!(visitor.visit_expr(expr)),
11271127
ExprKind::Assign(lhs, rhs, _span) => {
11281128
try_visit!(visitor.visit_expr(lhs));

‎compiler/rustc_ast_lowering/src/expr.rs

+5-2
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
227227
*fn_arg_span,
228228
),
229229
},
230-
ExprKind::Gen(capture_clause, block, genblock_kind) => {
230+
ExprKind::Gen(capture_clause, block, genblock_kind, decl_span) => {
231231
let desugaring_kind = match genblock_kind {
232232
GenBlockKind::Async => hir::CoroutineDesugaring::Async,
233233
GenBlockKind::Gen => hir::CoroutineDesugaring::Gen,
@@ -237,6 +237,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
237237
*capture_clause,
238238
e.id,
239239
None,
240+
*decl_span,
240241
e.span,
241242
desugaring_kind,
242243
hir::CoroutineSource::Block,
@@ -616,6 +617,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
616617
capture_clause: CaptureBy,
617618
closure_node_id: NodeId,
618619
return_ty: Option<hir::FnRetTy<'hir>>,
620+
fn_decl_span: Span,
619621
span: Span,
620622
desugaring_kind: hir::CoroutineDesugaring,
621623
coroutine_source: hir::CoroutineSource,
@@ -692,7 +694,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
692694
bound_generic_params: &[],
693695
fn_decl,
694696
body,
695-
fn_decl_span: self.lower_span(span),
697+
fn_decl_span: self.lower_span(fn_decl_span),
696698
fn_arg_span: None,
697699
kind: hir::ClosureKind::Coroutine(coroutine_kind),
698700
constness: hir::Constness::NotConst,
@@ -1083,6 +1085,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
10831085
let (parameters, expr) = this.lower_coroutine_body_with_moved_arguments(
10841086
&inner_decl,
10851087
|this| this.with_new_scopes(fn_decl_span, |this| this.lower_expr_mut(body)),
1088+
fn_decl_span,
10861089
body.span,
10871090
coroutine_kind,
10881091
hir::CoroutineSource::Closure,

‎compiler/rustc_ast_lowering/src/item.rs

+8-8
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
211211
// declaration (decl), not the return types.
212212
let coroutine_kind = header.coroutine_kind;
213213
let body_id = this.lower_maybe_coroutine_body(
214+
*fn_sig_span,
214215
span,
215216
hir_id,
216217
decl,
@@ -799,6 +800,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
799800
}
800801
AssocItemKind::Fn(box Fn { sig, generics, body: Some(body), .. }) => {
801802
let body_id = self.lower_maybe_coroutine_body(
803+
sig.span,
802804
i.span,
803805
hir_id,
804806
&sig.decl,
@@ -915,6 +917,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
915917
),
916918
AssocItemKind::Fn(box Fn { sig, generics, body, .. }) => {
917919
let body_id = self.lower_maybe_coroutine_body(
920+
sig.span,
918921
i.span,
919922
hir_id,
920923
&sig.decl,
@@ -1111,6 +1114,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
11111114
/// `gen {}` block as appropriate.
11121115
fn lower_maybe_coroutine_body(
11131116
&mut self,
1117+
fn_decl_span: Span,
11141118
span: Span,
11151119
fn_id: hir::HirId,
11161120
decl: &FnDecl,
@@ -1124,6 +1128,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
11241128
let (parameters, expr) = this.lower_coroutine_body_with_moved_arguments(
11251129
decl,
11261130
|this| this.lower_block_expr(body),
1131+
fn_decl_span,
11271132
body.span,
11281133
coroutine_kind,
11291134
hir::CoroutineSource::Fn,
@@ -1145,6 +1150,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
11451150
&mut self,
11461151
decl: &FnDecl,
11471152
lower_body: impl FnOnce(&mut LoweringContext<'_, 'hir>) -> hir::Expr<'hir>,
1153+
fn_decl_span: Span,
11481154
body_span: Span,
11491155
coroutine_kind: CoroutineKind,
11501156
coroutine_source: hir::CoroutineSource,
@@ -1315,13 +1321,6 @@ impl<'hir> LoweringContext<'_, 'hir> {
13151321
};
13161322
let closure_id = coroutine_kind.closure_id();
13171323

1318-
let span = if let FnRetTy::Default(span) = decl.output
1319-
&& matches!(coroutine_source, rustc_hir::CoroutineSource::Closure)
1320-
{
1321-
body_span.with_lo(span.lo())
1322-
} else {
1323-
body_span
1324-
};
13251324
let coroutine_expr = self.make_desugared_coroutine_expr(
13261325
// The default capture mode here is by-ref. Later on during upvar analysis,
13271326
// we will force the captured arguments to by-move, but for async closures,
@@ -1330,7 +1329,8 @@ impl<'hir> LoweringContext<'_, 'hir> {
13301329
CaptureBy::Ref,
13311330
closure_id,
13321331
None,
1333-
span,
1332+
fn_decl_span,
1333+
body_span,
13341334
desugaring_kind,
13351335
coroutine_source,
13361336
mkbody,

‎compiler/rustc_ast_pretty/src/pprust/state/expr.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -540,7 +540,7 @@ impl<'a> State<'a> {
540540
self.ibox(0);
541541
self.print_block_with_attrs(blk, attrs);
542542
}
543-
ast::ExprKind::Gen(capture_clause, blk, kind) => {
543+
ast::ExprKind::Gen(capture_clause, blk, kind, _decl_span) => {
544544
self.word_nbsp(kind.modifier());
545545
self.print_capture_clause(*capture_clause);
546546
// cbox/ibox in analogy to the `ExprKind::Block` arm above

‎compiler/rustc_builtin_macros/src/assert/context.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,7 @@ impl<'cx, 'a> Context<'cx, 'a> {
298298
// sync with the `rfc-2011-nicer-assert-messages/all-expr-kinds.rs` test.
299299
ExprKind::Assign(_, _, _)
300300
| ExprKind::AssignOp(_, _, _)
301-
| ExprKind::Gen(_, _, _)
301+
| ExprKind::Gen(_, _, _, _)
302302
| ExprKind::Await(_, _)
303303
| ExprKind::Block(_, _)
304304
| ExprKind::Break(_, _)

‎compiler/rustc_parse/src/parser/expr.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -3432,8 +3432,9 @@ impl<'a> Parser<'a> {
34323432
}
34333433
}
34343434
let capture_clause = self.parse_capture_clause()?;
3435+
let decl_span = lo.to(self.prev_token.span);
34353436
let (attrs, body) = self.parse_inner_attrs_and_block()?;
3436-
let kind = ExprKind::Gen(capture_clause, body, kind);
3437+
let kind = ExprKind::Gen(capture_clause, body, kind, decl_span);
34373438
Ok(self.mk_expr_with_attrs(lo.to(self.prev_token.span), kind, attrs))
34383439
}
34393440

@@ -4022,7 +4023,7 @@ impl MutVisitor for CondChecker<'_> {
40224023
| ExprKind::Match(_, _, _)
40234024
| ExprKind::Closure(_)
40244025
| ExprKind::Block(_, _)
4025-
| ExprKind::Gen(_, _, _)
4026+
| ExprKind::Gen(_, _, _, _)
40264027
| ExprKind::TryBlock(_)
40274028
| ExprKind::Underscore
40284029
| ExprKind::Path(_, _)

‎compiler/rustc_resolve/src/def_collector.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,7 @@ impl<'a, 'b, 'tcx> visit::Visitor<'a> for DefCollector<'a, 'b, 'tcx> {
334334
None => closure_def,
335335
}
336336
}
337-
ExprKind::Gen(_, _, _) => {
337+
ExprKind::Gen(_, _, _, _) => {
338338
self.create_def(expr.id, kw::Empty, DefKind::Closure, expr.span)
339339
}
340340
ExprKind::ConstBlock(ref constant) => {

‎src/tools/clippy/clippy_lints/src/suspicious_operation_groupings.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -549,7 +549,7 @@ fn ident_difference_expr_with_base_location(
549549
| (Assign(_, _, _), Assign(_, _, _))
550550
| (TryBlock(_), TryBlock(_))
551551
| (Await(_, _), Await(_, _))
552-
| (Gen(_, _, _), Gen(_, _, _))
552+
| (Gen(_, _, _, _), Gen(_, _, _, _))
553553
| (Block(_, _), Block(_, _))
554554
| (Closure(_), Closure(_))
555555
| (Match(_, _, _), Match(_, _, _))

‎src/tools/clippy/clippy_utils/src/ast_utils.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ pub fn eq_expr(l: &Expr, r: &Expr) -> bool {
226226
&& eq_fn_decl(lf, rf)
227227
&& eq_expr(le, re)
228228
},
229-
(Gen(lc, lb, lk), Gen(rc, rb, rk)) => lc == rc && eq_block(lb, rb) && lk == rk,
229+
(Gen(lc, lb, lk, _), Gen(rc, rb, rk, _)) => lc == rc && eq_block(lb, rb) && lk == rk,
230230
(Range(lf, lt, ll), Range(rf, rt, rl)) => ll == rl && eq_expr_opt(lf, rf) && eq_expr_opt(lt, rt),
231231
(AddrOf(lbk, lm, le), AddrOf(rbk, rm, re)) => lbk == rbk && lm == rm && eq_expr(le, re),
232232
(Path(lq, lp), Path(rq, rp)) => both(lq, rq, eq_qself) && eq_path(lp, rp),

‎src/tools/rustfmt/src/expr.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -372,7 +372,7 @@ pub(crate) fn format_expr(
372372
))
373373
}
374374
}
375-
ast::ExprKind::Gen(capture_by, ref block, ref kind) => {
375+
ast::ExprKind::Gen(capture_by, ref block, ref kind, _) => {
376376
let mover = if matches!(capture_by, ast::CaptureBy::Value { .. }) {
377377
"move "
378378
} else {

‎tests/ui/async-await/async-block-control-flow-static-semantics.stderr

+12-14
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,18 @@
11
error[E0267]: `break` inside `async` block
22
--> $DIR/async-block-control-flow-static-semantics.rs:32:9
33
|
4-
LL | / async {
5-
LL | | break 0u8;
6-
| | ^^^^^^^^^ cannot `break` inside `async` block
7-
LL | | };
8-
| |_____- enclosing `async` block
4+
LL | async {
5+
| ----- enclosing `async` block
6+
LL | break 0u8;
7+
| ^^^^^^^^^ cannot `break` inside `async` block
98

109
error[E0267]: `break` inside `async` block
1110
--> $DIR/async-block-control-flow-static-semantics.rs:39:13
1211
|
13-
LL | / async {
14-
LL | | break 0u8;
15-
| | ^^^^^^^^^ cannot `break` inside `async` block
16-
LL | | };
17-
| |_________- enclosing `async` block
12+
LL | async {
13+
| ----- enclosing `async` block
14+
LL | break 0u8;
15+
| ^^^^^^^^^ cannot `break` inside `async` block
1816

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

32-
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`
30+
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`
3331
--> $DIR/async-block-control-flow-static-semantics.rs:26:39
3432
|
3533
LL | let _: &dyn Future<Output = ()> = &block;
3634
| ^^^^^^ expected `()`, found `u8`
3735
|
38-
= note: required for the cast from `&{async block@$DIR/async-block-control-flow-static-semantics.rs:23:17: 25:6}` to `&dyn Future<Output = ()>`
36+
= note: required for the cast from `&{async block@$DIR/async-block-control-flow-static-semantics.rs:23:17: 23:22}` to `&dyn Future<Output = ()>`
3937

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

48-
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`
46+
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`
4947
--> $DIR/async-block-control-flow-static-semantics.rs:17:39
5048
|
5149
LL | let _: &dyn Future<Output = ()> = &block;
5250
| ^^^^^^ expected `()`, found `u8`
5351
|
54-
= note: required for the cast from `&{async block@$DIR/async-block-control-flow-static-semantics.rs:14:17: 16:6}` to `&dyn Future<Output = ()>`
52+
= note: required for the cast from `&{async block@$DIR/async-block-control-flow-static-semantics.rs:14:17: 14:22}` to `&dyn Future<Output = ()>`
5553

5654
error[E0308]: mismatched types
5755
--> $DIR/async-block-control-flow-static-semantics.rs:49:44

‎tests/ui/async-await/async-borrowck-escaping-block-error.stderr

+4-6
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,8 @@ error[E0373]: async block may outlive the current function, but it borrows `x`,
22
--> $DIR/async-borrowck-escaping-block-error.rs:6:14
33
|
44
LL | Box::new(async { x } )
5-
| ^^^^^^^^-^^
6-
| | |
7-
| | `x` is borrowed here
5+
| ^^^^^ - `x` is borrowed here
6+
| |
87
| may outlive borrowed value `x`
98
|
109
note: async block is returned here
@@ -21,9 +20,8 @@ error[E0373]: async block may outlive the current function, but it borrows `x`,
2120
--> $DIR/async-borrowck-escaping-block-error.rs:11:5
2221
|
2322
LL | async { *x }
24-
| ^^^^^^^^--^^
25-
| | |
26-
| | `x` is borrowed here
23+
| ^^^^^ -- `x` is borrowed here
24+
| |
2725
| may outlive borrowed value `x`
2826
|
2927
note: async block is returned here

‎tests/ui/async-await/async-closures/wrong-fn-kind.stderr

+11-13
Original file line numberDiff line numberDiff line change
@@ -22,20 +22,18 @@ LL | fn needs_async_fn(_: impl async Fn()) {}
2222
error[E0596]: cannot borrow `x` as mutable, as it is a captured variable in a `Fn` closure
2323
--> $DIR/wrong-fn-kind.rs:9:20
2424
|
25-
LL | fn needs_async_fn(_: impl async Fn()) {}
26-
| --------------- change this to accept `FnMut` instead of `Fn`
25+
LL | fn needs_async_fn(_: impl async Fn()) {}
26+
| --------------- change this to accept `FnMut` instead of `Fn`
2727
...
28-
LL | needs_async_fn(async || {
29-
| -------------- ^-------
30-
| | |
31-
| _____|______________in this closure
32-
| | |
33-
| | expects `Fn` instead of `FnMut`
34-
LL | |
35-
LL | | x += 1;
36-
| | - mutable borrow occurs due to use of `x` in closure
37-
LL | | });
38-
| |_____^ cannot borrow as mutable
28+
LL | needs_async_fn(async || {
29+
| -------------- ^^^^^^^^
30+
| | |
31+
| | cannot borrow as mutable
32+
| | in this closure
33+
| expects `Fn` instead of `FnMut`
34+
LL |
35+
LL | x += 1;
36+
| - mutable borrow occurs due to use of `x` in closure
3937

4038
error: aborting due to 2 previous errors
4139

‎tests/ui/async-await/async-is-unwindsafe.stderr

+12-13
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,19 @@
11
error[E0277]: the type `&mut Context<'_>` may not be safely transferred across an unwind boundary
22
--> $DIR/async-is-unwindsafe.rs:12:5
33
|
4-
LL | is_unwindsafe(async {
5-
| ______^ -
6-
| | ___________________|
7-
LL | ||
8-
LL | || use std::ptr::null;
9-
LL | || use std::task::{Context, RawWaker, RawWakerVTable, Waker};
10-
... ||
11-
LL | || drop(cx_ref);
12-
LL | || });
13-
| ||_____-^ `&mut Context<'_>` may not be safely transferred across an unwind boundary
14-
| |_____|
15-
| within this `{async block@$DIR/async-is-unwindsafe.rs:12:19: 29:6}`
4+
LL | is_unwindsafe(async {
5+
| ^ ----- within this `{async block@$DIR/async-is-unwindsafe.rs:12:19: 12:24}`
6+
| _____|
7+
| |
8+
LL | |
9+
LL | | use std::ptr::null;
10+
LL | | use std::task::{Context, RawWaker, RawWakerVTable, Waker};
11+
... |
12+
LL | | drop(cx_ref);
13+
LL | | });
14+
| |______^ `&mut Context<'_>` may not be safely transferred across an unwind boundary
1615
|
17-
= 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`
16+
= 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`
1817
= note: `UnwindSafe` is implemented for `&Context<'_>`, but not for `&mut Context<'_>`
1918
note: future does not implement `UnwindSafe` as this value is used across an await
2019
--> $DIR/async-is-unwindsafe.rs:25:18

‎tests/ui/async-await/coroutine-desc.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ LL | fun(async {}, async {});
88
| | expected all arguments to be this `async` block type because they need to match the type of this parameter
99
| arguments to this function are incorrect
1010
|
11-
= note: expected `async` block `{async block@$DIR/coroutine-desc.rs:10:9: 10:17}`
12-
found `async` block `{async block@$DIR/coroutine-desc.rs:10:19: 10:27}`
11+
= note: expected `async` block `{async block@$DIR/coroutine-desc.rs:10:9: 10:14}`
12+
found `async` block `{async block@$DIR/coroutine-desc.rs:10:19: 10:24}`
1313
= note: no two async blocks, even if identical, have the same type
1414
= help: consider pinning your async block and casting it to a trait object
1515
note: function defined here

‎tests/ui/async-await/coroutine-not-future.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,11 @@ note: required by a bound in `takes_coroutine`
2626
LL | fn takes_coroutine<ResumeTy>(_g: impl Coroutine<ResumeTy, Yield = (), Return = ()>) {}
2727
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `takes_coroutine`
2828

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

0 commit comments

Comments
 (0)