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(); + 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,8 +2001,15 @@ } break; case tok::equal: - if (Style.isCSharp() && FormatTok->is(TT_FatArrow)) - parseStructuralElement(); + 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 nextToken(); break;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits