Skip to content

Commit 783948f

Browse files
authored
Fix handling of match arm's rewrite (#3775)
1 parent e81ec20 commit 783948f

File tree

6 files changed

+95
-31
lines changed

6 files changed

+95
-31
lines changed

src/macros.rs

+11-22
Original file line numberDiff line numberDiff line change
@@ -190,24 +190,6 @@ fn return_macro_parse_failure_fallback(
190190
Some(context.snippet(span).to_owned())
191191
}
192192

193-
struct InsideMacroGuard<'a> {
194-
context: &'a RewriteContext<'a>,
195-
is_nested: bool,
196-
}
197-
198-
impl<'a> InsideMacroGuard<'a> {
199-
fn inside_macro_context(context: &'a RewriteContext<'_>) -> InsideMacroGuard<'a> {
200-
let is_nested = context.inside_macro.replace(true);
201-
InsideMacroGuard { context, is_nested }
202-
}
203-
}
204-
205-
impl<'a> Drop for InsideMacroGuard<'a> {
206-
fn drop(&mut self) {
207-
self.context.inside_macro.replace(self.is_nested);
208-
}
209-
}
210-
211193
pub(crate) fn rewrite_macro(
212194
mac: &ast::Mac,
213195
extra_ident: Option<ast::Ident>,
@@ -221,9 +203,16 @@ pub(crate) fn rewrite_macro(
221203
if should_skip {
222204
None
223205
} else {
224-
let guard = InsideMacroGuard::inside_macro_context(context);
206+
let guard = context.enter_macro();
225207
let result = catch_unwind(AssertUnwindSafe(|| {
226-
rewrite_macro_inner(mac, extra_ident, context, shape, position, guard.is_nested)
208+
rewrite_macro_inner(
209+
mac,
210+
extra_ident,
211+
context,
212+
shape,
213+
position,
214+
guard.is_nested(),
215+
)
227216
}));
228217
match result {
229218
Err(..) | Ok(None) => {
@@ -263,7 +252,7 @@ fn rewrite_macro_inner(
263252
) -> Option<String> {
264253
if context.config.use_try_shorthand() {
265254
if let Some(expr) = convert_try_mac(mac, context) {
266-
context.inside_macro.replace(false);
255+
context.leave_macro();
267256
return expr.rewrite(context, shape);
268257
}
269258
}
@@ -412,7 +401,7 @@ fn rewrite_macro_inner(
412401
Some(SeparatorTactic::Never)
413402
};
414403
if FORCED_BRACKET_MACROS.contains(macro_name) && !is_nested_macro {
415-
context.inside_macro.replace(false);
404+
context.leave_macro();
416405
if context.use_block_indent() {
417406
force_trailing_comma = Some(SeparatorTactic::Vertical);
418407
};

src/matches.rs

+5-7
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use crate::source_map::SpanUtils;
1919
use crate::spanned::Spanned;
2020
use crate::utils::{
2121
contains_skip, extra_offset, first_line_width, inner_attributes, last_line_extendable, mk_sp,
22-
ptr_vec_to_ref_vec, semicolon_for_expr, trimmed_last_line_width,
22+
ptr_vec_to_ref_vec, semicolon_for_expr, trimmed_last_line_width, unicode_str_width,
2323
};
2424

2525
/// A simple wrapper type against `ast::Arm`. Used inside `write_list()`.
@@ -452,7 +452,9 @@ fn rewrite_match_body(
452452

453453
match rewrite {
454454
Some(ref body_str)
455-
if is_block || (!body_str.contains('\n') && body_str.len() <= body_shape.width) =>
455+
if is_block
456+
|| (!body_str.contains('\n')
457+
&& unicode_str_width(body_str) <= body_shape.width) =>
456458
{
457459
return combine_orig_body(body_str);
458460
}
@@ -470,11 +472,6 @@ fn rewrite_match_body(
470472
next_line_body_shape.width,
471473
);
472474
match (orig_body, next_line_body) {
473-
(Some(ref orig_str), Some(ref next_line_str))
474-
if orig_str == next_line_str || context.inside_macro() =>
475-
{
476-
combine_orig_body(orig_str)
477-
}
478475
(Some(ref orig_str), Some(ref next_line_str))
479476
if prefer_next_line(orig_str, next_line_str, RhsTactics::Default) =>
480477
{
@@ -575,6 +572,7 @@ fn can_flatten_block_around_this(body: &ast::Expr) -> bool {
575572
| ast::ExprKind::Box(ref expr)
576573
| ast::ExprKind::Try(ref expr)
577574
| ast::ExprKind::Unary(_, ref expr)
575+
| ast::ExprKind::Index(ref expr, _)
578576
| ast::ExprKind::Cast(ref expr, _) => can_flatten_block_around_this(expr),
579577
_ => false,
580578
}

src/rewrite.rs

+30-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ pub(crate) struct RewriteContext<'a> {
2929
pub(crate) parse_session: &'a ParseSess,
3030
pub(crate) source_map: &'a SourceMap,
3131
pub(crate) config: &'a Config,
32-
pub(crate) inside_macro: RefCell<bool>,
32+
pub(crate) inside_macro: Rc<RefCell<bool>>,
3333
// Force block indent style even if we are using visual indent style.
3434
pub(crate) use_block: RefCell<bool>,
3535
// When `is_if_else_block` is true, unindent the comment on top
@@ -45,6 +45,23 @@ pub(crate) struct RewriteContext<'a> {
4545
pub(crate) skipped_range: Rc<RefCell<Vec<(usize, usize)>>>,
4646
}
4747

48+
pub(crate) struct InsideMacroGuard {
49+
is_nested_macro_context: bool,
50+
inside_macro_ref: Rc<RefCell<bool>>,
51+
}
52+
53+
impl InsideMacroGuard {
54+
pub(crate) fn is_nested(&self) -> bool {
55+
self.is_nested_macro_context
56+
}
57+
}
58+
59+
impl Drop for InsideMacroGuard {
60+
fn drop(&mut self) {
61+
self.inside_macro_ref.replace(self.is_nested_macro_context);
62+
}
63+
}
64+
4865
impl<'a> RewriteContext<'a> {
4966
pub(crate) fn snippet(&self, span: Span) -> &str {
5067
self.snippet_provider.span_to_snippet(span).unwrap()
@@ -63,6 +80,18 @@ impl<'a> RewriteContext<'a> {
6380
*self.inside_macro.borrow()
6481
}
6582

83+
pub(crate) fn enter_macro(&self) -> InsideMacroGuard {
84+
let is_nested_macro_context = self.inside_macro.replace(true);
85+
InsideMacroGuard {
86+
is_nested_macro_context,
87+
inside_macro_ref: self.inside_macro.clone(),
88+
}
89+
}
90+
91+
pub(crate) fn leave_macro(&self) {
92+
self.inside_macro.replace(false);
93+
}
94+
6695
pub(crate) fn is_if_else_block(&self) -> bool {
6796
*self.is_if_else_block.borrow()
6897
}

src/visitor.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -870,7 +870,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
870870
parse_session: self.parse_session,
871871
source_map: self.source_map,
872872
config: self.config,
873-
inside_macro: RefCell::new(false),
873+
inside_macro: Rc::new(RefCell::new(false)),
874874
use_block: RefCell::new(false),
875875
is_if_else_block: RefCell::new(false),
876876
force_one_line_chain: RefCell::new(false),

tests/source/match.rs

+19
Original file line numberDiff line numberDiff line change
@@ -549,3 +549,22 @@ fn issue_3005() {
549549
},
550550
}
551551
}
552+
553+
// #3774
554+
fn issue_3774() {
555+
{
556+
{
557+
{
558+
match foo {
559+
Lam(_, _, _) | Pi(_, _, _) | Let(_, _, _, _) | Embed(_) | Var(_) => unreachab(),
560+
Lam(_, _, _) | Pi(_, _, _) | Let(_, _, _, _) | Embed(_) | Var(_) => unreacha!(),
561+
Lam(_, _, _) | Pi(_, _, _) | Let(_, _, _, _) | Embed(_) | Var(_) => unreachabl(),
562+
Lam(_, _, _) | Pi(_, _, _) | Let(_, _, _, _) | Embed(_) | Var(_) => unreachae!(),
563+
Lam(_, _, _) | Pi(_, _, _) | Let(_, _, _, _) | Embed(_) | Var(_) => unreachable(),
564+
Lam(_, _, _) | Pi(_, _, _) | Let(_, _, _, _) | Embed(_) | Var(_) => unreachable!(),
565+
Lam(_, _, _) | Pi(_, _, _) | Let(_, _, _, _) | Embed(_) | Var(_) => rrunreachable!(),
566+
}
567+
}
568+
}
569+
}
570+
}

tests/target/match.rs

+29
Original file line numberDiff line numberDiff line change
@@ -574,3 +574,32 @@ fn issue_3005() {
574574
}
575575
}
576576
}
577+
578+
// #3774
579+
fn issue_3774() {
580+
{
581+
{
582+
{
583+
match foo {
584+
Lam(_, _, _) | Pi(_, _, _) | Let(_, _, _, _) | Embed(_) | Var(_) => unreachab(),
585+
Lam(_, _, _) | Pi(_, _, _) | Let(_, _, _, _) | Embed(_) | Var(_) => unreacha!(),
586+
Lam(_, _, _) | Pi(_, _, _) | Let(_, _, _, _) | Embed(_) | Var(_) => {
587+
unreachabl()
588+
}
589+
Lam(_, _, _) | Pi(_, _, _) | Let(_, _, _, _) | Embed(_) | Var(_) => {
590+
unreachae!()
591+
}
592+
Lam(_, _, _) | Pi(_, _, _) | Let(_, _, _, _) | Embed(_) | Var(_) => {
593+
unreachable()
594+
}
595+
Lam(_, _, _) | Pi(_, _, _) | Let(_, _, _, _) | Embed(_) | Var(_) => {
596+
unreachable!()
597+
}
598+
Lam(_, _, _) | Pi(_, _, _) | Let(_, _, _, _) | Embed(_) | Var(_) => {
599+
rrunreachable!()
600+
}
601+
}
602+
}
603+
}
604+
}
605+
}

0 commit comments

Comments
 (0)