From b21c80cb2598bf477e61aba14085e4a7f14d53b9 Mon Sep 17 00:00:00 2001 From: Alexander Glusker Date: Fri, 29 Mar 2024 10:29:39 +0300 Subject: [PATCH 1/3] Ignoring empty statements in closures. Resolve #6116 --- src/closures.rs | 16 ++++++++++++++-- tests/source/issue-6116/main.rs | 7 +++++++ tests/target/issue-6116/main.rs | 3 +++ 3 files changed, 24 insertions(+), 2 deletions(-) create mode 100644 tests/source/issue-6116/main.rs create mode 100644 tests/target/issue-6116/main.rs diff --git a/src/closures.rs b/src/closures.rs index 1f59fbc2960..4aa0930e9d0 100644 --- a/src/closures.rs +++ b/src/closures.rs @@ -108,7 +108,10 @@ fn get_inner_expr<'a>( if !needs_block(block, prefix, context) { // block.stmts.len() == 1 except with `|| {{}}`; // https://github.com/rust-lang/rustfmt/issues/3844 - if let Some(expr) = block.stmts.first().and_then(stmt_expr) { + if let Some(expr) = iter_stmts_without_empty(&block.stmts) + .next() + .and_then(stmt_expr) + { return get_inner_expr(expr, prefix, context); } } @@ -117,6 +120,15 @@ fn get_inner_expr<'a>( expr } +fn iter_stmts_without_empty( + stmts: &thin_vec::ThinVec, +) -> impl Iterator { + stmts.iter().filter(|x| match x.kind { + crate::ast::StmtKind::Empty => false, + _ => true, + }) +} + // Figure out if a block is necessary. fn needs_block(block: &ast::Block, prefix: &str, context: &RewriteContext<'_>) -> bool { let has_attributes = block.stmts.first().map_or(false, |first_stmt| { @@ -124,7 +136,7 @@ fn needs_block(block: &ast::Block, prefix: &str, context: &RewriteContext<'_>) - }); is_unsafe_block(block) - || block.stmts.len() > 1 + || iter_stmts_without_empty(&block.stmts).count() > 1 || has_attributes || block_contains_comment(context, block) || prefix.contains('\n') diff --git a/tests/source/issue-6116/main.rs b/tests/source/issue-6116/main.rs new file mode 100644 index 00000000000..910131a4761 --- /dev/null +++ b/tests/source/issue-6116/main.rs @@ -0,0 +1,7 @@ +fn foo() -> fn(i32) -> i32 { + |a| { + ; + a + } +} + diff --git a/tests/target/issue-6116/main.rs b/tests/target/issue-6116/main.rs new file mode 100644 index 00000000000..dc0e48f1bfe --- /dev/null +++ b/tests/target/issue-6116/main.rs @@ -0,0 +1,3 @@ +fn foo() -> fn(i32) -> i32 { + |a| a +} From d40e2890fa6a8676497531a91c42d38f43e763d1 Mon Sep 17 00:00:00 2001 From: Alexander Glusker Date: Sat, 30 Mar 2024 02:49:01 +0300 Subject: [PATCH 2/3] review patch issue-6116 --- src/closures.rs | 10 +++------- tests/source/issue-6116/main2.rs | 8 ++++++++ tests/source/issue-6116/main3.rs | 8 ++++++++ tests/source/issue-6116/main4.rs | 7 +++++++ tests/target/issue-6116/main2.rs | 6 ++++++ tests/target/issue-6116/main3.rs | 3 +++ tests/target/issue-6116/main4.rs | 6 ++++++ 7 files changed, 41 insertions(+), 7 deletions(-) create mode 100644 tests/source/issue-6116/main2.rs create mode 100644 tests/source/issue-6116/main3.rs create mode 100644 tests/source/issue-6116/main4.rs create mode 100644 tests/target/issue-6116/main2.rs create mode 100644 tests/target/issue-6116/main3.rs create mode 100644 tests/target/issue-6116/main4.rs diff --git a/src/closures.rs b/src/closures.rs index 4aa0930e9d0..8c53d9d7579 100644 --- a/src/closures.rs +++ b/src/closures.rs @@ -1,3 +1,4 @@ +use rustc_ast::ast::StmtKind; use rustc_ast::{ast, ptr}; use rustc_span::Span; use thin_vec::thin_vec; @@ -120,13 +121,8 @@ fn get_inner_expr<'a>( expr } -fn iter_stmts_without_empty( - stmts: &thin_vec::ThinVec, -) -> impl Iterator { - stmts.iter().filter(|x| match x.kind { - crate::ast::StmtKind::Empty => false, - _ => true, - }) +fn iter_stmts_without_empty(stmts: &[ast::Stmt]) -> impl Iterator { + stmts.iter().filter(|x| !matches!(x.kind, StmtKind::Empty)) } // Figure out if a block is necessary. diff --git a/tests/source/issue-6116/main2.rs b/tests/source/issue-6116/main2.rs new file mode 100644 index 00000000000..1114fb1c461 --- /dev/null +++ b/tests/source/issue-6116/main2.rs @@ -0,0 +1,8 @@ +fn bar() -> fn(i32) -> i32 { + |a| { + ; + a; + b + } +} + diff --git a/tests/source/issue-6116/main3.rs b/tests/source/issue-6116/main3.rs new file mode 100644 index 00000000000..dbba9d96164 --- /dev/null +++ b/tests/source/issue-6116/main3.rs @@ -0,0 +1,8 @@ +fn foo() -> fn(i32) -> i32 { + |a| { + ; + ; + ;;;; + a + } +} diff --git a/tests/source/issue-6116/main4.rs b/tests/source/issue-6116/main4.rs new file mode 100644 index 00000000000..754aed66e72 --- /dev/null +++ b/tests/source/issue-6116/main4.rs @@ -0,0 +1,7 @@ +fn foo() -> fn(i32) -> i32 { + |a| { + /*comment before empty statement */; + a + } +} + diff --git a/tests/target/issue-6116/main2.rs b/tests/target/issue-6116/main2.rs new file mode 100644 index 00000000000..89679587ebc --- /dev/null +++ b/tests/target/issue-6116/main2.rs @@ -0,0 +1,6 @@ +fn bar() -> fn(i32) -> i32 { + |a| { + a; + b + } +} diff --git a/tests/target/issue-6116/main3.rs b/tests/target/issue-6116/main3.rs new file mode 100644 index 00000000000..dc0e48f1bfe --- /dev/null +++ b/tests/target/issue-6116/main3.rs @@ -0,0 +1,3 @@ +fn foo() -> fn(i32) -> i32 { + |a| a +} diff --git a/tests/target/issue-6116/main4.rs b/tests/target/issue-6116/main4.rs new file mode 100644 index 00000000000..e2eef35ef7b --- /dev/null +++ b/tests/target/issue-6116/main4.rs @@ -0,0 +1,6 @@ +fn foo() -> fn(i32) -> i32 { + |a| { + /*comment before empty statement */ + a + } +} From 4f817e0ab602b7fc81d35c8fc59b12be88a5de50 Mon Sep 17 00:00:00 2001 From: Alexander Glusker Date: Sat, 30 Mar 2024 06:24:44 +0300 Subject: [PATCH 3/3] 2st review on issue-6116 solvation --- src/closures.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/closures.rs b/src/closures.rs index 8c53d9d7579..bdd08bc2d35 100644 --- a/src/closures.rs +++ b/src/closures.rs @@ -1,4 +1,3 @@ -use rustc_ast::ast::StmtKind; use rustc_ast::{ast, ptr}; use rustc_span::Span; use thin_vec::thin_vec; @@ -109,10 +108,7 @@ fn get_inner_expr<'a>( if !needs_block(block, prefix, context) { // block.stmts.len() == 1 except with `|| {{}}`; // https://github.com/rust-lang/rustfmt/issues/3844 - if let Some(expr) = iter_stmts_without_empty(&block.stmts) - .next() - .and_then(stmt_expr) - { + if let Some(expr) = iter_stmts_without_empty(&block).next().and_then(stmt_expr) { return get_inner_expr(expr, prefix, context); } } @@ -121,8 +117,11 @@ fn get_inner_expr<'a>( expr } -fn iter_stmts_without_empty(stmts: &[ast::Stmt]) -> impl Iterator { - stmts.iter().filter(|x| !matches!(x.kind, StmtKind::Empty)) +fn iter_stmts_without_empty(block: &ast::Block) -> impl Iterator { + block + .stmts + .iter() + .filter(|stmt| !matches!(stmt.kind, ast::StmtKind::Empty)) } // Figure out if a block is necessary. @@ -132,7 +131,7 @@ fn needs_block(block: &ast::Block, prefix: &str, context: &RewriteContext<'_>) - }); is_unsafe_block(block) - || iter_stmts_without_empty(&block.stmts).count() > 1 + || iter_stmts_without_empty(&block).count() > 1 || has_attributes || block_contains_comment(context, block) || prefix.contains('\n')