[clang] 533fbae - [clang-format] Add experimental option to remove LLVM braces
via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 14 15:10:40 PST 2022
Author: Owen Pan
Date: 2022-01-14T15:10:17-08:00
New Revision: 533fbae8d8d87fb151f852b5273e6e1626321567
URL: https://github.com/llvm/llvm-project/commit/533fbae8d8d87fb151f852b5273e6e1626321567
DIFF: https://github.com/llvm/llvm-project/commit/533fbae8d8d87fb151f852b5273e6e1626321567.diff
LOG: [clang-format] Add experimental option to remove LLVM braces
See the style examples at:
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements
Differential Revision: https://reviews.llvm.org/D116316
Added:
Modified:
clang/docs/ClangFormatStyleOptions.rst
clang/docs/ReleaseNotes.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
Removed:
################################################################################
diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst
index 9bc62587c8fb1..b752df864c56a 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -3402,6 +3402,62 @@ the configuration (without a prefix: ``Auto``).
/* 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.
+
+ .. 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);
+ }
+
+ 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();
+ }
+ }
+
**SeparateDefinitionBlocks** (``SeparateDefinitionStyle``) :versionbadge:`clang-format 14`
Specifies the use of empty lines to separate definition blocks, including
classes, structs, enums, and functions.
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 8b039b20c0d6e..b01f3035fc00c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -310,6 +310,9 @@ clang-format
`const` `volatile` `static` `inline` `constexpr` `restrict`
to be controlled relative to the `type`.
+- Option ``RemoveBracesLLVM`` has been added to remove optional braces of
+ control statements for the LLVM style.
+
- Option ``SeparateDefinitionBlocks`` has been added to insert or remove empty
lines between definition blocks including functions, classes, structs, enums,
and namespaces.
diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index fe35b4bd69c94..7841a7a9949d0 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -3049,6 +3049,60 @@ struct FormatStyle {
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.
+ /// \endwarning
+ /// \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);
+ /// }
+ ///
+ /// 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;
+
/// \brief The style if definition blocks should be separated.
enum SeparateDefinitionStyle {
/// Leave definition blocks as they are.
@@ -3858,6 +3912,7 @@ struct FormatStyle {
QualifierOrder == R.QualifierOrder &&
RawStringFormats == R.RawStringFormats &&
ReferenceAlignment == R.ReferenceAlignment &&
+ RemoveBracesLLVM == R.RemoveBracesLLVM &&
SeparateDefinitionBlocks == R.SeparateDefinitionBlocks &&
ShortNamespaceLines == R.ShortNamespaceLines &&
SortIncludes == R.SortIncludes &&
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index ffb1b14b76b6f..ce37ec7c9a5d3 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -781,6 +781,7 @@ template <> struct MappingTraits<FormatStyle> {
IO.mapOptional("RawStringFormats", Style.RawStringFormats);
IO.mapOptional("ReferenceAlignment", Style.ReferenceAlignment);
IO.mapOptional("ReflowComments", Style.ReflowComments);
+ IO.mapOptional("RemoveBracesLLVM", Style.RemoveBracesLLVM);
IO.mapOptional("SeparateDefinitionBlocks", Style.SeparateDefinitionBlocks);
IO.mapOptional("ShortNamespaceLines", Style.ShortNamespaceLines);
IO.mapOptional("SortIncludes", Style.SortIncludes);
@@ -1214,6 +1215,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
LLVMStyle.UseCRLF = false;
LLVMStyle.UseTab = FormatStyle::UT_Never;
LLVMStyle.ReflowComments = true;
+ LLVMStyle.RemoveBracesLLVM = false;
LLVMStyle.SpacesInParentheses = false;
LLVMStyle.SpacesInSquareBrackets = false;
LLVMStyle.SpaceInEmptyBlock = false;
@@ -1748,6 +1750,45 @@ FormatStyle::GetLanguageStyle(FormatStyle::LanguageKind Language) const {
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) {
+ removeBraces(Line->Children, Result);
+ if (!Line->Affected)
+ continue;
+ for (FormatToken *Token = Line->First; Token; Token = Token->Next) {
+ if (!Token->Optional)
+ continue;
+ assert(Token->isOneOf(tok::l_brace, tok::r_brace));
+ 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)
@@ -3049,6 +3090,11 @@ reformat(const FormatStyle &Style, StringRef Code,
});
}
+ 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) {
diff --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h
index d410ede32240d..8377273263737 100644
--- a/clang/lib/Format/FormatToken.h
+++ b/clang/lib/Format/FormatToken.h
@@ -442,6 +442,9 @@ struct FormatToken {
/// 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;
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 6181880b64569..cc449f9bb0398 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -780,6 +780,7 @@ class AnnotatingParser {
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) {
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index 410cce79a0876..69fe21cd87f01 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -14,6 +14,7 @@
#include "UnwrappedLineParser.h"
#include "FormatToken.h"
+#include "TokenAnnotator.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/Support/Debug.h"
#include "llvm/Support/raw_ostream.h"
@@ -315,6 +316,7 @@ void UnwrappedLineParser::reset() {
PreprocessorDirectives.clear();
CurrentLines = &Lines;
DeclarationScopeStack.clear();
+ NestedTooDeep.clear();
PPStack.clear();
Line->FirstStartColumn = FirstStartColumn;
}
@@ -428,7 +430,47 @@ void UnwrappedLineParser::parseCSharpAttribute() {
} while (!eof());
}
-void UnwrappedLineParser::parseLevel(bool HasOpeningBrace) {
+bool UnwrappedLineParser::precededByCommentOrPPDirective() const {
+ if (!Lines.empty() && Lines.back().InPPDirective)
+ return true;
+
+ const FormatToken *Previous = Tokens->getPreviousToken();
+ return Previous && Previous->is(tok::comment) &&
+ (Previous->IsMultiline || Previous->NewlinesBefore > 0);
+}
+
+bool UnwrappedLineParser::mightFitOnOneLine() const {
+ const auto ColumnLimit = Style.ColumnLimit;
+ if (ColumnLimit == 0)
+ return true;
+
+ if (Lines.empty())
+ return true;
+
+ const auto &PreviousLine = Lines.back();
+ const auto &Tokens = PreviousLine.Tokens;
+ assert(!Tokens.empty());
+ const auto *LastToken = Tokens.back().Tok;
+ assert(LastToken);
+ if (!LastToken->isOneOf(tok::semi, tok::comment))
+ return true;
+
+ AnnotatedLine Line(PreviousLine);
+ assert(Line.Last == LastToken);
+
+ TokenAnnotator Annotator(Style, Keywords);
+ Annotator.annotate(Line);
+ Annotator.calculateFormattingInformation(Line);
+
+ return Line.Level * Style.IndentWidth + LastToken->TotalLength <= ColumnLimit;
+}
+
+// Returns true if a simple block, or false otherwise. (A simple block has a
+// single statement that fits on a single line.)
+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();
@@ -449,11 +491,24 @@ void UnwrappedLineParser::parseLevel(bool HasOpeningBrace) {
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();
+ if (Next->is(tok::comment) && Next->NewlinesBefore == 0)
+ return false;
+ return mightFitOnOneLine();
+ }
nextToken();
addUnwrappedLine();
break;
@@ -493,10 +548,13 @@ void UnwrappedLineParser::parseLevel(bool HasOpeningBrace) {
}
LLVM_FALLTHROUGH;
default:
- parseStructuralElement(!HasOpeningBrace);
+ parseStructuralElement(IfKind, !HasOpeningBrace);
+ ++StatementCount;
+ assert(StatementCount > 0 && "StatementCount overflow!");
break;
}
} while (!eof());
+ return false;
}
void UnwrappedLineParser::calculateBraceTypes(bool ExpectClassBody) {
@@ -652,11 +710,13 @@ size_t UnwrappedLineParser::computePPHash() const {
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);
@@ -691,16 +751,28 @@ void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, unsigned AddLevels,
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();
@@ -731,6 +803,8 @@ void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, unsigned AddLevels,
CurrentLines->size() - 1;
}
}
+
+ return IfKind;
}
static bool isGoogScope(const UnwrappedLine &Line) {
@@ -1185,7 +1259,8 @@ void UnwrappedLineParser::readTokenWithJavaScriptASI() {
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();
@@ -1228,7 +1303,7 @@ void UnwrappedLineParser::parseStructuralElement(bool IsTopLevel) {
if (Style.isJavaScript() && Line->MustBeDeclaration)
// field/method declaration.
break;
- parseIfThenElse();
+ parseIfThenElse(IfKind);
return;
case tok::kw_for:
case tok::kw_while:
@@ -2138,7 +2213,39 @@ void UnwrappedLineParser::parseSquare(bool LambdaIntroducer) {
} 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 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))
@@ -2155,10 +2262,17 @@ void UnwrappedLineParser::parseIfThenElse() {
if (FormatTok->Tok.is(tok::l_paren))
parseParens();
HandleAttributes();
+
bool NeedsUnwrappedLine = false;
+ keepAncestorBraces();
+
+ FormatToken *IfLeftBrace = nullptr;
+ IfStmtKind IfBlockKind = IfStmtKind::NotIf;
+
if (FormatTok->Tok.is(tok::l_brace)) {
+ IfLeftBrace = FormatTok;
CompoundStatementIndenter Indenter(this, Style, Line->Level);
- parseBlock();
+ IfBlockKind = parseBlock();
if (Style.BraceWrapping.BeforeElse)
addUnwrappedLine();
else
@@ -2169,22 +2283,48 @@ void UnwrappedLineParser::parseIfThenElse() {
parseStructuralElement();
--Line->Level;
}
+
+ bool KeepIfBraces = false;
+ if (Style.RemoveBracesLLVM) {
+ assert(!NestedTooDeep.empty());
+ KeepIfBraces = (IfLeftBrace && !IfLeftBrace->MatchingParen) ||
+ NestedTooDeep.back() || IfBlockKind == IfStmtKind::IfOnly ||
+ IfBlockKind == IfStmtKind::IfElseIf;
+ }
+
+ 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)) {
+ 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();
@@ -2194,9 +2334,40 @@ void UnwrappedLineParser::parseIfThenElse() {
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());
+ const bool 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() {
@@ -2234,6 +2405,9 @@ void UnwrappedLineParser::parseTryCatch() {
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();
@@ -2267,8 +2441,11 @@ void UnwrappedLineParser::parseTryCatch() {
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;
@@ -2279,6 +2456,10 @@ void UnwrappedLineParser::parseTryCatch() {
else
NeedsUnwrappedLine = true;
}
+
+ if (Style.RemoveBracesLLVM)
+ NestedTooDeep.pop_back();
+
if (NeedsUnwrappedLine)
addUnwrappedLine();
}
@@ -2385,9 +2566,18 @@ void UnwrappedLineParser::parseForOrWhileLoop() {
nextToken();
if (FormatTok->Tok.is(tok::l_paren))
parseParens();
+
+ keepAncestorBraces();
+
if (FormatTok->Tok.is(tok::l_brace)) {
+ FormatToken *LeftBrace = FormatTok;
CompoundStatementIndenter Indenter(this, Style, Line->Level);
parseBlock();
+ if (Style.RemoveBracesLLVM) {
+ assert(!NestedTooDeep.empty());
+ if (!NestedTooDeep.back())
+ markOptionalBraces(LeftBrace);
+ }
addUnwrappedLine();
} else {
addUnwrappedLine();
@@ -2395,11 +2585,17 @@ void UnwrappedLineParser::parseForOrWhileLoop() {
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();
@@ -2412,6 +2608,9 @@ void UnwrappedLineParser::parseDoWhile() {
--Line->Level;
}
+ if (Style.RemoveBracesLLVM)
+ NestedTooDeep.pop_back();
+
// FIXME: Add error handling.
if (!FormatTok->Tok.is(tok::kw_while)) {
addUnwrappedLine();
@@ -2481,6 +2680,9 @@ void UnwrappedLineParser::parseSwitch() {
nextToken();
if (FormatTok->Tok.is(tok::l_paren))
parseParens();
+
+ keepAncestorBraces();
+
if (FormatTok->Tok.is(tok::l_brace)) {
CompoundStatementIndenter Indenter(this, Style, Line->Level);
parseBlock();
@@ -2491,6 +2693,9 @@ void UnwrappedLineParser::parseSwitch() {
parseStructuralElement();
--Line->Level;
}
+
+ if (Style.RemoveBracesLLVM)
+ NestedTooDeep.pop_back();
}
void UnwrappedLineParser::parseAccessSpecifier() {
diff --git a/clang/lib/Format/UnwrappedLineParser.h b/clang/lib/Format/UnwrappedLineParser.h
index 0c79723d50fcc..8d4d4dca7633f 100644
--- a/clang/lib/Format/UnwrappedLineParser.h
+++ b/clang/lib/Format/UnwrappedLineParser.h
@@ -81,12 +81,21 @@ class UnwrappedLineParser {
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 mightFitOnOneLine() 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 +105,15 @@ class UnwrappedLineParser {
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 +246,10 @@ class UnwrappedLineParser {
// 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 {
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 27bf1d14e6664..fa993857db351 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -18889,6 +18889,7 @@ TEST_F(FormatTest, ParsesConfigurationBools) {
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);
@@ -23334,6 +23335,367 @@ TEST_F(FormatTest, ShortTemplatedArgumentLists) {
verifyFormat("struct Z : X<decltype([] { return 0; }){}> {};", 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);
+
+ verifyFormat("int abs = [](int i) {\n"
+ " if (i >= 0)\n"
+ " return i;\n"
+ " return -i;\n"
+ "};",
+ "int abs = [](int i) {\n"
+ " if (i >= 0) {\n"
+ " return i;\n"
+ " }\n"
+ " return -i;\n"
+ "};",
+ Style);
+
+ Style.ColumnLimit = 20;
+
+ verifyFormat("if (a) {\n"
+ " b = c + // 1 -\n"
+ " d;\n"
+ "}",
+ Style);
+
+ verifyFormat("if (a) {\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"
+ " b = c > 0 ? d : e;\n"
+ "}",
+ Style);
+
+ Style.ColumnLimit = 0;
+
+ verifyFormat("if (a)\n"
+ " b234567890223456789032345678904234567890 = "
+ "c234567890223456789032345678904234567890;",
+ "if (a) {\n"
+ " b234567890223456789032345678904234567890 = "
+ "c234567890223456789032345678904234567890;\n"
+ "}",
+ Style);
+}
+
} // namespace
} // namespace format
} // namespace clang
More information about the cfe-commits
mailing list