Skip to content

Commit 4ef646e

Browse files
authored
[clang-format]: Fix formatting of if statements with BlockIndent (#77699)
A bug with BlockIndent prevents line breaks within if (and else if) clauses. While fixing this bug, it appears that AlignAfterOpenBracket is not designed to work with loop and if statements, but AlwaysBreak works on if clauses. The documentation and tests are not clear on whether or not this behavior is intended. This PR preserves the `AlwaysBreak` behavior on `if` clauses without supporting `BlockIndent` on `if` clauses to avoid regressions while fixing the bug. It may be reasonable to create an explicit option for alignment of if (and loop) clauses intentionally for both `AlwaysBreak` and `BlockIndent` Fixes #54663. Migrated from Differential Revision: https://reviews.llvm.org/D154755 See more discussion there. Addressed last open comment from the rev about refactoring the complex conditional logic involved with the `AlignAfterOpenBracket` line break behavior.
1 parent 7e63940 commit 4ef646e

File tree

2 files changed

+53
-11
lines changed

2 files changed

+53
-11
lines changed

clang/lib/Format/ContinuationIndenter.cpp

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -771,15 +771,25 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
771771
// parenthesis by disallowing any further line breaks if there is no line
772772
// break after the opening parenthesis. Don't break if it doesn't conserve
773773
// columns.
774+
auto IsOpeningBracket = [&](const FormatToken &Tok) {
775+
auto IsStartOfBracedList = [&]() {
776+
return Tok.is(tok::l_brace) && Tok.isNot(BK_Block) &&
777+
Style.Cpp11BracedListStyle;
778+
};
779+
if (!Tok.isOneOf(tok::l_paren, TT_TemplateOpener, tok::l_square) &&
780+
!IsStartOfBracedList()) {
781+
return false;
782+
}
783+
if (!Tok.Previous)
784+
return true;
785+
if (Tok.Previous->isIf())
786+
return Style.AlignAfterOpenBracket == FormatStyle::BAS_AlwaysBreak;
787+
return !Tok.Previous->isOneOf(TT_CastRParen, tok::kw_for, tok::kw_while,
788+
tok::kw_switch);
789+
};
774790
if ((Style.AlignAfterOpenBracket == FormatStyle::BAS_AlwaysBreak ||
775791
Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent) &&
776-
(Previous.isOneOf(tok::l_paren, TT_TemplateOpener, tok::l_square) ||
777-
(Previous.is(tok::l_brace) && Previous.isNot(BK_Block) &&
778-
Style.Cpp11BracedListStyle)) &&
779-
State.Column > getNewLineColumn(State) &&
780-
(!Previous.Previous ||
781-
!Previous.Previous->isOneOf(TT_CastRParen, tok::kw_for, tok::kw_while,
782-
tok::kw_switch)) &&
792+
IsOpeningBracket(Previous) && State.Column > getNewLineColumn(State) &&
783793
// Don't do this for simple (no expressions) one-argument function calls
784794
// as that feels like needlessly wasting whitespace, e.g.:
785795
//

clang/unittests/Format/FormatTest.cpp

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26172,8 +26172,8 @@ TEST_F(FormatTest, AlignAfterOpenBracketBlockIndentIfStatement) {
2617226172
"}",
2617326173
Style);
2617426174

26175-
verifyFormat("if (quitelongarg !=\n"
26176-
" (alsolongarg - 1)) { // ABC is a very longgggggggggggg "
26175+
verifyFormat("if (quiteLongArg !=\n"
26176+
" (alsoLongArg - 1)) { // ABC is a very longgggggggggggg "
2617726177
"comment\n"
2617826178
" return;\n"
2617926179
"}",
@@ -26186,12 +26186,44 @@ TEST_F(FormatTest, AlignAfterOpenBracketBlockIndentIfStatement) {
2618626186
"}",
2618726187
Style);
2618826188

26189-
verifyFormat("if (quitelongarg !=\n"
26190-
" (alsolongarg - 1)) { // ABC is a very longgggggggggggg "
26189+
verifyFormat("if (quiteLongArg !=\n"
26190+
" (alsoLongArg - 1)) { // ABC is a very longgggggggggggg "
2619126191
"comment\n"
2619226192
" return;\n"
2619326193
"}",
2619426194
Style);
26195+
26196+
verifyFormat("void foo() {\n"
26197+
" if (camelCaseName < alsoLongName ||\n"
26198+
" anotherEvenLongerName <=\n"
26199+
" thisReallyReallyReallyReallyReallyReallyLongerName ||"
26200+
"\n"
26201+
" otherName < thisLastName) {\n"
26202+
" return;\n"
26203+
" } else if (quiteLongName < alsoLongName ||\n"
26204+
" anotherEvenLongerName <=\n"
26205+
" thisReallyReallyReallyReallyReallyReallyLonger"
26206+
"Name ||\n"
26207+
" otherName < thisLastName) {\n"
26208+
" return;\n"
26209+
" }\n"
26210+
"}",
26211+
Style);
26212+
26213+
Style.ContinuationIndentWidth = 2;
26214+
verifyFormat("void foo() {\n"
26215+
" if (ThisIsRatherALongIfClause && thatIExpectToBeBroken ||\n"
26216+
" ontoMultipleLines && whenFormattedCorrectly) {\n"
26217+
" if (false) {\n"
26218+
" return;\n"
26219+
" } else if (thisIsRatherALongIfClause && "
26220+
"thatIExpectToBeBroken ||\n"
26221+
" ontoMultipleLines && whenFormattedCorrectly) {\n"
26222+
" return;\n"
26223+
" }\n"
26224+
" }\n"
26225+
"}",
26226+
Style);
2619526227
}
2619626228

2619726229
TEST_F(FormatTest, AlignAfterOpenBracketBlockIndentForStatement) {

0 commit comments

Comments
 (0)