owenpan created this revision.
owenpan added reviewers: MyDeveloperDay, curdeius, HazardyKnusperkeks.
owenpan added a project: clang-format.
owenpan requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
This new option `InsertBraces` has been tested on `make clang` and `make
check-clang`. It has almost zero overhead when turned off.
- Why this new patch?
Because of this comment
<https://discourse.llvm.org/t/doubts-regarding-clang-format/58091/5>, I ran
https://reviews.llvm.org/D95168?id=322240 (the last diff before @MyDeveloperDay
took over) on `make clang` and indeed it failed a lot.
- Why isn't this option an `enum` or a `struct`?
We plan to re-configure/re-package `RemoveBracesLLVM` to support other styles.
After that, one can simply turn on `InsertBraces` and specify the suboptions of
`RemoveBraces`.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D120217
Files:
clang/docs/ClangFormatStyleOptions.rst
clang/include/clang/Format/Format.h
clang/lib/Format/Format.cpp
clang/lib/Format/FormatToken.h
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
@@ -19474,6 +19474,7 @@
CHECK_PARSE_BOOL_FIELD(IndentRequiresClause, "IndentRequires");
CHECK_PARSE_BOOL(IndentRequiresClause);
CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
+ CHECK_PARSE_BOOL(InsertBraces);
CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);
CHECK_PARSE_BOOL(ObjCSpaceBeforeProtocolList);
@@ -24290,6 +24291,188 @@
verifyFormat("template <int N> struct Foo<char[N]> {};", Style);
}
+TEST_F(FormatTest, InsertBraces) {
+ FormatStyle Style = getLLVMStyle();
+ Style.InsertBraces = true;
+
+ verifyFormat("if (a) {\n"
+ " switch (b) {\n"
+ " case 1:\n"
+ " c = 0;\n"
+ " break;\n"
+ " default:\n"
+ " c = 1;\n"
+ " }\n"
+ "}",
+ "if (a)\n"
+ " switch (b) {\n"
+ " case 1:\n"
+ " c = 0;\n"
+ " break;\n"
+ " default:\n"
+ " c = 1;\n"
+ " }",
+ Style);
+
+ verifyFormat("for (auto node : nodes) {\n"
+ " if (node) {\n"
+ " break;\n"
+ " }\n"
+ "}",
+ "for (auto node : nodes)\n"
+ " if (node)\n"
+ " break;",
+ Style);
+
+ verifyFormat("for (auto node : nodes) {\n"
+ " if (node)\n"
+ "}",
+ "for (auto node : nodes)\n"
+ " if (node)",
+ Style);
+
+ verifyFormat("do {\n"
+ " --a;\n"
+ "} while (a);",
+ "do\n"
+ " --a;\n"
+ "while (a);",
+ Style);
+
+ verifyFormat("if (i) {\n"
+ " ++i;\n"
+ "} else {\n"
+ " --i;\n"
+ "}",
+ "if (i)\n"
+ " ++i;\n"
+ "else {\n"
+ " --i;\n"
+ "}",
+ Style);
+
+ verifyFormat("void f() {\n"
+ " while (j--) {\n"
+ " while (i) {\n"
+ " --i;\n"
+ " }\n"
+ " }\n"
+ "}",
+ "void f() {\n"
+ " while (j--)\n"
+ " while (i)\n"
+ " --i;\n"
+ "}",
+ Style);
+
+ verifyFormat("f({\n"
+ " if (a) {\n"
+ " g();\n"
+ " }\n"
+ "});",
+ "f({\n"
+ " if (a)\n"
+ " g();\n"
+ "});",
+ Style);
+
+ verifyFormat("if (a) {\n"
+ " f();\n"
+ "} else if (b) {\n"
+ " g();\n"
+ "} else {\n"
+ " h();\n"
+ "}",
+ "if (a)\n"
+ " f();\n"
+ "else if (b)\n"
+ " g();\n"
+ "else\n"
+ " h();",
+ Style);
+
+ verifyFormat("if (a) {\n"
+ " f();\n"
+ "}\n"
+ "// comment\n"
+ "/* comment */",
+ "if (a)\n"
+ " f();\n"
+ "// comment\n"
+ "/* comment */",
+ Style);
+
+ verifyFormat("if (a) {\n"
+ " // foo\n"
+ " // bar\n"
+ " f();\n"
+ "}",
+ "if (a)\n"
+ " // foo\n"
+ " // bar\n"
+ " f();",
+ Style);
+
+ verifyFormat("if (a) { // comment\n"
+ " // comment\n"
+ " f();\n"
+ "}",
+ "if (a) // comment\n"
+ " // comment\n"
+ " f();",
+ Style);
+
+ verifyFormat("if (a) {\n"
+ " f();\n"
+ "}\n"
+ "#undef A\n"
+ "#undef B",
+ "if (a)\n"
+ " f();\n"
+ "#undef A\n"
+ "#undef B",
+ Style);
+
+ verifyFormat("if (a)\n"
+ "#ifdef A\n"
+ " f();\n"
+ "#else\n"
+ " g();\n"
+ "#endif",
+ Style);
+
+ verifyFormat("#if 0\n"
+ "#elif 1\n"
+ "#endif\n"
+ "void f() {\n"
+ " if (a) {\n"
+ " g();\n"
+ " }\n"
+ "}",
+ "#if 0\n"
+ "#elif 1\n"
+ "#endif\n"
+ "void f() {\n"
+ " if (a) g();\n"
+ "}",
+ Style);
+
+ Style.ColumnLimit = 15;
+
+ verifyFormat("#define A \\\n"
+ " if (a) \\\n"
+ " f();",
+ Style);
+
+ verifyFormat("if (a + b >\n"
+ " c) {\n"
+ " f();\n"
+ "}",
+ "if (a + b > c)\n"
+ " f();",
+ Style);
+}
+
TEST_F(FormatTest, RemoveBraces) {
FormatStyle Style = getLLVMStyle();
Style.RemoveBracesLLVM = true;
Index: clang/lib/Format/UnwrappedLineParser.h
===================================================================
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -119,6 +119,7 @@
void parseParens(TokenType AmpAmpTokenType = TT_Unknown);
void parseSquare(bool LambdaIntroducer = false);
void keepAncestorBraces();
+ void parseUnbracedBody(bool CheckEOF = false);
FormatToken *parseIfThenElse(IfStmtKind *IfKind, bool KeepBraces = false);
void parseTryCatch();
void parseForOrWhileLoop();
Index: clang/lib/Format/UnwrappedLineParser.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2299,6 +2299,53 @@
NestedTooDeep.push_back(false);
}
+static FormatToken *getLastNonComment(const UnwrappedLine &Line) {
+ for (const auto &Token : llvm::reverse(Line.Tokens))
+ if (Token.Tok->isNot(tok::comment))
+ return Token.Tok;
+
+ return nullptr;
+}
+
+void UnwrappedLineParser::parseUnbracedBody(bool CheckEOF) {
+ FormatToken *Tok = nullptr;
+
+ if (Style.InsertBraces && !Line->InPPDirective && !Line->Tokens.empty() &&
+ PreprocessorDirectives.empty()) {
+ Tok = getLastNonComment(*Line);
+ assert(Tok);
+ if (Tok->BraceCount < 0) {
+ assert(Tok->BraceCount == -1);
+ Tok = nullptr;
+ } else {
+ Tok->BraceCount = -1;
+ }
+ }
+
+ addUnwrappedLine();
+ ++Line->Level;
+ parseStructuralElement();
+
+ if (Tok) {
+ assert(!Line->InPPDirective);
+ Tok = nullptr;
+ for (const auto &L : llvm::reverse(*CurrentLines)) {
+ if (!L.InPPDirective) {
+ Tok = getLastNonComment(L);
+ if (Tok)
+ break;
+ }
+ }
+ assert(Tok);
+ ++Tok->BraceCount;
+ }
+
+ if (CheckEOF && FormatTok->is(tok::eof))
+ addUnwrappedLine();
+
+ --Line->Level;
+}
+
static void markOptionalBraces(FormatToken *LeftBrace) {
if (!LeftBrace)
return;
@@ -2353,10 +2400,7 @@
else
NeedsUnwrappedLine = true;
} else {
- addUnwrappedLine();
- ++Line->Level;
- parseStructuralElement();
- --Line->Level;
+ parseUnbracedBody();
}
bool KeepIfBraces = false;
@@ -2402,12 +2446,7 @@
if (IsPrecededByComment)
--Line->Level;
} else {
- addUnwrappedLine();
- ++Line->Level;
- parseStructuralElement();
- if (FormatTok->is(tok::eof))
- addUnwrappedLine();
- --Line->Level;
+ parseUnbracedBody(/*CheckEOF=*/true);
}
} else {
if (Style.RemoveBracesLLVM)
@@ -2653,10 +2692,7 @@
}
addUnwrappedLine();
} else {
- addUnwrappedLine();
- ++Line->Level;
- parseStructuralElement();
- --Line->Level;
+ parseUnbracedBody();
}
if (Style.RemoveBracesLLVM)
@@ -2675,10 +2711,7 @@
if (Style.BraceWrapping.BeforeWhile)
addUnwrappedLine();
} else {
- addUnwrappedLine();
- ++Line->Level;
- parseStructuralElement();
- --Line->Level;
+ parseUnbracedBody();
}
if (Style.RemoveBracesLLVM)
Index: clang/lib/Format/FormatToken.h
===================================================================
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -489,6 +489,12 @@
/// Is optional and can be removed.
bool Optional = false;
+ /// Number of optional braces to be inserted after this token:
+ /// -1: a single left brace
+ /// 0: no braces
+ /// >0: number of right braces
+ int8_t BraceCount = 0;
+
/// 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
@@ -768,6 +768,7 @@
IO.mapOptional("IndentWidth", Style.IndentWidth);
IO.mapOptional("IndentWrappedFunctionNames",
Style.IndentWrappedFunctionNames);
+ IO.mapOptional("InsertBraces", Style.InsertBraces);
IO.mapOptional("InsertTrailingCommas", Style.InsertTrailingCommas);
IO.mapOptional("JavaImportGroups", Style.JavaImportGroups);
IO.mapOptional("JavaScriptQuotes", Style.JavaScriptQuotes);
@@ -1223,6 +1224,7 @@
LLVMStyle.IndentWrappedFunctionNames = false;
LLVMStyle.IndentWidth = 2;
LLVMStyle.PPIndentWidth = -1;
+ LLVMStyle.InsertBraces = false;
LLVMStyle.InsertTrailingCommas = FormatStyle::TCS_None;
LLVMStyle.JavaScriptQuotes = FormatStyle::JSQS_Leave;
LLVMStyle.JavaScriptWrapImports = true;
@@ -1818,6 +1820,48 @@
}
};
+class BracesInserter : public TokenAnalyzer {
+public:
+ BracesInserter(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;
+ insertBraces(AnnotatedLines, Result);
+ return {Result, 0};
+ }
+
+private:
+ // Insert optional braces.
+ void insertBraces(SmallVectorImpl<AnnotatedLine *> &Lines,
+ tooling::Replacements &Result) {
+ const auto &SourceMgr = Env.getSourceManager();
+ for (AnnotatedLine *Line : Lines) {
+ insertBraces(Line->Children, Result);
+ if (!Line->Affected)
+ continue;
+ for (FormatToken *Token = Line->First; Token; Token = Token->Next) {
+ if (Token->BraceCount == 0)
+ continue;
+ std::string Brace;
+ if (Token->BraceCount < 0) {
+ assert(Token->BraceCount == -1);
+ Brace = '{';
+ } else {
+ Brace = std::string(Token->BraceCount, '}');
+ }
+ Token->BraceCount = 0;
+ const auto Start = Token->Tok.getEndLoc();
+ cantFail(Result.add(tooling::Replacement(SourceMgr, Start, 0, Brace)));
+ }
+ }
+ }
+};
+
class JavaScriptRequoter : public TokenAnalyzer {
public:
JavaScriptRequoter(const Environment &Env, const FormatStyle &Style)
@@ -3130,6 +3174,11 @@
});
}
+ if (Style.isCpp() && Style.InsertBraces)
+ Passes.emplace_back([&](const Environment &Env) {
+ return BracesInserter(Env, Expanded).process();
+ });
+
if (Style.isCpp() && Style.RemoveBracesLLVM)
Passes.emplace_back([&](const Environment &Env) {
return BracesRemover(Env, Expanded).process();
Index: clang/include/clang/Format/Format.h
===================================================================
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -2569,6 +2569,37 @@
/// \version 3.7
bool IndentWrappedFunctionNames;
+ /// Insert braces after control statements (``if``, ``else``, ``for``, ``do``,
+ /// and ``while``) in C++.
+ /// \warning
+ /// 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);
+ /// else } else {
+ /// return; return;
+ /// }
+ ///
+ /// while (i--) vs. while (i--) {
+ /// for (auto *A : D.attrs()) for (auto *A : D.attrs()) {
+ /// handleAttr(A); handleAttr(A);
+ /// }
+ /// }
+ ///
+ /// do vs. do {
+ /// --i; --i;
+ /// while (i); } while (i);
+ /// \endcode
+ /// \version 15
+ bool InsertBraces;
+
/// A vector of prefixes ordered by the desired groups for Java imports.
///
/// One group's prefix can be a subset of another - the longest prefix is
Index: clang/docs/ClangFormatStyleOptions.rst
===================================================================
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -2754,6 +2754,38 @@
LoooooooooooooooooooooooooooooooooooooooongReturnType
LoooooooooooooooooooooooooooooooongFunctionDeclaration();
+**InsertBraces** (``Boolean``) :versionbadge:`clang-format 15`
+ Insert braces after control statements (``if``, ``else``, ``for``, ``do``,
+ and ``while``) in C++.
+
+ .. warning::
+
+ 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);
+ else } else {
+ return; return;
+ }
+
+ while (i--) vs. while (i--) {
+ for (auto *A : D.attrs()) for (auto *A : D.attrs()) {
+ handleAttr(A); handleAttr(A);
+ }
+ }
+
+ do vs. do {
+ --i; --i;
+ while (i); } while (i);
+
**InsertTrailingCommas** (``TrailingCommaStyle``) :versionbadge:`clang-format 12`
If set to ``TCS_Wrapped`` will insert trailing commas in container
literals (arrays and objects) that wrap across multiple lines.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
