Skip to content

Commit 729505f

Browse files
surechensurechen
committed
Fixing span manipulation and indentation of the suggestion introduced by #126187
According to: #128084 (comment) https://github.com/rust-lang/rust/pull/126187/files#r1634897691 I try to add a span field to `Block` which doesn't include brace '{' and '}', because I need to add a suggestion at the end of function's body, and this can help me find the right place. But this will make `Block` larger. I don't want to break the original span field in `Block`, I'm worried that it will cause a lot of code failures and I think it may be more intuitive for span to include braces.
1 parent 739b1fd commit 729505f

File tree

22 files changed

+117
-92
lines changed

22 files changed

+117
-92
lines changed

Diff for: ‎compiler/rustc_ast/src/ast.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -544,6 +544,9 @@ pub struct Block {
544544
/// Distinguishes between `unsafe { ... }` and `{ ... }`.
545545
pub rules: BlockCheckMode,
546546
pub span: Span,
547+
// Only for function or method written by developers that do have a block,
548+
// but not including the blocks automatically inserted by the compiler.
549+
pub no_brace_span: Option<Span>,
547550
pub tokens: Option<LazyAttrTokenStream>,
548551
/// The following *isn't* a parse error, but will cause multiple errors in following stages.
549552
/// ```compile_fail
@@ -3501,7 +3504,7 @@ mod size_asserts {
35013504
static_assert_size!(AssocItem, 88);
35023505
static_assert_size!(AssocItemKind, 16);
35033506
static_assert_size!(Attribute, 32);
3504-
static_assert_size!(Block, 32);
3507+
static_assert_size!(Block, 48);
35053508
static_assert_size!(Expr, 72);
35063509
static_assert_size!(ExprKind, 40);
35073510
static_assert_size!(Fn, 160);

Diff for: ‎compiler/rustc_ast/src/mut_visit.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -1076,7 +1076,8 @@ fn walk_mt<T: MutVisitor>(vis: &mut T, MutTy { ty, mutbl: _ }: &mut MutTy) {
10761076
}
10771077

10781078
pub fn walk_block<T: MutVisitor>(vis: &mut T, block: &mut P<Block>) {
1079-
let Block { id, stmts, rules: _, span, tokens, could_be_bare_literal: _ } = block.deref_mut();
1079+
let Block { id, stmts, rules: _, span, tokens, could_be_bare_literal: _, no_brace_span: _ } =
1080+
block.deref_mut();
10801081
vis.visit_id(id);
10811082
stmts.flat_map_in_place(|stmt| vis.flat_map_stmt(stmt));
10821083
visit_lazy_tts(vis, tokens);

Diff for: ‎compiler/rustc_ast/src/visit.rs

+9-1
Original file line numberDiff line numberDiff line change
@@ -943,7 +943,15 @@ pub fn walk_field_def<'a, V: Visitor<'a>>(visitor: &mut V, field: &'a FieldDef)
943943
}
944944

945945
pub fn walk_block<'a, V: Visitor<'a>>(visitor: &mut V, block: &'a Block) -> V::Result {
946-
let Block { stmts, id: _, rules: _, span: _, tokens: _, could_be_bare_literal: _ } = block;
946+
let Block {
947+
stmts,
948+
id: _,
949+
rules: _,
950+
span: _,
951+
tokens: _,
952+
could_be_bare_literal: _,
953+
no_brace_span: _,
954+
} = block;
947955
walk_list!(visitor, visit_stmt, stmts);
948956
V::Result::output()
949957
}

Diff for: ‎compiler/rustc_ast_lowering/src/block.rs

+12-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use rustc_ast::{Block, BlockCheckMode, Local, LocalKind, Stmt, StmtKind};
22
use rustc_hir as hir;
3+
use rustc_span::Span;
34
use smallvec::SmallVec;
45

56
use crate::{ImplTraitContext, ImplTraitPosition, LoweringContext};
@@ -21,7 +22,17 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
2122
let (stmts, expr) = self.lower_stmts(&b.stmts);
2223
let rules = self.lower_block_check_mode(&b.rules);
2324
let hir_id = self.lower_node_id(b.id);
24-
hir::Block { hir_id, stmts, expr, rules, span: self.lower_span(b.span), targeted_by_break }
25+
let lower = |span: Span| -> Option<Span> { Some(self.lower_span(span)) };
26+
let no_brace_span = b.no_brace_span.and_then(lower);
27+
hir::Block {
28+
hir_id,
29+
stmts,
30+
expr,
31+
rules,
32+
span: self.lower_span(b.span),
33+
no_brace_span,
34+
targeted_by_break,
35+
}
2536
}
2637

2738
fn lower_stmts(

Diff for: ‎compiler/rustc_ast_lowering/src/delegation.rs

+1
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
353353
hir_id: self.next_id(),
354354
rules: hir::BlockCheckMode::DefaultBlock,
355355
span,
356+
no_brace_span: None,
356357
targeted_by_break: false,
357358
});
358359

Diff for: ‎compiler/rustc_ast_lowering/src/expr.rs

+2
Original file line numberDiff line numberDiff line change
@@ -592,6 +592,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
592592
hir_id: self.next_id(),
593593
rules: hir::BlockCheckMode::DefaultBlock,
594594
span,
595+
no_brace_span: None,
595596
targeted_by_break: false,
596597
});
597598
self.arena.alloc(hir::Expr {
@@ -2116,6 +2117,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
21162117
hir_id,
21172118
rules: hir::BlockCheckMode::UnsafeBlock(hir::UnsafeSource::CompilerGenerated),
21182119
span: self.lower_span(span),
2120+
no_brace_span: None,
21192121
targeted_by_break: false,
21202122
}),
21212123
None,

Diff for: ‎compiler/rustc_ast_lowering/src/format.rs

+1
Original file line numberDiff line numberDiff line change
@@ -612,6 +612,7 @@ fn expand_format_args<'hir>(
612612
hir_id,
613613
rules: hir::BlockCheckMode::UnsafeBlock(hir::UnsafeSource::CompilerGenerated),
614614
span: macsp,
615+
no_brace_span: None,
615616
targeted_by_break: false,
616617
}));
617618
let args = ctx.arena.alloc_from_iter([lit_pieces, args, format_options, unsafe_arg]);

Diff for: ‎compiler/rustc_ast_lowering/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -2571,6 +2571,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
25712571
hir_id: self.next_id(),
25722572
rules: hir::BlockCheckMode::DefaultBlock,
25732573
span: self.lower_span(span),
2574+
no_brace_span: None,
25742575
targeted_by_break: false,
25752576
};
25762577
self.arena.alloc(blk)

Diff for: ‎compiler/rustc_builtin_macros/src/deriving/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ fn call_unreachable(cx: &ExtCtxt<'_>, span: Span) -> P<ast::Expr> {
112112
id: ast::DUMMY_NODE_ID,
113113
rules: ast::BlockCheckMode::Unsafe(ast::CompilerGenerated),
114114
span,
115+
no_brace_span: None,
115116
tokens: None,
116117
could_be_bare_literal: false,
117118
}))

Diff for: ‎compiler/rustc_expand/src/build.rs

+1
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,7 @@ impl<'a> ExtCtxt<'a> {
247247
id: ast::DUMMY_NODE_ID,
248248
rules: BlockCheckMode::Default,
249249
span,
250+
no_brace_span: None,
250251
tokens: None,
251252
could_be_bare_literal: false,
252253
})

Diff for: ‎compiler/rustc_hir/src/hir.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -1095,6 +1095,10 @@ pub struct Block<'hir> {
10951095
pub rules: BlockCheckMode,
10961096
/// The span includes the curly braces `{` and `}` around the block.
10971097
pub span: Span,
1098+
// The span doesn't include the curly braces `{` and `}` around the block.
1099+
// Only for function or method written by developers that do have a block,
1100+
// but not including the blocks automatically inserted by the compiler.
1101+
pub no_brace_span: Option<Span>,
10981102
/// If true, then there may exist `break 'a` values that aim to
10991103
/// break out of this block early.
11001104
/// Used by `'label: {}` blocks and by `try {}` blocks.
@@ -4040,7 +4044,7 @@ mod size_asserts {
40404044

40414045
use super::*;
40424046
// tidy-alphabetical-start
4043-
static_assert_size!(Block<'_>, 48);
4047+
static_assert_size!(Block<'_>, 56);
40444048
static_assert_size!(Body<'_>, 24);
40454049
static_assert_size!(Expr<'_>, 64);
40464050
static_assert_size!(ExprKind<'_>, 48);

Diff for: ‎compiler/rustc_parse/src/parser/diagnostics.rs

+1
Original file line numberDiff line numberDiff line change
@@ -928,6 +928,7 @@ impl<'a> Parser<'a> {
928928
thin_vec![self.mk_stmt_err(expr.span, guar)],
929929
s,
930930
lo.to(self.prev_token.span),
931+
None,
931932
);
932933
tail.could_be_bare_literal = true;
933934
if maybe_struct_name.is_ident() && can_be_struct_literal {

Diff for: ‎compiler/rustc_parse/src/parser/expr.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -1731,7 +1731,7 @@ impl<'a> Parser<'a> {
17311731

17321732
// Replace `'label: non_block_expr` with `'label: {non_block_expr}` in order to suppress future errors about `break 'label`.
17331733
let stmt = self.mk_stmt(span, StmtKind::Expr(expr));
1734-
let blk = self.mk_block(thin_vec![stmt], BlockCheckMode::Default, span);
1734+
let blk = self.mk_block(thin_vec![stmt], BlockCheckMode::Default, span, None);
17351735
self.mk_expr(span, ExprKind::Block(blk, label))
17361736
});
17371737

@@ -2847,7 +2847,8 @@ impl<'a> Parser<'a> {
28472847
.dcx()
28482848
.emit_err(errors::MissingExpressionInForLoop { span: expr.span.shrink_to_lo() });
28492849
let err_expr = self.mk_expr(expr.span, ExprKind::Err(guar));
2850-
let block = self.mk_block(thin_vec![], BlockCheckMode::Default, self.prev_token.span);
2850+
let block =
2851+
self.mk_block(thin_vec![], BlockCheckMode::Default, self.prev_token.span, None);
28512852
return Ok(self.mk_expr(
28522853
lo.to(self.prev_token.span),
28532854
ExprKind::ForLoop { pat, iter: err_expr, body: block, label: opt_label, kind },

Diff for: ‎compiler/rustc_parse/src/parser/stmt.rs

+12-2
Original file line numberDiff line numberDiff line change
@@ -578,6 +578,8 @@ impl<'a> Parser<'a> {
578578
) -> PResult<'a, P<Block>> {
579579
let mut stmts = ThinVec::new();
580580
let mut snapshot = None;
581+
let start_without_brace_sp = self.token.span;
582+
let mut end_without_brace_sp = self.token.span;
581583
while !self.eat(&token::CloseDelim(Delimiter::Brace)) {
582584
if self.token == token::Eof {
583585
break;
@@ -641,8 +643,14 @@ impl<'a> Parser<'a> {
641643
// Found only `;` or `}`.
642644
continue;
643645
};
646+
end_without_brace_sp = self.prev_token.span;
644647
}
645-
Ok(self.mk_block(stmts, s, lo.to(self.prev_token.span)))
648+
Ok(self.mk_block(
649+
stmts,
650+
s,
651+
lo.to(self.prev_token.span),
652+
Some(start_without_brace_sp.to(end_without_brace_sp)),
653+
))
646654
}
647655

648656
/// Parses a statement, including the trailing semicolon.
@@ -843,12 +851,14 @@ impl<'a> Parser<'a> {
843851
stmts: ThinVec<Stmt>,
844852
rules: BlockCheckMode,
845853
span: Span,
854+
no_brace_span: Option<Span>,
846855
) -> P<Block> {
847856
P(Block {
848857
stmts,
849858
id: DUMMY_NODE_ID,
850859
rules,
851860
span,
861+
no_brace_span,
852862
tokens: None,
853863
could_be_bare_literal: false,
854864
})
@@ -863,6 +873,6 @@ impl<'a> Parser<'a> {
863873
}
864874

865875
pub(super) fn mk_block_err(&self, span: Span, guar: ErrorGuaranteed) -> P<Block> {
866-
self.mk_block(thin_vec![self.mk_stmt_err(span, guar)], BlockCheckMode::Default, span)
876+
self.mk_block(thin_vec![self.mk_stmt_err(span, guar)], BlockCheckMode::Default, span, None)
867877
}
868878
}

Diff for: ‎compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs

+2-5
Original file line numberDiff line numberDiff line change
@@ -4666,12 +4666,9 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
46664666
let body = self.tcx.hir().body(body_id);
46674667
if let hir::ExprKind::Block(b, _) = body.value.kind
46684668
&& b.expr.is_none()
4669+
&& let Some(span) = b.no_brace_span
46694670
{
4670-
sugg_spans.push((
4671-
// The span will point to the closing curly brace `}` of the block.
4672-
b.span.shrink_to_hi().with_lo(b.span.hi() - BytePos(1)),
4673-
"\n Ok(())\n}".to_string(),
4674-
));
4671+
sugg_spans.push((span.shrink_to_hi(), "\n Ok(())".to_string()));
46754672
}
46764673
err.multipart_suggestion_verbose(
46774674
format!("consider adding return type"),

Diff for: ‎src/tools/rustfmt/src/closures.rs

+1
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,7 @@ fn rewrite_closure_with_block(
174174
.map(|attr| attr.span.to(body.span))
175175
.unwrap_or(body.span),
176176
could_be_bare_literal: false,
177+
no_brace_span: None,
177178
};
178179
let block = crate::expr::rewrite_block_with_visitor(
179180
context,

Diff for: ‎src/tools/rustfmt/src/macros.rs

+1
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,7 @@ fn rewrite_empty_macro_def_body(
393393
span,
394394
tokens: None,
395395
could_be_bare_literal: false,
396+
no_brace_span: None,
396397
};
397398
block.rewrite(context, shape)
398399
}

Diff for: ‎tests/ui-fulldeps/pprust-expr-roundtrip.rs

+1
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ fn iter_exprs(depth: usize, f: &mut dyn FnMut(P<Expr>)) {
125125
span: DUMMY_SP,
126126
tokens: None,
127127
could_be_bare_literal: false,
128+
no_brace_span: None,
128129
});
129130
iter_exprs(depth - 1, &mut |e| g(ExprKind::If(e, block.clone(), None)));
130131
}

Diff for: ‎tests/ui/return/return-from-residual-sugg-issue-125997.fixed

+5-11
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,14 @@ use std::io::prelude::*;
88

99
fn test1() -> Result<(), Box<dyn std::error::Error>> {
1010
let mut _file = File::create("foo.txt")?;
11-
//~^ ERROR the `?` operator can only be used in a function
12-
1311
Ok(())
12+
//~^ ERROR the `?` operator can only be used in a function
1413
}
1514

1615
fn test2() -> Result<(), Box<dyn std::error::Error>> {
1716
let mut _file = File::create("foo.txt")?;
1817
//~^ ERROR the `?` operator can only be used in a function
1918
println!();
20-
2119
Ok(())
2220
}
2321

@@ -27,9 +25,8 @@ macro_rules! mac {
2725
let mut _file = File::create("foo.txt")?;
2826
//~^ ERROR the `?` operator can only be used in a function
2927
println!();
30-
3128
Ok(())
32-
}
29+
}
3330
};
3431
}
3532

@@ -38,24 +35,21 @@ struct A;
3835
impl A {
3936
fn test4(&self) -> Result<(), Box<dyn std::error::Error>> {
4037
let mut _file = File::create("foo.txt")?;
41-
//~^ ERROR the `?` operator can only be used in a method
42-
4338
Ok(())
44-
}
39+
//~^ ERROR the `?` operator can only be used in a method
40+
}
4541

4642
fn test5(&self) -> Result<(), Box<dyn std::error::Error>> {
4743
let mut _file = File::create("foo.txt")?;
4844
//~^ ERROR the `?` operator can only be used in a method
4945
println!();
50-
5146
Ok(())
52-
}
47+
}
5348
}
5449

5550
fn main() -> Result<(), Box<dyn std::error::Error>> {
5651
let mut _file = File::create("foo.txt")?;
5752
//~^ ERROR the `?` operator can only be used in a function
5853
mac!();
59-
6054
Ok(())
6155
}

Diff for: ‎tests/ui/return/return-from-residual-sugg-issue-125997.stderr

+6-20
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,8 @@ LL | let mut _file = File::create("foo.txt")?;
1010
help: consider adding return type
1111
|
1212
LL ~ fn test1() -> Result<(), Box<dyn std::error::Error>> {
13-
LL | let mut _file = File::create("foo.txt")?;
14-
LL |
15-
LL +
13+
LL ~ let mut _file = File::create("foo.txt")?;
1614
LL + Ok(())
17-
LL + }
1815
|
1916

2017
error[E0277]: the `?` operator can only be used in a function that returns `Result` or `Option` (or another type that implements `FromResidual`)
@@ -31,10 +28,8 @@ help: consider adding return type
3128
LL ~ fn test2() -> Result<(), Box<dyn std::error::Error>> {
3229
LL | let mut _file = File::create("foo.txt")?;
3330
LL |
34-
LL | println!();
35-
LL +
31+
LL ~ println!();
3632
LL + Ok(())
37-
LL + }
3833
|
3934

4035
error[E0277]: the `?` operator can only be used in a method that returns `Result` or `Option` (or another type that implements `FromResidual`)
@@ -49,11 +44,8 @@ LL | let mut _file = File::create("foo.txt")?;
4944
help: consider adding return type
5045
|
5146
LL ~ fn test4(&self) -> Result<(), Box<dyn std::error::Error>> {
52-
LL | let mut _file = File::create("foo.txt")?;
53-
LL |
54-
LL ~
47+
LL ~ let mut _file = File::create("foo.txt")?;
5548
LL + Ok(())
56-
LL + }
5749
|
5850

5951
error[E0277]: the `?` operator can only be used in a method that returns `Result` or `Option` (or another type that implements `FromResidual`)
@@ -70,10 +62,8 @@ help: consider adding return type
7062
LL ~ fn test5(&self) -> Result<(), Box<dyn std::error::Error>> {
7163
LL | let mut _file = File::create("foo.txt")?;
7264
LL |
73-
LL | println!();
74-
LL ~
65+
LL ~ println!();
7566
LL + Ok(())
76-
LL + }
7767
|
7868

7969
error[E0277]: the `?` operator can only be used in a function that returns `Result` or `Option` (or another type that implements `FromResidual`)
@@ -90,10 +80,8 @@ help: consider adding return type
9080
LL ~ fn main() -> Result<(), Box<dyn std::error::Error>> {
9181
LL | let mut _file = File::create("foo.txt")?;
9282
LL |
93-
LL | mac!();
94-
LL +
83+
LL ~ mac!();
9584
LL + Ok(())
96-
LL + }
9785
|
9886

9987
error[E0277]: the `?` operator can only be used in a function that returns `Result` or `Option` (or another type that implements `FromResidual`)
@@ -114,10 +102,8 @@ help: consider adding return type
114102
LL ~ fn test3() -> Result<(), Box<dyn std::error::Error>> {
115103
LL | let mut _file = File::create("foo.txt")?;
116104
LL |
117-
LL | println!();
118-
LL ~
105+
LL ~ println!();
119106
LL + Ok(())
120-
LL + }
121107
|
122108

123109
error: aborting due to 6 previous errors

0 commit comments

Comments
 (0)