[PATCH] D115738: [clang-format] Code following C# Lambda Expressions has wrong formatting
owenpan added inline comments.
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2007
+if (FormatTok->is(tok::l_brace)) {
+ if (Style.isCSharp() && Style.BraceWrapping.AfterFunction == true) {
+FormatTok->MustBreakBefore = true;
owenpan wrote:
> You already tested for C# on line 2004.
Actually, `if (Style.BraceWrapping.AfterFunction)` will do, i.e., `== true` is
redundant.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D115738/new/
https://reviews.llvm.org/D115738
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D115738: [clang-format] Code following C# Lambda Expressions has wrong formatting
jbcoe added inline comments.
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2007
+if (FormatTok->is(tok::l_brace)) {
+ if (Style.isCSharp() && Style.BraceWrapping.AfterFunction == true) {
+FormatTok->MustBreakBefore = true;
I think this check for `Style.isCSharp()` is redundant as it's already checked
above.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D115738/new/
https://reviews.llvm.org/D115738
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D115738: [clang-format] Code following C# Lambda Expressions has wrong formatting
owenpan added inline comments.
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2007
+if (FormatTok->is(tok::l_brace)) {
+ if (Style.isCSharp() && Style.BraceWrapping.AfterFunction == true) {
+FormatTok->MustBreakBefore = true;
You already tested for C# on line 2004.
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2009
+FormatTok->MustBreakBefore = true;
+ }
+ parseChildBlock();
Remove braces.
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2012
+}
+ } else
nextToken();
Add braces after `else` to match `if`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D115738/new/
https://reviews.llvm.org/D115738
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D115738: [clang-format] Code following C# Lambda Expressions has wrong formatting
MyDeveloperDay added a comment. I tested this on the original code that made me make the original change, and I like your fix much better ;-) Thank you for this patch, interested on working on other C# clang-format issues? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115738/new/ https://reviews.llvm.org/D115738 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D115738: [clang-format] Code following C# Lambda Expressions has wrong formatting
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115738/new/ https://reviews.llvm.org/D115738 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D115738: [clang-format] Code following C# Lambda Expressions has wrong formatting
peterstys updated this revision to Diff 394299.
peterstys added a comment.
Applied clang-formatting.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D115738/new/
https://reviews.llvm.org/D115738
Files:
clang/lib/Format/UnwrappedLineParser.cpp
clang/unittests/Format/FormatTestCSharp.cpp
Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -759,6 +759,128 @@
GoogleStyle);
}
+TEST_F(FormatTestCSharp, CSharpLambdasDontBreakFollowingCodeAlignment) {
+ FormatStyle GoogleStyle = getGoogleStyle(FormatStyle::LK_CSharp);
+ FormatStyle MicrosoftStyle = getMicrosoftStyle(FormatStyle::LK_CSharp);
+
+ verifyFormat(R"(//
+public class Sample {
+ public void Test() {
+while (true) {
+ preBindEnumerators.RemoveAll(enumerator => !enumerator.MoveNext());
+ CodeThatFollowsLambda();
+ IsWellAligned();
+}
+ }
+})",
+ GoogleStyle);
+
+ verifyFormat(R"(//
+public class Sample
+{
+public void Test()
+{
+while (true)
+{
+preBindEnumerators.RemoveAll(enumerator => !enumerator.MoveNext());
+CodeThatFollowsLambda();
+IsWellAligned();
+}
+}
+})",
+ MicrosoftStyle);
+}
+
+TEST_F(FormatTestCSharp, CSharpLambdasComplexLambdasDontBreakAlignment) {
+ FormatStyle GoogleStyle = getGoogleStyle(FormatStyle::LK_CSharp);
+ FormatStyle MicrosoftStyle = getMicrosoftStyle(FormatStyle::LK_CSharp);
+
+ verifyFormat(R"(//
+public class Test
+{
+private static void ComplexLambda(BuildReport protoReport)
+{
+allSelectedScenes =
+veryVeryLongCollectionNameThatPutsTheLineLenghtAboveTheThresholds.Where(scene => scene.enabled)
+.Select(scene => scene.path)
+.ToArray();
+if (allSelectedScenes.Count == 0)
+{
+return;
+}
+Functions();
+AreWell();
+Aligned();
+AfterLambdaBlock();
+}
+})",
+ MicrosoftStyle);
+
+ verifyFormat(R"(//
+public class Test {
+ private static void ComplexLambda(BuildReport protoReport) {
+allSelectedScenes = veryVeryLongCollectionNameThatPutsTheLineLenghtAboveTheThresholds
+.Where(scene => scene.enabled)
+.Select(scene => scene.path)
+.ToArray();
+if (allSelectedScenes.Count == 0) {
+ return;
+}
+Functions();
+AreWell();
+Aligned();
+AfterLambdaBlock();
+ }
+})",
+ GoogleStyle);
+}
+
+TEST_F(FormatTestCSharp, CSharpLambdasMulipleLambdasDontBreakAlignment) {
+ FormatStyle GoogleStyle = getGoogleStyle(FormatStyle::LK_CSharp);
+ FormatStyle MicrosoftStyle = getMicrosoftStyle(FormatStyle::LK_CSharp);
+
+ verifyFormat(R"(//
+public class Test
+{
+private static void MultipleLambdas(BuildReport protoReport)
+{
+allSelectedScenes =
+veryVeryLongCollectionNameThatPutsTheLineLenghtAboveTheThresholds.Where(scene => scene.enabled)
+.Select(scene => scene.path)
+.ToArray();
+preBindEnumerators.RemoveAll(enumerator => !enumerator.MoveNext());
+if (allSelectedScenes.Count == 0)
+{
+return;
+}
+Functions();
+AreWell();
+Aligned();
+AfterLambdaBlock();
+}
+})",
+ MicrosoftStyle);
+
+ verifyFormat(R"(//
+public class Test {
+ private static void MultipleLambdas(BuildReport protoReport) {
+allSelectedScenes = veryVeryLongCollectionNameThatPutsTheLineLenghtAboveTheThresholds
+.Where(scene => scene.enabled)
+.Select(scene => scene.path)
+.ToArray();
+preBindEnumerators.RemoveAll(enumerator => !enumerator.MoveNext());
+if (allSelectedScenes.Count == 0) {
+ return;
+}
+Functions();
+AreWell();
+Aligned();
+AfterLambdaBlock();
+ }
+})",
+ GoogleStyle);
+}
+
TEST_F(FormatTestCSharp, CSharpObjectInitializers) {
FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2001,9 +2001,15 @@
}
break;
case tok::equal:
- if (Style.isCSharp() && FormatTok->is(TT_FatArrow))
-parseStructuralElement();
- else
+ if (Style.isCSharp() && FormatTok->is(TT_FatArrow)) {
+nextToken();
+if (FormatTok->is(tok::l_brace)) {
+ if (Style.isCSharp() && Style.BraceWrapping.AfterFunction == true) {
+FormatTok->MustBreakBefore = true;
+ }
+ parseChildBlock();
+}
+ } else
[PATCH] D115738: [clang-format] Code following C# Lambda Expressions has wrong formatting
peterstys created this revision.
peterstys added a reviewer: MyDeveloperDay.
peterstys requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
The alignment fix introduced by https://reviews.llvm.org/D104388 caused a
regression whereby formatting of code that follows the lambda block is
incorrect i.e. separate expressions are put on the same line.
For example:
public void Test() {
while (true) {
preBindEnumerators.RemoveAll(enumerator => !enumerator.MoveNext());
CodeThatFollowsLambda();
IsWellAligned();
}
will be formatted as:
public void Test() {
while (true) {
preBindEnumerators.RemoveAll(enumerator => !enumerator.MoveNext());
CodeThatFollowsLambda(); IsWellAligned();
}
The problem is that the "Fat arrow" token parsing inside the parseParens()
function uses parseStructuralElement() which does not like to be called inside
the parenthesis. It seems that code that follows is considered part of the
parenthesis block.
As a fix, parseStructuralElement parser inside the parseParens() was replaced
with parseChildBlock() if the following token was a "left brace" and
nextToken() otherwise. Added few new unit tests to demonstrate the regressions.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D115738
Files:
clang/lib/Format/UnwrappedLineParser.cpp
clang/unittests/Format/FormatTestCSharp.cpp
Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -759,6 +759,122 @@
GoogleStyle);
}
+TEST_F(FormatTestCSharp, CSharpLambdasDontBreakFollowingCodeAlignment) {
+ FormatStyle GoogleStyle = getGoogleStyle(FormatStyle::LK_CSharp);
+ FormatStyle MicrosoftStyle = getMicrosoftStyle(FormatStyle::LK_CSharp);
+
+ verifyFormat(R"(//
+public class Sample {
+ public void Test() {
+while (true) {
+ preBindEnumerators.RemoveAll(enumerator => !enumerator.MoveNext());
+ CodeThatFollowsLambda();
+ IsWellAligned();
+}
+ }
+})", GoogleStyle);
+
+ verifyFormat(R"(//
+public class Sample
+{
+public void Test()
+{
+while (true)
+{
+preBindEnumerators.RemoveAll(enumerator => !enumerator.MoveNext());
+CodeThatFollowsLambda();
+IsWellAligned();
+}
+}
+})", MicrosoftStyle);
+}
+
+TEST_F(FormatTestCSharp, CSharpLambdasComplexLambdasDontBreakAlignment) {
+ FormatStyle GoogleStyle = getGoogleStyle(FormatStyle::LK_CSharp);
+ FormatStyle MicrosoftStyle = getMicrosoftStyle(FormatStyle::LK_CSharp);
+
+ verifyFormat(R"(//
+public class Test
+{
+private static void ComplexLambda(BuildReport protoReport)
+{
+allSelectedScenes =
+veryVeryLongCollectionNameThatPutsTheLineLenghtAboveTheThresholds.Where(scene => scene.enabled)
+.Select(scene => scene.path)
+.ToArray();
+if (allSelectedScenes.Count == 0)
+{
+return;
+}
+Functions();
+AreWell();
+Aligned();
+AfterLambdaBlock();
+}
+})", MicrosoftStyle);
+
+ verifyFormat(R"(//
+public class Test {
+ private static void ComplexLambda(BuildReport protoReport) {
+allSelectedScenes = veryVeryLongCollectionNameThatPutsTheLineLenghtAboveTheThresholds
+.Where(scene => scene.enabled)
+.Select(scene => scene.path)
+.ToArray();
+if (allSelectedScenes.Count == 0) {
+ return;
+}
+Functions();
+AreWell();
+Aligned();
+AfterLambdaBlock();
+ }
+})", GoogleStyle);
+}
+
+TEST_F(FormatTestCSharp, CSharpLambdasMulipleLambdasDontBreakAlignment) {
+ FormatStyle GoogleStyle = getGoogleStyle(FormatStyle::LK_CSharp);
+ FormatStyle MicrosoftStyle = getMicrosoftStyle(FormatStyle::LK_CSharp);
+
+ verifyFormat(R"(//
+public class Test
+{
+private static void MultipleLambdas(BuildReport protoReport)
+{
+allSelectedScenes =
+veryVeryLongCollectionNameThatPutsTheLineLenghtAboveTheThresholds.Where(scene => scene.enabled)
+.Select(scene => scene.path)
+.ToArray();
+preBindEnumerators.RemoveAll(enumerator => !enumerator.MoveNext());
+if (allSelectedScenes.Count == 0)
+{
+return;
+}
+Functions();
+AreWell();
+Aligned();
+AfterLambdaBlock();
+}
+})", MicrosoftStyle);
+
+ verifyFormat(R"(//
+public class Test {
+ private static void MultipleLambdas(BuildReport protoReport) {
+allSelectedScenes = veryVeryLongCollectionNameThatPutsTheLineLenghtAboveTheThresholds
+.Where(scene => scene.enabled)
+.Select(scene => scene.path)
+.ToArray();
+
