Skip to content

Commit 5619b64

Browse files
anatawa12anatawa12
authored
Don't remove block label from closure body (#6468)
1 parent 054efdd commit 5619b64

File tree

3 files changed

+418
-8
lines changed

3 files changed

+418
-8
lines changed

‎src/closures.rs‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use rustc_ast::{ast, ptr};
1+
use rustc_ast::{Label, ast, ptr};
22
use rustc_span::Span;
33
use thin_vec::thin_vec;
44
use tracing::debug;
@@ -72,7 +72,7 @@ pub(crate) fn rewrite_closure(
7272

7373
result.or_else(|_| {
7474
// Either we require a block, or tried without and failed.
75-
rewrite_closure_block(block, &prefix, context, body_shape)
75+
rewrite_closure_block(body, &prefix, context, body_shape)
7676
})
7777
} else {
7878
rewrite_closure_expr(body, &prefix, context, body_shape).or_else(|_| {
@@ -104,8 +104,8 @@ fn get_inner_expr<'a>(
104104
prefix: &str,
105105
context: &RewriteContext<'_>,
106106
) -> &'a ast::Expr {
107-
if let ast::ExprKind::Block(ref block, _) = expr.kind {
108-
if !needs_block(block, prefix, context) {
107+
if let ast::ExprKind::Block(ref block, ref label) = expr.kind {
108+
if !needs_block(block, label, prefix, context) {
109109
// block.stmts.len() == 1 except with `|| {{}}`;
110110
// https://github.com/rust-lang/rustfmt/issues/3844
111111
if let Some(expr) = block.stmts.first().and_then(stmt_expr) {
@@ -118,7 +118,12 @@ fn get_inner_expr<'a>(
118118
}
119119

120120
// Figure out if a block is necessary.
121-
fn needs_block(block: &ast::Block, prefix: &str, context: &RewriteContext<'_>) -> bool {
121+
fn needs_block(
122+
block: &ast::Block,
123+
label: &Option<Label>,
124+
prefix: &str,
125+
context: &RewriteContext<'_>,
126+
) -> bool {
122127
let has_attributes = block.stmts.first().map_or(false, |first_stmt| {
123128
!get_attrs_from_stmt(first_stmt).is_empty()
124129
});
@@ -128,6 +133,7 @@ fn needs_block(block: &ast::Block, prefix: &str, context: &RewriteContext<'_>) -
128133
|| has_attributes
129134
|| block_contains_comment(context, block)
130135
|| prefix.contains('\n')
136+
|| label.is_some()
131137
}
132138

133139
fn veto_block(e: &ast::Expr) -> bool {
@@ -230,11 +236,16 @@ fn rewrite_closure_expr(
230236

231237
// Rewrite closure whose body is block.
232238
fn rewrite_closure_block(
233-
block: &ast::Block,
239+
block: &ast::Expr,
234240
prefix: &str,
235241
context: &RewriteContext<'_>,
236242
shape: Shape,
237243
) -> RewriteResult {
244+
debug_assert!(
245+
matches!(block.kind, ast::ExprKind::Block(..)),
246+
"expected a block expression"
247+
);
248+
238249
Ok(format!(
239250
"{} {}",
240251
prefix,
@@ -353,6 +364,8 @@ pub(crate) fn rewrite_last_closure(
353364
expr: &ast::Expr,
354365
shape: Shape,
355366
) -> RewriteResult {
367+
debug!("rewrite_last_closure {:?}", expr);
368+
356369
if let ast::ExprKind::Closure(ref closure) = expr.kind {
357370
let ast::Closure {
358371
ref binder,
@@ -366,10 +379,11 @@ pub(crate) fn rewrite_last_closure(
366379
fn_arg_span: _,
367380
} = **closure;
368381
let body = match body.kind {
369-
ast::ExprKind::Block(ref block, _)
382+
ast::ExprKind::Block(ref block, ref label)
370383
if !is_unsafe_block(block)
371384
&& !context.inside_macro()
372-
&& is_simple_block(context, block, Some(&body.attrs)) =>
385+
&& is_simple_block(context, block, Some(&body.attrs))
386+
&& label.is_none() =>
373387
{
374388
stmt_expr(&block.stmts[0]).unwrap_or(body)
375389
}

‎tests/source/closure-block-labels.rs‎

Lines changed: 192 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,192 @@
1+
// _0: in-macro
2+
// _1: last argument in function invocation
3+
// _2: non-last argument in function invocation
4+
// _3: simple expression
5+
6+
// test0: the cause reported in issue: label is used, and there is usage, multiple statements
7+
pub fn rustfmt_test0_0(condition: bool) {
8+
test_macro!(|transaction| 'block: {
9+
if condition {
10+
break 'block 0;
11+
}
12+
13+
todo!()
14+
});
15+
}
16+
17+
pub fn rustfmt_test0_1(condition: bool) {
18+
test_func(|transaction| 'block: {
19+
if condition {
20+
break 'block 0;
21+
}
22+
23+
todo!()
24+
});
25+
}
26+
27+
pub fn rustfmt_test0_2(condition: bool) {
28+
test_func2(|transaction| 'block: {
29+
if condition {
30+
break 'block 0;
31+
}
32+
33+
todo!()
34+
}, 0);
35+
}
36+
37+
pub fn rustfmt_test0_3(condition: bool) {
38+
let x = |transaction| 'block: {
39+
if condition {
40+
break 'block 0;
41+
}
42+
43+
todo!()
44+
};
45+
}
46+
47+
48+
// test1: label is unused, and there is usage, multiple statements
49+
pub fn rustfmt_test1_0(condition: bool) {
50+
test_macro!(|transaction| 'block: {
51+
if condition {
52+
todo!("");
53+
}
54+
55+
todo!()
56+
});
57+
}
58+
59+
pub fn rustfmt_test1_1(condition: bool) {
60+
test_func(|transaction| 'block: {
61+
if condition {
62+
todo!("");
63+
}
64+
65+
todo!()
66+
});
67+
}
68+
69+
pub fn rustfmt_test1_2(condition: bool) {
70+
test_func2(|transaction| 'block: {
71+
if condition {
72+
todo!("");
73+
}
74+
75+
todo!()
76+
}, 0);
77+
}
78+
79+
pub fn rustfmt_test1_3(condition: bool) {
80+
let x = |transaction| 'block: {
81+
if condition {
82+
todo!("");
83+
}
84+
85+
todo!()
86+
};
87+
}
88+
89+
90+
91+
// test2: label is used, single expression
92+
pub fn rustfmt_test2_0(condition: bool) {
93+
test_macro!(|transaction| 'block: {
94+
break 'block 0;
95+
});
96+
}
97+
98+
pub fn rustfmt_test2_1(condition: bool) {
99+
test_func(|transaction| 'block: {
100+
break 'block 0;
101+
});
102+
}
103+
104+
pub fn rustfmt_test2_2(condition: bool) {
105+
test_func2(|transaction| 'block: {
106+
break 'block 0;
107+
}, 0);
108+
}
109+
110+
pub fn rustfmt_test2_3(condition: bool) {
111+
let x = |transaction| 'block: {
112+
break 'block 0;
113+
};
114+
}
115+
116+
// test3: label is unused, single general multi-line expression
117+
pub fn rustfmt_test3_0(condition: bool) {
118+
test_macro!(|transaction| 'block: {
119+
vec![
120+
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
121+
0,
122+
]
123+
});
124+
}
125+
126+
pub fn rustfmt_test3_1(condition: bool) {
127+
test_func(|transaction| 'block: {
128+
vec![
129+
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
130+
0,
131+
]
132+
});
133+
}
134+
135+
pub fn rustfmt_test3_2(condition: bool) {
136+
test_func2(|transaction| 'block: {
137+
vec![
138+
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
139+
0,
140+
]
141+
}, 0);
142+
}
143+
144+
pub fn rustfmt_test3_3(condition: bool) {
145+
let x = |transaction| 'block: {
146+
vec![
147+
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
148+
0,
149+
]
150+
};
151+
}
152+
153+
// test4: label is unused, single block statement-expression
154+
pub fn rustfmt_test4_0(condition: bool) {
155+
test_macro!(|transaction| 'block: {
156+
if condition {
157+
break 'block 1;
158+
} else {
159+
break 'block 0;
160+
}
161+
});
162+
}
163+
164+
pub fn rustfmt_test4_1(condition: bool) {
165+
test_func(|transaction| 'block: {
166+
if condition {
167+
break 'block 1;
168+
} else {
169+
break 'block 0;
170+
}
171+
});
172+
}
173+
174+
pub fn rustfmt_test4_2(condition: bool) {
175+
test_func2(|transaction| 'block: {
176+
if condition {
177+
break 'block 1;
178+
} else {
179+
break 'block 0;
180+
}
181+
}, 1);
182+
}
183+
184+
pub fn rustfmt_test4_3(condition: bool) {
185+
let x = |transaction| 'block: {
186+
if condition {
187+
break 'block 1;
188+
} else {
189+
break 'block 0;
190+
}
191+
};
192+
}

0 commit comments

Comments
 (0)