owenpan updated this revision to Diff 398231.
owenpan added a comment.
- Use `verifyFormat` instead of `EXPECT_EQ`.
- Make this option have less impact on the unwrapped line parser by checking
`Style.RemoveBracesLLVM`.
- Remove some unused code.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D116316/new/
https://reviews.llvm.org/D116316
Files:
clang/docs/ClangFormatStyleOptions.rst
clang/include/clang/Format/Format.h
clang/lib/Format/Format.cpp
clang/lib/Format/FormatToken.h
clang/lib/Format/TokenAnnotator.cpp
clang/lib/Format/UnwrappedLineParser.cpp
clang/lib/Format/UnwrappedLineParser.h
clang/unittests/Format/FormatTest.cpp
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -18682,6 +18682,7 @@
CHECK_PARSE_BOOL(ObjCSpaceBeforeProtocolList);
CHECK_PARSE_BOOL(Cpp11BracedListStyle);
CHECK_PARSE_BOOL(ReflowComments);
+ CHECK_PARSE_BOOL(RemoveBracesLLVM);
CHECK_PARSE_BOOL(SortUsingDeclarations);
CHECK_PARSE_BOOL(SpacesInParentheses);
CHECK_PARSE_BOOL(SpacesInSquareBrackets);
@@ -23020,6 +23021,319 @@
Style);
}
+TEST_F(FormatTest, RemoveBraces) {
+ FormatStyle Style = getLLVMStyle();
+ Style.RemoveBracesLLVM = true;
+
+ // The following eight test cases are fully-braced versions of the examples at
+ // "llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-
+ // statement-bodies-of-if-else-loop-statements".
+
+ // 1. Omit the braces, since the body is simple and clearly associated with
+ // the if.
+ verifyFormat("if (isa<FunctionDecl>(D))\n"
+ " handleFunctionDecl(D);\n"
+ "else if (isa<VarDecl>(D))\n"
+ " handleVarDecl(D);",
+ "if (isa<FunctionDecl>(D)) {\n"
+ " handleFunctionDecl(D);\n"
+ "} else if (isa<VarDecl>(D)) {\n"
+ " handleVarDecl(D);\n"
+ "}",
+ Style);
+
+ // 2. Here we document the condition itself and not the body.
+ verifyFormat("if (isa<VarDecl>(D)) {\n"
+ " // It is necessary that we explain the situation with this\n"
+ " // surprisingly long comment, so it would be unclear\n"
+ " // without the braces whether the following statement is in\n"
+ " // the scope of the `if`.\n"
+ " // Because the condition is documented, we can't really\n"
+ " // hoist this comment that applies to the body above the\n"
+ " // if.\n"
+ " handleOtherDecl(D);\n"
+ "}",
+ Style);
+
+ // 3. Use braces on the outer `if` to avoid a potential dangling else
+ // situation.
+ verifyFormat("if (isa<VarDecl>(D)) {\n"
+ " for (auto *A : D.attrs())\n"
+ " if (shouldProcessAttr(A))\n"
+ " handleAttr(A);\n"
+ "}",
+ "if (isa<VarDecl>(D)) {\n"
+ " for (auto *A : D.attrs()) {\n"
+ " if (shouldProcessAttr(A)) {\n"
+ " handleAttr(A);\n"
+ " }\n"
+ " }\n"
+ "}",
+ Style);
+
+ // 4. Use braces for the `if` block to keep it uniform with the else block.
+ verifyFormat("if (isa<FunctionDecl>(D)) {\n"
+ " handleFunctionDecl(D);\n"
+ "} else {\n"
+ " // In this else case, it is necessary that we explain the\n"
+ " // situation with this surprisingly long comment, so it\n"
+ " // would be unclear without the braces whether the\n"
+ " // following statement is in the scope of the `if`.\n"
+ " handleOtherDecl(D);\n"
+ "}",
+ Style);
+
+ // 5. This should also omit braces. The `for` loop contains only a single
+ // statement, so it shouldn't have braces. The `if` also only contains a
+ // single simple statement (the for loop), so it also should omit braces.
+ verifyFormat("if (isa<FunctionDecl>(D))\n"
+ " for (auto *A : D.attrs())\n"
+ " handleAttr(A);",
+ "if (isa<FunctionDecl>(D)) {\n"
+ " for (auto *A : D.attrs()) {\n"
+ " handleAttr(A);\n"
+ " }\n"
+ "}",
+ Style);
+
+ // 6. Use braces for the outer `if` since the nested `for` is braced.
+ verifyFormat("if (isa<FunctionDecl>(D)) {\n"
+ " for (auto *A : D.attrs()) {\n"
+ " // In this for loop body, it is necessary that we explain\n"
+ " // the situation with this surprisingly long comment,\n"
+ " // forcing braces on the `for` block.\n"
+ " handleAttr(A);\n"
+ " }\n"
+ "}",
+ Style);
+
+ // 7. Use braces on the outer block because there are more than two levels of
+ // nesting.
+ verifyFormat("if (isa<FunctionDecl>(D)) {\n"
+ " for (auto *A : D.attrs())\n"
+ " for (ssize_t i : llvm::seq<ssize_t>(count))\n"
+ " handleAttrOnDecl(D, A, i);\n"
+ "}",
+ "if (isa<FunctionDecl>(D)) {\n"
+ " for (auto *A : D.attrs()) {\n"
+ " for (ssize_t i : llvm::seq<ssize_t>(count)) {\n"
+ " handleAttrOnDecl(D, A, i);\n"
+ " }\n"
+ " }\n"
+ "}",
+ Style);
+
+ // 8. Use braces on the outer block because of a nested `if`, otherwise the
+ // compiler would warn: `add explicit braces to avoid dangling else`
+ verifyFormat("if (auto *D = dyn_cast<FunctionDecl>(D)) {\n"
+ " if (shouldProcess(D))\n"
+ " handleVarDecl(D);\n"
+ " else\n"
+ " markAsIgnored(D);\n"
+ "}",
+ "if (auto *D = dyn_cast<FunctionDecl>(D)) {\n"
+ " if (shouldProcess(D)) {\n"
+ " handleVarDecl(D);\n"
+ " } else {\n"
+ " markAsIgnored(D);\n"
+ " }\n"
+ "}",
+ Style);
+
+ verifyFormat("if (a)\n"
+ " b; // comment\n"
+ "else if (c)\n"
+ " d; /* comment */\n"
+ "else\n"
+ " e;",
+ "if (a) {\n"
+ " b; // comment\n"
+ "} else if (c) {\n"
+ " d; /* comment */\n"
+ "} else {\n"
+ " e;\n"
+ "}",
+ Style);
+
+ verifyFormat("if (a) {\n"
+ " b;\n"
+ " c;\n"
+ "} else if (d) {\n"
+ " e;\n"
+ "}",
+ Style);
+
+ verifyFormat("if (a) {\n"
+ "#undef NDEBUG\n"
+ " b;\n"
+ "} else {\n"
+ " c;\n"
+ "}",
+ Style);
+
+ verifyFormat("if (a) {\n"
+ " // comment\n"
+ "} else if (b) {\n"
+ " c;\n"
+ "}",
+ Style);
+
+ verifyFormat("if (a) {\n"
+ " b;\n"
+ "} else {\n"
+ " { c; }\n"
+ "}",
+ Style);
+
+ verifyFormat("if (a) {\n"
+ " if (b) // comment\n"
+ " c;\n"
+ "} else if (d) {\n"
+ " e;\n"
+ "}",
+ "if (a) {\n"
+ " if (b) { // comment\n"
+ " c;\n"
+ " }\n"
+ "} else if (d) {\n"
+ " e;\n"
+ "}",
+ Style);
+
+ verifyFormat("if (a) {\n"
+ " if (b) {\n"
+ " c;\n"
+ " // comment\n"
+ " } else if (d) {\n"
+ " e;\n"
+ " }\n"
+ "}",
+ Style);
+
+ verifyFormat("if (a) {\n"
+ " if (b)\n"
+ " c;\n"
+ "}",
+ "if (a) {\n"
+ " if (b) {\n"
+ " c;\n"
+ " }\n"
+ "}",
+ Style);
+
+ verifyFormat("if (a)\n"
+ " if (b)\n"
+ " c;\n"
+ " else\n"
+ " d;\n"
+ "else\n"
+ " e;",
+ "if (a) {\n"
+ " if (b) {\n"
+ " c;\n"
+ " } else {\n"
+ " d;\n"
+ " }\n"
+ "} else {\n"
+ " e;\n"
+ "}",
+ Style);
+
+ verifyFormat("if (a) {\n"
+ " // comment\n"
+ " if (b)\n"
+ " c;\n"
+ " else if (d)\n"
+ " e;\n"
+ "} else {\n"
+ " g;\n"
+ "}",
+ "if (a) {\n"
+ " // comment\n"
+ " if (b) {\n"
+ " c;\n"
+ " } else if (d) {\n"
+ " e;\n"
+ " }\n"
+ "} else {\n"
+ " g;\n"
+ "}",
+ Style);
+
+ verifyFormat("if (a)\n"
+ " b;\n"
+ "else if (c)\n"
+ " d;\n"
+ "else\n"
+ " e;",
+ "if (a) {\n"
+ " b;\n"
+ "} else {\n"
+ " if (c) {\n"
+ " d;\n"
+ " } else {\n"
+ " e;\n"
+ " }\n"
+ "}",
+ Style);
+
+ verifyFormat("if (a) {\n"
+ " if (b)\n"
+ " c;\n"
+ " else if (d)\n"
+ " e;\n"
+ "} else {\n"
+ " g;\n"
+ "}",
+ "if (a) {\n"
+ " if (b)\n"
+ " c;\n"
+ " else {\n"
+ " if (d)\n"
+ " e;\n"
+ " }\n"
+ "} else {\n"
+ " g;\n"
+ "}",
+ Style);
+
+ verifyFormat("if (a)\n"
+ " b;\n"
+ "else if (c)\n"
+ " while (d)\n"
+ " e;\n"
+ "// comment",
+ "if (a)\n"
+ "{\n"
+ " b;\n"
+ "} else if (c) {\n"
+ " while (d) {\n"
+ " e;\n"
+ " }\n"
+ "}\n"
+ "// comment",
+ Style);
+
+ verifyFormat("if (a) {\n"
+ " b;\n"
+ "} else if (c) {\n"
+ " d;\n"
+ "} else {\n"
+ " e;\n"
+ " g;\n"
+ "}",
+ Style);
+
+ verifyFormat("if (a) {\n"
+ " b;\n"
+ "} else if (c) {\n"
+ " d;\n"
+ "} else {\n"
+ " e;\n"
+ "} // comment",
+ Style);
+}
+
} // namespace
} // namespace format
} // namespace clang
Index: clang/lib/Format/UnwrappedLineParser.h
===================================================================
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -81,12 +81,20 @@
void parse();
private:
+ enum class IfStmtKind {
+ NotIf, // Not an if statement.
+ IfOnly, // An if statement without the else clause.
+ IfElse, // An if statement followed by else but not else if.
+ IfElseIf // An if statement followed by else if.
+ };
+
void reset();
void parseFile();
- void parseLevel(bool HasOpeningBrace);
- void parseBlock(bool MustBeDeclaration = false, unsigned AddLevels = 1u,
- bool MunchSemi = true,
- bool UnindentWhitesmithsBraces = false);
+ bool precededByCommentOrPPDirective() const;
+ bool parseLevel(bool HasOpeningBrace, IfStmtKind *IfKind = nullptr);
+ IfStmtKind parseBlock(bool MustBeDeclaration = false, unsigned AddLevels = 1u,
+ bool MunchSemi = true,
+ bool UnindentWhitesmithsBraces = false);
void parseChildBlock();
void parsePPDirective();
void parsePPDefine();
@@ -96,13 +104,15 @@
void parsePPEndIf();
void parsePPUnknown();
void readTokenWithJavaScriptASI();
- void parseStructuralElement(bool IsTopLevel = false);
+ void parseStructuralElement(IfStmtKind *IfKind = nullptr,
+ bool IsTopLevel = false);
bool tryToParseBracedList();
bool parseBracedList(bool ContinueOnSemicolons = false, bool IsEnum = false,
tok::TokenKind ClosingBraceKind = tok::r_brace);
void parseParens();
void parseSquare(bool LambdaIntroducer = false);
- void parseIfThenElse();
+ void keepAncestorBraces();
+ FormatToken *parseIfThenElse(IfStmtKind *IfKind, bool KeepBraces = false);
void parseTryCatch();
void parseForOrWhileLoop();
void parseDoWhile();
@@ -235,6 +245,10 @@
// owned outside of and handed into the UnwrappedLineParser.
ArrayRef<FormatToken *> AllTokens;
+ // Keeps a stack of the states of nested control statements (true if the
+ // statement contains more than some predefined number of nested statements).
+ SmallVector<bool, 8> NestedTooDeep;
+
// Represents preprocessor branch type, so we can find matching
// #if/#else/#endif directives.
enum PPBranchKind {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -316,6 +316,7 @@
PreprocessorDirectives.clear();
CurrentLines = &Lines;
DeclarationScopeStack.clear();
+ NestedTooDeep.clear();
PPStack.clear();
Line->FirstStartColumn = FirstStartColumn;
}
@@ -431,7 +432,28 @@
} while (!eof());
}
-void UnwrappedLineParser::parseLevel(bool HasOpeningBrace) {
+bool UnwrappedLineParser::precededByCommentOrPPDirective() const {
+ const size_t Size = Lines.size();
+ if (Size > 0 && Lines[Size - 1].InPPDirective)
+ return true;
+#if 1
+ const unsigned Position = Tokens->getPosition();
+ if (Position == 0)
+ return false;
+ const FormatToken *Previous = AllTokens[Position - 1];
+ assert(Previous);
+ return Previous->is(tok::comment) &&
+#else // Use the #else part after D116318 lands.
+ const FormatToken *Previous = Tokens->getPreviousToken();
+ return Previous && Previous->is(tok::comment) &&
+#endif
+ (Previous->IsMultiline || Previous->NewlinesBefore > 0);
+}
+
+bool UnwrappedLineParser::parseLevel(bool HasOpeningBrace, IfStmtKind *IfKind) {
+ const bool IsPrecededByCommentOrPPDirective =
+ !Style.RemoveBracesLLVM || precededByCommentOrPPDirective();
+ unsigned StatementCount = 0;
bool SwitchLabelEncountered = false;
do {
tok::TokenKind kind = FormatTok->Tok.getKind();
@@ -452,11 +474,22 @@
if (!FormatTok->is(TT_MacroBlockBegin) && tryToParseBracedList())
continue;
parseBlock();
+ ++StatementCount;
+ assert(StatementCount > 0 && "StatementCount overflow!");
addUnwrappedLine();
break;
case tok::r_brace:
- if (HasOpeningBrace)
- return;
+ if (HasOpeningBrace) {
+ if (!Style.RemoveBracesLLVM)
+ return false;
+ if (FormatTok->isNot(tok::r_brace) || StatementCount != 1 ||
+ IsPrecededByCommentOrPPDirective ||
+ precededByCommentOrPPDirective()) {
+ return false;
+ }
+ const FormatToken *Next = Tokens->peekNextToken();
+ return Next->NewlinesBefore > 0 || Next->isNot(tok::comment);
+ }
nextToken();
addUnwrappedLine();
break;
@@ -496,10 +529,13 @@
}
LLVM_FALLTHROUGH;
default:
- parseStructuralElement(!HasOpeningBrace);
+ parseStructuralElement(IfKind, !HasOpeningBrace);
+ ++StatementCount;
+ assert(StatementCount > 0 && "StatementCount overflow!");
break;
}
} while (!eof());
+ return false;
}
void UnwrappedLineParser::calculateBraceTypes(bool ExpectClassBody) {
@@ -655,11 +691,13 @@
return h;
}
-void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, unsigned AddLevels,
- bool MunchSemi,
- bool UnindentWhitesmithsBraces) {
+UnwrappedLineParser::IfStmtKind
+UnwrappedLineParser::parseBlock(bool MustBeDeclaration, unsigned AddLevels,
+ bool MunchSemi,
+ bool UnindentWhitesmithsBraces) {
assert(FormatTok->isOneOf(tok::l_brace, TT_MacroBlockBegin) &&
"'{' or macro block token expected");
+ FormatToken *Tok = FormatTok;
const bool MacroBlock = FormatTok->is(TT_MacroBlockBegin);
FormatTok->setBlockKind(BK_Block);
@@ -694,16 +732,28 @@
MustBeDeclaration);
if (AddLevels > 0u && Style.BreakBeforeBraces != FormatStyle::BS_Whitesmiths)
Line->Level += AddLevels;
- parseLevel(/*HasOpeningBrace=*/true);
+
+ IfStmtKind IfKind = IfStmtKind::NotIf;
+ const bool SimpleBlock = parseLevel(/*HasOpeningBrace=*/true, &IfKind);
if (eof())
- return;
+ return IfKind;
if (MacroBlock ? !FormatTok->is(TT_MacroBlockEnd)
: !FormatTok->is(tok::r_brace)) {
Line->Level = InitialLevel;
FormatTok->setBlockKind(BK_Block);
- return;
+ return IfKind;
+ }
+
+ if (SimpleBlock && Tok->is(tok::l_brace)) {
+ assert(FormatTok->is(tok::r_brace));
+ const FormatToken *Previous = Tokens->getPreviousToken();
+ assert(Previous);
+ if (Previous->isNot(tok::r_brace) || Previous->Optional) {
+ Tok->MatchingParen = FormatTok;
+ FormatTok->MatchingParen = Tok;
+ }
}
size_t PPEndHash = computePPHash();
@@ -734,6 +784,8 @@
CurrentLines->size() - 1;
}
}
+
+ return IfKind;
}
static bool isGoogScope(const UnwrappedLine &Line) {
@@ -1186,7 +1238,8 @@
return addUnwrappedLine();
}
-void UnwrappedLineParser::parseStructuralElement(bool IsTopLevel) {
+void UnwrappedLineParser::parseStructuralElement(IfStmtKind *IfKind,
+ bool IsTopLevel) {
if (Style.Language == FormatStyle::LK_TableGen &&
FormatTok->is(tok::pp_include)) {
nextToken();
@@ -1229,7 +1282,7 @@
if (Style.isJavaScript() && Line->MustBeDeclaration)
// field/method declaration.
break;
- parseIfThenElse();
+ parseIfThenElse(IfKind);
return;
case tok::kw_for:
case tok::kw_while:
@@ -2128,7 +2181,63 @@
} while (!eof());
}
-void UnwrappedLineParser::parseIfThenElse() {
+void UnwrappedLineParser::keepAncestorBraces() {
+ if (!Style.RemoveBracesLLVM)
+ return;
+
+ const int MaxNestingLevels = 2;
+ const int Size = NestedTooDeep.size();
+ if (Size >= MaxNestingLevels)
+ NestedTooDeep[Size - MaxNestingLevels] = true;
+ NestedTooDeep.push_back(false);
+}
+
+static bool resetBraces(FormatToken *LeftBrace) {
+ assert(LeftBrace);
+ assert(LeftBrace->is(tok::l_brace));
+
+ FormatToken *RightBrace = LeftBrace->MatchingParen;
+ if (!RightBrace) {
+ assert(!LeftBrace->Optional);
+ return false;
+ }
+
+ assert(RightBrace->is(tok::r_brace));
+ assert(RightBrace->MatchingParen == LeftBrace);
+ assert(LeftBrace->Optional == RightBrace->Optional);
+
+ const bool KeepBraces = !LeftBrace->Optional;
+
+ LeftBrace->Optional = false;
+ LeftBrace->MatchingParen = nullptr;
+ RightBrace->Optional = false;
+ RightBrace->MatchingParen = nullptr;
+
+ return KeepBraces;
+}
+
+static void markOptionalBraces(FormatToken *LeftBrace) {
+ if (!LeftBrace)
+ return;
+
+ assert(LeftBrace->is(tok::l_brace));
+
+ FormatToken *RightBrace = LeftBrace->MatchingParen;
+ if (!RightBrace) {
+ assert(!LeftBrace->Optional);
+ return;
+ }
+
+ assert(RightBrace->is(tok::r_brace));
+ assert(RightBrace->MatchingParen == LeftBrace);
+ assert(LeftBrace->Optional == RightBrace->Optional);
+
+ LeftBrace->Optional = true;
+ RightBrace->Optional = true;
+}
+
+FormatToken *UnwrappedLineParser::parseIfThenElse(IfStmtKind *IfKind,
+ bool KeepBraces) {
auto HandleAttributes = [this]() {
// Handle AttributeMacro, e.g. `if (x) UNLIKELY`.
if (FormatTok->is(TT_AttributeMacro))
@@ -2145,10 +2254,21 @@
if (FormatTok->Tok.is(tok::l_paren))
parseParens();
HandleAttributes();
+
bool NeedsUnwrappedLine = false;
+ keepAncestorBraces();
+
+ bool KeepIfBraces = false;
+ FormatToken *IfLeftBrace = nullptr;
+ IfStmtKind IfBlockKind = IfStmtKind::NotIf;
+
if (FormatTok->Tok.is(tok::l_brace)) {
+ if (Style.RemoveBracesLLVM) {
+ KeepIfBraces = resetBraces(FormatTok);
+ IfLeftBrace = FormatTok;
+ }
CompoundStatementIndenter Indenter(this, Style, Line->Level);
- parseBlock();
+ IfBlockKind = parseBlock();
if (Style.BraceWrapping.BeforeElse)
addUnwrappedLine();
else
@@ -2159,22 +2279,52 @@
parseStructuralElement();
--Line->Level;
}
+
+ if (Style.RemoveBracesLLVM) {
+ assert(!NestedTooDeep.empty());
+ KeepIfBraces =
+ KeepIfBraces || (IfLeftBrace && !IfLeftBrace->MatchingParen) ||
+ NestedTooDeep.back() || IfBlockKind == IfStmtKind::IfOnly || // LLVM
+ IfBlockKind == IfStmtKind::IfElseIf;
+ }
+
+ bool KeepElseBraces = false;
+ FormatToken *ElseLeftBrace = nullptr;
+ IfStmtKind Kind = IfStmtKind::IfOnly;
+
if (FormatTok->Tok.is(tok::kw_else)) {
+ if (Style.RemoveBracesLLVM) {
+ NestedTooDeep.back() = false;
+ Kind = IfStmtKind::IfElse;
+ }
nextToken();
HandleAttributes();
if (FormatTok->Tok.is(tok::l_brace)) {
+ if (Style.RemoveBracesLLVM) {
+ KeepElseBraces = resetBraces(FormatTok);
+ ElseLeftBrace = FormatTok;
+ }
CompoundStatementIndenter Indenter(this, Style, Line->Level);
- parseBlock();
+ if (parseBlock() == IfStmtKind::IfOnly)
+ Kind = IfStmtKind::IfElseIf;
addUnwrappedLine();
} else if (FormatTok->Tok.is(tok::kw_if)) {
FormatToken *Previous = Tokens->getPreviousToken();
- bool PrecededByComment = Previous && Previous->is(tok::comment);
- if (PrecededByComment) {
+ const bool IsPrecededByComment = Previous && Previous->is(tok::comment);
+ if (IsPrecededByComment) {
addUnwrappedLine();
++Line->Level;
}
- parseIfThenElse();
- if (PrecededByComment)
+ bool TooDeep = true;
+ if (Style.RemoveBracesLLVM) {
+ Kind = IfStmtKind::IfElseIf;
+ TooDeep = NestedTooDeep.pop_back_val();
+ }
+ ElseLeftBrace =
+ parseIfThenElse(/*IfKind=*/nullptr, KeepBraces || KeepIfBraces);
+ if (Style.RemoveBracesLLVM)
+ NestedTooDeep.push_back(TooDeep);
+ if (IsPrecededByComment)
--Line->Level;
} else {
addUnwrappedLine();
@@ -2184,9 +2334,41 @@
addUnwrappedLine();
--Line->Level;
}
- } else if (NeedsUnwrappedLine) {
- addUnwrappedLine();
+ } else {
+ if (Style.RemoveBracesLLVM)
+ KeepIfBraces = KeepIfBraces || IfBlockKind == IfStmtKind::IfElse;
+ if (NeedsUnwrappedLine)
+ addUnwrappedLine();
+ }
+
+ if (!Style.RemoveBracesLLVM)
+ return nullptr;
+
+ assert(!NestedTooDeep.empty());
+ KeepElseBraces = KeepElseBraces ||
+ (ElseLeftBrace && !ElseLeftBrace->MatchingParen) ||
+ NestedTooDeep.back();
+
+ NestedTooDeep.pop_back();
+
+ if (!KeepBraces && !KeepIfBraces && !KeepElseBraces) {
+ markOptionalBraces(IfLeftBrace);
+ markOptionalBraces(ElseLeftBrace);
+ } else if (IfLeftBrace) {
+ FormatToken *IfRightBrace = IfLeftBrace->MatchingParen;
+ if (IfRightBrace) {
+ assert(IfRightBrace->MatchingParen == IfLeftBrace);
+ assert(!IfLeftBrace->Optional);
+ assert(!IfRightBrace->Optional);
+ IfLeftBrace->MatchingParen = nullptr;
+ IfRightBrace->MatchingParen = nullptr;
+ }
}
+
+ if (IfKind)
+ *IfKind = Kind;
+
+ return IfLeftBrace;
}
void UnwrappedLineParser::parseTryCatch() {
@@ -2224,6 +2406,9 @@
if (Style.Language == FormatStyle::LK_Java && FormatTok->is(tok::l_paren)) {
parseParens();
}
+
+ keepAncestorBraces();
+
if (FormatTok->is(tok::l_brace)) {
CompoundStatementIndenter Indenter(this, Style, Line->Level);
parseBlock();
@@ -2257,8 +2442,11 @@
parseParens();
continue;
}
- if (FormatTok->isOneOf(tok::semi, tok::r_brace, tok::eof))
+ if (FormatTok->isOneOf(tok::semi, tok::r_brace, tok::eof)) {
+ if (Style.RemoveBracesLLVM)
+ NestedTooDeep.pop_back();
return;
+ }
nextToken();
}
NeedsUnwrappedLine = false;
@@ -2269,6 +2457,10 @@
else
NeedsUnwrappedLine = true;
}
+
+ if (Style.RemoveBracesLLVM)
+ NestedTooDeep.pop_back();
+
if (NeedsUnwrappedLine)
addUnwrappedLine();
}
@@ -2375,9 +2567,19 @@
nextToken();
if (FormatTok->Tok.is(tok::l_paren))
parseParens();
+
+ keepAncestorBraces();
+
if (FormatTok->Tok.is(tok::l_brace)) {
+ const bool KeepBraces = !Style.RemoveBracesLLVM || resetBraces(FormatTok);
+ FormatToken *LeftBrace = FormatTok;
CompoundStatementIndenter Indenter(this, Style, Line->Level);
parseBlock();
+ if (Style.RemoveBracesLLVM) {
+ assert(!NestedTooDeep.empty());
+ if (!KeepBraces && !NestedTooDeep.back())
+ markOptionalBraces(LeftBrace);
+ }
addUnwrappedLine();
} else {
addUnwrappedLine();
@@ -2385,11 +2587,17 @@
parseStructuralElement();
--Line->Level;
}
+
+ if (Style.RemoveBracesLLVM)
+ NestedTooDeep.pop_back();
}
void UnwrappedLineParser::parseDoWhile() {
assert(FormatTok->Tok.is(tok::kw_do) && "'do' expected");
nextToken();
+
+ keepAncestorBraces();
+
if (FormatTok->Tok.is(tok::l_brace)) {
CompoundStatementIndenter Indenter(this, Style, Line->Level);
parseBlock();
@@ -2402,6 +2610,9 @@
--Line->Level;
}
+ if (Style.RemoveBracesLLVM)
+ NestedTooDeep.pop_back();
+
// FIXME: Add error handling.
if (!FormatTok->Tok.is(tok::kw_while)) {
addUnwrappedLine();
@@ -2471,6 +2682,9 @@
nextToken();
if (FormatTok->Tok.is(tok::l_paren))
parseParens();
+
+ keepAncestorBraces();
+
if (FormatTok->Tok.is(tok::l_brace)) {
CompoundStatementIndenter Indenter(this, Style, Line->Level);
parseBlock();
@@ -2481,6 +2695,9 @@
parseStructuralElement();
--Line->Level;
}
+
+ if (Style.RemoveBracesLLVM)
+ NestedTooDeep.pop_back();
}
void UnwrappedLineParser::parseAccessSpecifier() {
Index: clang/lib/Format/TokenAnnotator.cpp
===================================================================
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -780,6 +780,7 @@
unsigned CommaCount = 0;
while (CurrentToken) {
if (CurrentToken->is(tok::r_brace)) {
+ assert(Left->Optional == CurrentToken->Optional);
Left->MatchingParen = CurrentToken;
CurrentToken->MatchingParen = Left;
if (Style.AlignArrayOfStructures != FormatStyle::AIAS_None) {
Index: clang/lib/Format/FormatToken.h
===================================================================
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -442,6 +442,9 @@
/// This starts an array initializer.
bool IsArrayInitializer = false;
+ /// Is optional and can be removed.
+ bool Optional = false;
+
/// If this token starts a block, this contains all the unwrapped lines
/// in it.
SmallVector<AnnotatedLine *, 1> Children;
Index: clang/lib/Format/Format.cpp
===================================================================
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -769,6 +769,7 @@
IO.mapOptional("RawStringFormats", Style.RawStringFormats);
IO.mapOptional("ReferenceAlignment", Style.ReferenceAlignment);
IO.mapOptional("ReflowComments", Style.ReflowComments);
+ IO.mapOptional("RemoveBracesLLVM", Style.RemoveBracesLLVM);
IO.mapOptional("ShortNamespaceLines", Style.ShortNamespaceLines);
IO.mapOptional("SortIncludes", Style.SortIncludes);
IO.mapOptional("SortJavaStaticImport", Style.SortJavaStaticImport);
@@ -1199,6 +1200,7 @@
LLVMStyle.UseCRLF = false;
LLVMStyle.UseTab = FormatStyle::UT_Never;
LLVMStyle.ReflowComments = true;
+ LLVMStyle.RemoveBracesLLVM = false;
LLVMStyle.SpacesInParentheses = false;
LLVMStyle.SpacesInSquareBrackets = false;
LLVMStyle.SpaceInEmptyBlock = false;
@@ -1732,6 +1734,43 @@
namespace {
+class BracesRemover : public TokenAnalyzer {
+public:
+ BracesRemover(const Environment &Env, const FormatStyle &Style)
+ : TokenAnalyzer(Env, Style) {}
+
+ std::pair<tooling::Replacements, unsigned>
+ analyze(TokenAnnotator &Annotator,
+ SmallVectorImpl<AnnotatedLine *> &AnnotatedLines,
+ FormatTokenLexer &Tokens) override {
+ AffectedRangeMgr.computeAffectedLines(AnnotatedLines);
+ tooling::Replacements Result;
+ removeBraces(AnnotatedLines, Result);
+ return {Result, 0};
+ }
+
+private:
+ // Remove optional braces.
+ void removeBraces(SmallVectorImpl<AnnotatedLine *> &Lines,
+ tooling::Replacements &Result) {
+ const auto &SourceMgr = Env.getSourceManager();
+ for (AnnotatedLine *Line : Lines) {
+ if (!Line->Affected)
+ continue;
+ for (FormatToken *Token = Line->First; Token; Token = Token->Next) {
+ if (!Token->Optional)
+ continue;
+ const auto Start = Token == Line->Last
+ ? Token->WhitespaceRange.getBegin()
+ : Token->Tok.getLocation();
+ const auto Range =
+ CharSourceRange::getCharRange(Start, Token->Tok.getEndLoc());
+ cantFail(Result.add(tooling::Replacement(SourceMgr, Range, "")));
+ }
+ }
+ }
+};
+
class JavaScriptRequoter : public TokenAnalyzer {
public:
JavaScriptRequoter(const Environment &Env, const FormatStyle &Style)
@@ -3038,6 +3077,11 @@
});
}
+ if (Style.isCpp() && Style.RemoveBracesLLVM)
+ Passes.emplace_back([&](const Environment &Env) {
+ return BracesRemover(Env, Expanded).process();
+ });
+
if (Style.Language == FormatStyle::LK_Cpp) {
if (Style.FixNamespaceComments)
Passes.emplace_back([&](const Environment &Env) {
Index: clang/include/clang/Format/Format.h
===================================================================
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -3050,6 +3050,58 @@
bool ReflowComments;
// clang-format on
+ /// Remove optional braces of control statements (``if``, ``else``, ``for``,
+ /// and ``while``) in C++ according to the LLVM coding style.
+ /// \warning
+ /// This option will be renamed and expanded to support other styles!
+ /// Setting this option to `true` could lead to incorrect code formatting due
+ /// to clang-format's lack of complete semantic information. As such, extra
+ /// care should be taken to review code changes made by this option.
+ /// \endwarning
+ /// \code
+ /// false: true:
+ ///
+ /// if (isa<FunctionDecl>(D)) { vs. if (isa<FunctionDecl>(D))
+ /// handleFunctionDecl(D); handleFunctionDecl(D);
+ /// } else if (isa<VarDecl>(D)) { else if (isa<VarDecl>(D))
+ /// handleVarDecl(D); handleVarDecl(D);
+ /// }
+ ///
+ /// if (isa<VarDecl>(D)) { vs. if (isa<VarDecl>(D)) {
+ /// for (auto *A : D.attrs()) { for (auto *A : D.attrs())
+ /// if (shouldProcessAttr(A)) { if (shouldProcessAttr(A))
+ /// handleAttr(A); handleAttr(A);
+ /// } }
+ /// }
+ /// }
+ ///
+ /// if (isa<FunctionDecl>(D)) { vs. if (isa<FunctionDecl>(D))
+ /// for (auto *A : D.attrs()) { for (auto *A : D.attrs())
+ /// handleAttr(A); handleAttr(A);
+ /// }
+ /// }
+ ///
+ /// if (auto *D = (T)(D)) { vs. if (auto *D = (T)(D)) {
+ /// if (shouldProcess(D)) { if (shouldProcess(D))
+ /// handleVarDecl(D); handleVarDecl(D);
+ /// } else { else
+ /// markAsIgnored(D); markAsIgnored(D);
+ /// } }
+ /// }
+ ///
+ /// if (a) { vs. if (a)
+ /// b(); b();
+ /// } else { else if (c)
+ /// if (c) { d();
+ /// d(); else
+ /// } else { e();
+ /// e();
+ /// }
+ /// }
+ /// \endcode
+ /// \version 14
+ bool RemoveBracesLLVM;
+
/// The maximal number of unwrapped lines that a short namespace spans.
/// Defaults to 1.
///
@@ -3791,6 +3843,7 @@
QualifierOrder == R.QualifierOrder &&
RawStringFormats == R.RawStringFormats &&
ReferenceAlignment == R.ReferenceAlignment &&
+ RemoveBracesLLVM == R.RemoveBracesLLVM &&
ShortNamespaceLines == R.ShortNamespaceLines &&
SortIncludes == R.SortIncludes &&
SortJavaStaticImport == R.SortJavaStaticImport &&
Index: clang/docs/ClangFormatStyleOptions.rst
===================================================================
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -3395,6 +3395,59 @@
/* second veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of
* information */
+**RemoveBracesLLVM** (``Boolean``) :versionbadge:`clang-format 14`
+ Remove optional braces of control statements (``if``, ``else``, ``for``,
+ and ``while``) in C++ according to the LLVM coding style.
+
+ .. warning::
+
+ This option will be renamed and expanded to support other styles!
+ Setting this option to `true` could lead to incorrect code formatting due
+ to clang-format's lack of complete semantic information. As such, extra
+ care should be taken to review code changes made by this option.
+
+ .. code-block:: c++
+
+ false: true:
+
+ if (isa<FunctionDecl>(D)) { vs. if (isa<FunctionDecl>(D))
+ handleFunctionDecl(D); handleFunctionDecl(D);
+ } else if (isa<VarDecl>(D)) { else if (isa<VarDecl>(D))
+ handleVarDecl(D); handleVarDecl(D);
+ }
+
+ if (isa<VarDecl>(D)) { vs. if (isa<VarDecl>(D)) {
+ for (auto *A : D.attrs()) { for (auto *A : D.attrs())
+ if (shouldProcessAttr(A)) { if (shouldProcessAttr(A))
+ handleAttr(A); handleAttr(A);
+ } }
+ }
+ }
+
+ if (isa<FunctionDecl>(D)) { vs. if (isa<FunctionDecl>(D))
+ for (auto *A : D.attrs()) { for (auto *A : D.attrs())
+ handleAttr(A); handleAttr(A);
+ }
+ }
+
+ if (auto *D = (T)(D)) { vs. if (auto *D = (T)(D)) {
+ if (shouldProcess(D)) { if (shouldProcess(D))
+ handleVarDecl(D); handleVarDecl(D);
+ } else { else
+ markAsIgnored(D); markAsIgnored(D);
+ } }
+ }
+
+ if (a) { vs. if (a)
+ b(); b();
+ } else { else if (c)
+ if (c) { d();
+ d(); else
+ } else { e();
+ e();
+ }
+ }
+
**ShortNamespaceLines** (``Unsigned``) :versionbadge:`clang-format 14`
The maximal number of unwrapped lines that a short namespace spans.
Defaults to 1.
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
