MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: curdeius, HazardyKnusperkeks, Wawha, krasimir.
MyDeveloperDay added projects: clang, clang-format.
MyDeveloperDay requested review of this revision.
https://bugs.llvm.org/show_bug.cgi?id=50702
I believe D44609: [clang-format] New option BeforeLambdaBody to manage lambda
line break inside function parameter call (in Allman style)
<https://reviews.llvm.org/D44609> may be too aggressive with brace wrapping
rules which doesn't always apply to Lamdbas
The introduction of BeforeLambdaBody and AllowShortLambdasOnASingleLine has
impact on brace handling on other block types, which I suspect we didn't see
before as people may not be using the BeforeLambdaBody style
>From what I can tell this can be seen by the unit test I change as its not
>honouring the orginal LLVM brace wrapping style for the `Fct()` function
I added a unit test from PR50702 and have removed some of the code (which has
zero impact on the unit test, which kind of suggests its unnecessary), some
additional attempt has been made to try and ensure we'll only break on what is
actually a LamdbaLBrace
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D104222
Files:
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/FormatTest.cpp
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -18856,8 +18856,7 @@
" });\n"
" });",
LLVMWithBeforeLambdaBody);
- verifyFormat("void Fct()\n"
- "{\n"
+ verifyFormat("void Fct() {\n"
" return {[]()\n"
" {\n"
" return 17;\n"
@@ -19061,6 +19060,16 @@
" });\n"
" });",
LLVMWithBeforeLambdaBody);
+
+ LLVMWithBeforeLambdaBody.AllowShortLambdasOnASingleLine =
+ FormatStyle::ShortLambdaStyle::SLS_None;
+ verifyFormat("namespace test {\n"
+ "class Test {\n"
+ "public:\n"
+ " Test() = default;\n"
+ "};\n"
+ "} // namespace test",
+ LLVMWithBeforeLambdaBody);
}
TEST_F(FormatTest, LambdaWithLineComments) {
Index: clang/lib/Format/TokenAnnotator.cpp
===================================================================
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3754,13 +3754,6 @@
if (Right.is(TT_InlineASMBrace))
return Right.HasUnescapedNewline;
- auto ShortLambdaOption = Style.AllowShortLambdasOnASingleLine;
- if (Style.BraceWrapping.BeforeLambdaBody &&
- (isAllmanBraceIncludedBreakableLambda(Left, ShortLambdaOption) ||
- isAllmanBraceIncludedBreakableLambda(Right, ShortLambdaOption))) {
- return true;
- }
-
if (isAllmanBrace(Left) || isAllmanBrace(Right))
return (Line.startsWith(tok::kw_enum) && Style.BraceWrapping.AfterEnum) ||
(Line.startsWith(tok::kw_typedef, tok::kw_enum) &&
@@ -4186,7 +4179,7 @@
return false;
auto ShortLambdaOption = Style.AllowShortLambdasOnASingleLine;
- if (Style.BraceWrapping.BeforeLambdaBody) {
+ if (Style.BraceWrapping.BeforeLambdaBody && Right.is(TT_LambdaLBrace)) {
if (isAllmanLambdaBrace(Left))
return !isItAnEmptyLambdaAllowed(Left, ShortLambdaOption);
if (isAllmanLambdaBrace(Right))
@@ -4198,7 +4191,6 @@
Right.isMemberAccess() ||
Right.isOneOf(TT_TrailingReturnArrow, TT_LambdaArrow, tok::lessless,
tok::colon, tok::l_square, tok::at) ||
- (Style.BraceWrapping.BeforeLambdaBody && Right.is(TT_LambdaLBrace)) ||
(Left.is(tok::r_paren) &&
Right.isOneOf(tok::identifier, tok::kw_const)) ||
(Left.is(tok::l_paren) && !Right.is(tok::r_paren)) ||
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -18856,8 +18856,7 @@
" });\n"
" });",
LLVMWithBeforeLambdaBody);
- verifyFormat("void Fct()\n"
- "{\n"
+ verifyFormat("void Fct() {\n"
" return {[]()\n"
" {\n"
" return 17;\n"
@@ -19061,6 +19060,16 @@
" });\n"
" });",
LLVMWithBeforeLambdaBody);
+
+ LLVMWithBeforeLambdaBody.AllowShortLambdasOnASingleLine =
+ FormatStyle::ShortLambdaStyle::SLS_None;
+ verifyFormat("namespace test {\n"
+ "class Test {\n"
+ "public:\n"
+ " Test() = default;\n"
+ "};\n"
+ "} // namespace test",
+ LLVMWithBeforeLambdaBody);
}
TEST_F(FormatTest, LambdaWithLineComments) {
Index: clang/lib/Format/TokenAnnotator.cpp
===================================================================
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3754,13 +3754,6 @@
if (Right.is(TT_InlineASMBrace))
return Right.HasUnescapedNewline;
- auto ShortLambdaOption = Style.AllowShortLambdasOnASingleLine;
- if (Style.BraceWrapping.BeforeLambdaBody &&
- (isAllmanBraceIncludedBreakableLambda(Left, ShortLambdaOption) ||
- isAllmanBraceIncludedBreakableLambda(Right, ShortLambdaOption))) {
- return true;
- }
-
if (isAllmanBrace(Left) || isAllmanBrace(Right))
return (Line.startsWith(tok::kw_enum) && Style.BraceWrapping.AfterEnum) ||
(Line.startsWith(tok::kw_typedef, tok::kw_enum) &&
@@ -4186,7 +4179,7 @@
return false;
auto ShortLambdaOption = Style.AllowShortLambdasOnASingleLine;
- if (Style.BraceWrapping.BeforeLambdaBody) {
+ if (Style.BraceWrapping.BeforeLambdaBody && Right.is(TT_LambdaLBrace)) {
if (isAllmanLambdaBrace(Left))
return !isItAnEmptyLambdaAllowed(Left, ShortLambdaOption);
if (isAllmanLambdaBrace(Right))
@@ -4198,7 +4191,6 @@
Right.isMemberAccess() ||
Right.isOneOf(TT_TrailingReturnArrow, TT_LambdaArrow, tok::lessless,
tok::colon, tok::l_square, tok::at) ||
- (Style.BraceWrapping.BeforeLambdaBody && Right.is(TT_LambdaLBrace)) ||
(Left.is(tok::r_paren) &&
Right.isOneOf(tok::identifier, tok::kw_const)) ||
(Left.is(tok::l_paren) && !Right.is(tok::r_paren)) ||
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
