[clang] [clang-format]: Fix formatting of if statements with BlockIndent (PR #77699)
Gedare Bloom via cfe-commits
cfe-commits at lists.llvm.org
Sun Jan 21 17:27:27 PST 2024
https://github.com/gedare updated https://github.com/llvm/llvm-project/pull/77699
>From ff055d9c064d1fb359d59eeb47cb5f8c6c422bec Mon Sep 17 00:00:00 2001
From: Gedare Bloom <gedare at rtems.org>
Date: Fri, 7 Jul 2023 17:28:47 -0600
Subject: [PATCH 1/5] Fix formatting of if statements with BlockIndent
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 is intended.
This patch preserves the AlwaysBreak behavior and supports BlockIndent on if
clauses while fixing the bug.
It may be reasonable to go the other way and create an explicit option for
alignment of if (and loop) clauses intentionally.
Fixes #54663.
Differential Revision: https://reviews.llvm.org/D154755
---
clang/lib/Format/ContinuationIndenter.cpp | 6 ++--
clang/unittests/Format/FormatTest.cpp | 40 ++++++++++++++++++++---
2 files changed, 40 insertions(+), 6 deletions(-)
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index 102504182c4505b..8b288c62473db9f 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -775,8 +775,10 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
Style.Cpp11BracedListStyle)) &&
State.Column > getNewLineColumn(State) &&
(!Previous.Previous ||
- !Previous.Previous->isOneOf(TT_CastRParen, tok::kw_for, tok::kw_while,
- tok::kw_switch)) &&
+ !(Previous.Previous->isOneOf(TT_CastRParen, tok::kw_for, tok::kw_while,
+ tok::kw_switch) ||
+ (Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent &&
+ Previous.Previous->isIf()))) &&
// Don't do this for simple (no expressions) one-argument function calls
// as that feels like needlessly wasting whitespace, e.g.:
//
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 25ef5c680af862b..8d55e62e2558d3d 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -25889,8 +25889,8 @@ TEST_F(FormatTest, AlignAfterOpenBracketBlockIndentIfStatement) {
"}",
Style);
- verifyFormat("if (quitelongarg !=\n"
- " (alsolongarg - 1)) { // ABC is a very longgggggggggggg "
+ verifyFormat("if (quiteLongArg !=\n"
+ " (alsoLongArg - 1)) { // ABC is a very longgggggggggggg "
"comment\n"
" return;\n"
"}",
@@ -25903,12 +25903,44 @@ TEST_F(FormatTest, AlignAfterOpenBracketBlockIndentIfStatement) {
"}",
Style);
- verifyFormat("if (quitelongarg !=\n"
- " (alsolongarg - 1)) { // ABC is a very longgggggggggggg "
+ verifyFormat("if (quiteLongArg !=\n"
+ " (alsoLongArg - 1)) { // ABC is a very longgggggggggggg "
"comment\n"
" return;\n"
"}",
Style);
+
+ verifyFormat("void foo() {\n"
+ " if (camelCaseName < alsoLongName ||\n"
+ " anotherEvenLongerName <=\n"
+ " thisReallyReallyReallyReallyReallyReallyLongerName ||"
+ "\n"
+ " otherName < thisLastName) {\n"
+ " return;\n"
+ " } else if (quiteLongName < alsoLongName ||\n"
+ " anotherEvenLongerName <=\n"
+ " thisReallyReallyReallyReallyReallyReallyLonger"
+ "Name ||\n"
+ " otherName < thisLastName) {\n"
+ " return;\n"
+ " }\n"
+ "}",
+ Style);
+
+ Style.ContinuationIndentWidth = 2;
+ verifyFormat("void foo() {\n"
+ " if (ThisIsRatherALongIfClause && thatIExpectToBeBroken ||\n"
+ " ontoMultipleLines && whenFormattedCorrectly) {\n"
+ " if (false) {\n"
+ " return;\n"
+ " } else if (thisIsRatherALongIfClause && "
+ "thatIExpectToBeBroken ||\n"
+ " ontoMultipleLines && whenFormattedCorrectly) {\n"
+ " return;\n"
+ " }\n"
+ " }\n"
+ "}",
+ Style);
}
TEST_F(FormatTest, AlignAfterOpenBracketBlockIndentForStatement) {
>From 55cfce78dbfc87af455e6bf822b8f0a6a5f9f3d6 Mon Sep 17 00:00:00 2001
From: Gedare Bloom <gedare at rtems.org>
Date: Wed, 10 Jan 2024 14:44:41 -0700
Subject: [PATCH 2/5] Refactor complex conditional logic as a lambda
---
clang/lib/Format/ContinuationIndenter.cpp | 25 +++++++++++++++--------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index 8b288c62473db9f..29426c225fec9ed 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -768,17 +768,24 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
// parenthesis by disallowing any further line breaks if there is no line
// break after the opening parenthesis. Don't break if it doesn't conserve
// columns.
+ auto isOpeningBracket = [&](const FormatToken &Tok) {
+ auto isStartOfBracedList = [&](const FormatToken &Tok) {
+ return Tok.is(tok::l_brace) && Tok.isNot(BK_Block) &&
+ Style.Cpp11BracedListStyle;
+ };
+ if (Tok.isOneOf(tok::l_paren, TT_TemplateOpener, tok::l_square) ||
+ isStartOfBracedList(Tok)) {
+ if (!Tok.Previous)
+ return true;
+ if (Tok.Previous->isIf())
+ return Style.AlignAfterOpenBracket == FormatStyle::BAS_AlwaysBreak;
+ return !Tok.Previous->isOneOf(TT_CastRParen, tok::kw_for, tok::kw_while,
+ tok::kw_switch);
+ }
+ };
if ((Style.AlignAfterOpenBracket == FormatStyle::BAS_AlwaysBreak ||
Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent) &&
- (Previous.isOneOf(tok::l_paren, TT_TemplateOpener, tok::l_square) ||
- (Previous.is(tok::l_brace) && Previous.isNot(BK_Block) &&
- Style.Cpp11BracedListStyle)) &&
- State.Column > getNewLineColumn(State) &&
- (!Previous.Previous ||
- !(Previous.Previous->isOneOf(TT_CastRParen, tok::kw_for, tok::kw_while,
- tok::kw_switch) ||
- (Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent &&
- Previous.Previous->isIf()))) &&
+ isOpeningBracket(Previous) && State.Column > getNewLineColumn(State) &&
// Don't do this for simple (no expressions) one-argument function calls
// as that feels like needlessly wasting whitespace, e.g.:
//
>From a08d90f09861554da844a2e3864a79cc2c4c50c4 Mon Sep 17 00:00:00 2001
From: Gedare Bloom <gedare at rtems.org>
Date: Thu, 11 Jan 2024 07:57:24 -0700
Subject: [PATCH 3/5] add misssing return
---
clang/lib/Format/ContinuationIndenter.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index 29426c225fec9ed..f6f7689bf37a3ec 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -782,6 +782,7 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
return !Tok.Previous->isOneOf(TT_CastRParen, tok::kw_for, tok::kw_while,
tok::kw_switch);
}
+ return false;
};
if ((Style.AlignAfterOpenBracket == FormatStyle::BAS_AlwaysBreak ||
Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent) &&
>From 14f76702e6ad9960e743990f777ba4f0a6771ae4 Mon Sep 17 00:00:00 2001
From: Gedare Bloom <gedare at rtems.org>
Date: Thu, 11 Jan 2024 13:36:07 -0700
Subject: [PATCH 4/5] Fix CamelCase lambda names
---
clang/lib/Format/ContinuationIndenter.cpp | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index f6f7689bf37a3ec..e50093fd9f42b83 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -768,13 +768,13 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
// parenthesis by disallowing any further line breaks if there is no line
// break after the opening parenthesis. Don't break if it doesn't conserve
// columns.
- auto isOpeningBracket = [&](const FormatToken &Tok) {
- auto isStartOfBracedList = [&](const FormatToken &Tok) {
+ const auto IsOpeningBracket = [&](const FormatToken &Tok) {
+ const auto IsStartOfBracedList = [&]() {
return Tok.is(tok::l_brace) && Tok.isNot(BK_Block) &&
Style.Cpp11BracedListStyle;
};
if (Tok.isOneOf(tok::l_paren, TT_TemplateOpener, tok::l_square) ||
- isStartOfBracedList(Tok)) {
+ IsStartOfBracedList()) {
if (!Tok.Previous)
return true;
if (Tok.Previous->isIf())
@@ -786,7 +786,7 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
};
if ((Style.AlignAfterOpenBracket == FormatStyle::BAS_AlwaysBreak ||
Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent) &&
- isOpeningBracket(Previous) && State.Column > getNewLineColumn(State) &&
+ IsOpeningBracket(Previous) && State.Column > getNewLineColumn(State) &&
// Don't do this for simple (no expressions) one-argument function calls
// as that feels like needlessly wasting whitespace, e.g.:
//
>From 23395f6e8c9f08acf946b0db9eed3505bc11cc68 Mon Sep 17 00:00:00 2001
From: Gedare Bloom <gedare at rtems.org>
Date: Sun, 21 Jan 2024 18:27:13 -0700
Subject: [PATCH 5/5] Adopt suggested refactoring from review
---
clang/lib/Format/ContinuationIndenter.cpp | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index e50093fd9f42b83..fbab5d042041e82 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -768,21 +768,21 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
// parenthesis by disallowing any further line breaks if there is no line
// break after the opening parenthesis. Don't break if it doesn't conserve
// columns.
- const auto IsOpeningBracket = [&](const FormatToken &Tok) {
- const auto IsStartOfBracedList = [&]() {
+ auto IsOpeningBracket = [&](const FormatToken &Tok) {
+ auto IsStartOfBracedList = [&]() {
return Tok.is(tok::l_brace) && Tok.isNot(BK_Block) &&
Style.Cpp11BracedListStyle;
};
- if (Tok.isOneOf(tok::l_paren, TT_TemplateOpener, tok::l_square) ||
- IsStartOfBracedList()) {
- if (!Tok.Previous)
- return true;
- if (Tok.Previous->isIf())
- return Style.AlignAfterOpenBracket == FormatStyle::BAS_AlwaysBreak;
- return !Tok.Previous->isOneOf(TT_CastRParen, tok::kw_for, tok::kw_while,
- tok::kw_switch);
+ if (!Tok.isOneOf(tok::l_paren, TT_TemplateOpener, tok::l_square) &&
+ !IsStartOfBracedList()) {
+ return false;
}
- return false;
+ if (!Tok.Previous)
+ return true;
+ if (Tok.Previous->isIf())
+ return Style.AlignAfterOpenBracket == FormatStyle::BAS_AlwaysBreak;
+ return !Tok.Previous->isOneOf(TT_CastRParen, tok::kw_for, tok::kw_while,
+ tok::kw_switch);
};
if ((Style.AlignAfterOpenBracket == FormatStyle::BAS_AlwaysBreak ||
Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent) &&
More information about the cfe-commits
mailing list