Skip to content

Commit

Permalink
[clang-format] Handle "complex" conditionals in RemoveBracesLLVM
Browse files Browse the repository at this point in the history 
Do not remove braces if the conditional of if/for/while might not
fit on a single line even after the opening brace is removed.

Examples:
// ColumnLimit: 20
// 45678901234567890
if (a) { /* Remove. */
  foo();
}
if (-b >= c) { // Keep.
  bar();
}

Differential Revision: https://reviews.llvm.org/D126052
  • Loading branch information
@owenca
owenca committed May 21, 2022
1 parent 7be783a commit 1443dba
Show file tree
Hide file tree
Showing 3 changed files with 195 additions and 11 deletions.
37 changes: 29 additions & 8 deletions clang/lib/Format/UnwrappedLineParser.cpp
View file
Expand Up @@ -754,16 +754,19 @@ size_t UnwrappedLineParser::computePPHash() const {
return h;
}

// Checks whether \p ParsedLine might fit on a single line. We must clone the
// tokens of \p ParsedLine before running the token annotator on it so that we
// can restore them afterward.
bool UnwrappedLineParser::mightFitOnOneLine(UnwrappedLine &ParsedLine) const {
// Checks whether \p ParsedLine might fit on a single line. If \p OpeningBrace
// is not null, subtracts its length (plus the preceding space) when computing
// the length of \p ParsedLine. We must clone the tokens of \p ParsedLine before
// running the token annotator on it so that we can restore them afterward.
bool UnwrappedLineParser::mightFitOnOneLine(
UnwrappedLine &ParsedLine, const FormatToken *OpeningBrace) const {
const auto ColumnLimit = Style.ColumnLimit;
if (ColumnLimit == 0)
return true;

auto &Tokens = ParsedLine.Tokens;
assert(!Tokens.empty());

const auto *LastToken = Tokens.back().Tok;
assert(LastToken);

Expand All @@ -785,7 +788,11 @@ bool UnwrappedLineParser::mightFitOnOneLine(UnwrappedLine &ParsedLine) const {
Annotator.annotate(Line);
Annotator.calculateFormattingInformation(Line);

const int Length = LastToken->TotalLength;
auto Length = LastToken->TotalLength;
if (OpeningBrace) {
assert(OpeningBrace != Tokens.front().Tok);
Length -= OpeningBrace->TokenText.size() + 1;
}

Index = 0;
for (auto &Token : Tokens) {
Expand All @@ -805,6 +812,8 @@ UnwrappedLineParser::IfStmtKind UnwrappedLineParser::parseBlock(
assert(FormatTok->isOneOf(tok::l_brace, TT_MacroBlockBegin) &&
"'{' or macro block token expected");
FormatToken *Tok = FormatTok;
const bool FollowedByComment = Tokens->peekNextToken()->is(tok::comment);
auto Index = CurrentLines->size();
const bool MacroBlock = FormatTok->is(TT_MacroBlockBegin);
FormatTok->setBlockKind(BK_Block);

Expand Down Expand Up @@ -854,14 +863,26 @@ UnwrappedLineParser::IfStmtKind UnwrappedLineParser::parseBlock(
return IfKind;
}

if (SimpleBlock && !KeepBraces &&
Tok->isOneOf(TT_ControlStatementLBrace, TT_ElseLBrace)) {
if (SimpleBlock && !KeepBraces) {
assert(Tok->isOneOf(TT_ControlStatementLBrace, TT_ElseLBrace));
assert(FormatTok->is(tok::r_brace));
const FormatToken *Previous = Tokens->getPreviousToken();
assert(Previous);
if (Previous->isNot(tok::r_brace) || Previous->Optional) {
assert(!CurrentLines->empty());
if (mightFitOnOneLine(CurrentLines->back())) {
const FormatToken *OpeningBrace = Tok;
if (!Tok->Previous) { // Wrapped l_brace.
if (FollowedByComment) {
KeepBraces = true;
} else {
assert(Index > 0);
--Index; // The line above the wrapped l_brace.
OpeningBrace = nullptr;
}
}
if (!KeepBraces && mightFitOnOneLine(CurrentLines->back()) &&
(Tok->is(TT_ElseLBrace) ||
mightFitOnOneLine((*CurrentLines)[Index], OpeningBrace))) {
Tok->MatchingParen = FormatTok;
FormatTok->MatchingParen = Tok;
}
Expand Down
3 changes: 2 additions & 1 deletion clang/lib/Format/UnwrappedLineParser.h
View file
Expand Up @@ -95,7 +95,8 @@ class UnwrappedLineParser {
bool parseLevel(const FormatToken *OpeningBrace, bool CanContainBracedList,
IfStmtKind *IfKind = nullptr,
TokenType NextLBracesType = TT_Unknown);
bool mightFitOnOneLine(UnwrappedLine &Line) const;
bool mightFitOnOneLine(UnwrappedLine &Line,
const FormatToken *OpeningBrace = nullptr) const;
IfStmtKind parseBlock(bool MustBeDeclaration = false, unsigned AddLevels = 1u,
bool MunchSemi = true, bool KeepBraces = true,
bool UnindentWhitesmithsBraces = false,
Expand Down
166 changes: 164 additions & 2 deletions clang/unittests/Format/FormatTest.cpp
View file
Expand Up @@ -25453,7 +25453,6 @@ TEST_F(FormatTest, RemoveBraces) {
Style);

Style.ColumnLimit = 65;

verifyFormat("if (condition) {\n"
" ff(Indices,\n"
" [&](unsigned LHSI, unsigned RHSI) { return true; });\n"
Expand Down Expand Up @@ -25496,8 +25495,48 @@ TEST_F(FormatTest, RemoveBraces) {
"}",
Style);

Style.ColumnLimit = 0;
verifyFormat("if (-b >=\n"
" c) { // Keep.\n"
" foo();\n"
"} else {\n"
" bar();\n"
"}",
"if (-b >= c) { // Keep.\n"
" foo();\n"
"} else {\n"
" bar();\n"
"}",
Style);

verifyFormat("if (a) /* Remove. */\n"
" f();\n"
"else\n"
" g();",
"if (a) <% /* Remove. */\n"
" f();\n"
"%> else <%\n"
" g();\n"
"%>",
Style);

verifyFormat("while (\n"
" !i--) <% // Keep.\n"
" foo();\n"
"%>",
"while (!i--) <% // Keep.\n"
" foo();\n"
"%>",
Style);

verifyFormat("for (int &i : chars)\n"
" ++i;",
"for (int &i :\n"
" chars) {\n"
" ++i;\n"
"}",
Style);

Style.ColumnLimit = 0;
verifyFormat("if (a)\n"
" b234567890223456789032345678904234567890 = "
"c234567890223456789032345678904234567890;",
Expand All @@ -25506,6 +25545,129 @@ TEST_F(FormatTest, RemoveBraces) {
"c234567890223456789032345678904234567890;\n"
"}",
Style);

Style.BreakBeforeBraces = FormatStyle::BS_Custom;
Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
Style.BraceWrapping.BeforeElse = true;

Style.ColumnLimit = 65;

verifyFormat("if (condition)\n"
"{\n"
" ff(Indices,\n"
" [&](unsigned LHSI, unsigned RHSI) { return true; });\n"
"}\n"
"else\n"
"{\n"
" ff(Indices,\n"
" [&](unsigned LHSI, unsigned RHSI) { return true; });\n"
"}",
"if (condition) {\n"
" ff(Indices,\n"
" [&](unsigned LHSI, unsigned RHSI) { return true; });\n"
"} else {\n"
" ff(Indices,\n"
" [&](unsigned LHSI, unsigned RHSI) { return true; });\n"
"}",
Style);

verifyFormat("if (a)\n"
"{ //\n"
" foo();\n"
"}",
"if (a) { //\n"
" foo();\n"
"}",
Style);

Style.ColumnLimit = 20;

verifyFormat("int ab = [](int i) {\n"
" if (i > 0)\n"
" {\n"
" i = 12345678 -\n"
" i;\n"
" }\n"
" return i;\n"
"};",
"int ab = [](int i) {\n"
" if (i > 0) {\n"
" i = 12345678 -\n"
" i;\n"
" }\n"
" return i;\n"
"};",
Style);

verifyFormat("if (a)\n"
"{\n"
" b = c + // 1 -\n"
" d;\n"
"}",
"if (a) {\n"
" b = c + // 1 -\n"
" d;\n"
"}",
Style);

verifyFormat("if (a)\n"
"{\n"
" b = c >= 0 ? d\n"
" : e;\n"
"}",
"if (a) {\n"
" b = c >= 0 ? d : e;\n"
"}",
Style);

verifyFormat("if (a)\n"
" b = c > 0 ? d : e;",
"if (a)\n"
"{\n"
" b = c > 0 ? d : e;\n"
"}",
Style);

verifyFormat("if (foo + bar <=\n"
" baz)\n"
"{\n"
" func(arg1, arg2);\n"
"}",
"if (foo + bar <= baz) {\n"
" func(arg1, arg2);\n"
"}",
Style);

verifyFormat("if (foo + bar < baz)\n"
" func(arg1, arg2);\n"
"else\n"
" func();",
"if (foo + bar < baz)\n"
"<%\n"
" func(arg1, arg2);\n"
"%>\n"
"else\n"
"<%\n"
" func();\n"
"%>",
Style);

verifyFormat("while (i--)\n"
"<% // Keep.\n"
" foo();\n"
"%>",
"while (i--) <% // Keep.\n"
" foo();\n"
"%>",
Style);

verifyFormat("for (int &i : chars)\n"
" ++i;",
"for (int &i : chars)\n"
"{\n"
" ++i;\n"
"}",
Style);
}

TEST_F(FormatTest, AlignAfterOpenBracketBlockIndent) {
Expand Down

0 comments on commit 1443dba

Please sign in to comment.