From 74317e4d3a6e662412cdd971df57da326229fedb Mon Sep 17 00:00:00 2001 From: Daniel Jasper Date: Thu, 24 Oct 2013 14:14:49 +0000 Subject: [PATCH] clang-format: Be more conservative about column layout formatting. Specifically, if a braced list has at least one nested braced list, format it either all on one line or in one column (i.e. one item per line). This seems in general to be an improvement as the structure of nested braced lists can make a tightly packed outer braced list hard to read. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@193345 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Format/FormatToken.cpp | 10 +++++++++- unittests/Format/FormatTest.cpp | 14 ++++++++------ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/lib/Format/FormatToken.cpp b/lib/Format/FormatToken.cpp index 4ca4edbe15..8ac704a3bb 100644 --- a/lib/Format/FormatToken.cpp +++ b/lib/Format/FormatToken.cpp @@ -99,12 +99,15 @@ void CommaSeparatedList::precomputeFormattingInfos(const FormatToken *Token) { // trailing comments which are otherwise ignored for column alignment. SmallVector EndOfLineItemLength; + bool HasNestedBracedList = false; for (unsigned i = 0, e = Commas.size() + 1; i != e; ++i) { // Skip comments on their own line. while (ItemBegin->HasUnescapedNewline && ItemBegin->isTrailingComment()) ItemBegin = ItemBegin->Next; MustBreakBeforeItem.push_back(ItemBegin->MustBreakBefore); + if (ItemBegin->is(tok::l_brace)) + HasNestedBracedList = true; const FormatToken *ItemEnd = NULL; if (i == Commas.size()) { ItemEnd = Token->MatchingParen; @@ -142,7 +145,7 @@ void CommaSeparatedList::precomputeFormattingInfos(const FormatToken *Token) { ColumnFormat Format; Format.Columns = Columns; Format.ColumnSizes.resize(Columns); - Format.LineCount = 0; + Format.LineCount = 1; bool HasRowWithSufficientColumns = false; unsigned Column = 0; for (unsigned i = 0, e = ItemLengths.size(); i != e; ++i) { @@ -172,6 +175,11 @@ void CommaSeparatedList::precomputeFormattingInfos(const FormatToken *Token) { if (Format.TotalWidth > Style.ColumnLimit) continue; + // If this braced list has nested braced list, we format it either with one + // element per line or with all elements on one line. + if (HasNestedBracedList && Columns > 1 && Format.LineCount > 1) + continue; + Formats.push_back(Format); } } diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp index f6e42ca211..9b24f040d1 100644 --- a/unittests/Format/FormatTest.cpp +++ b/unittests/Format/FormatTest.cpp @@ -1864,14 +1864,14 @@ TEST_F(FormatTest, NestedStaticInitializers) { " 333333333333333333333333333333}},\n" " {{1, 2, 3}}, {{1, 2, 3}}};"); - // FIXME: We might at some point want to handle this similar to parameter - // lists, where we have an option to put each on a single line. verifyFormat( "struct {\n" " unsigned bit;\n" " const char *const name;\n" - "} kBitsToOs[] = { { kOsMac, \"Mac\" }, { kOsWin, \"Windows\" },\n" - " { kOsLinux, \"Linux\" }, { kOsCrOS, \"Chrome OS\" } };"); + "} kBitsToOs[] = { { kOsMac, \"Mac\" },\n" + " { kOsWin, \"Windows\" },\n" + " { kOsLinux, \"Linux\" },\n" + " { kOsCrOS, \"Chrome OS\" } };"); } TEST_F(FormatTest, FormatsSmallMacroDefinitionsInSingleLine) { @@ -4539,8 +4539,10 @@ TEST_F(FormatTest, FormatsBracedListsinColumnLayout) { " 1, 1, 1, 1,\n" " /**/ /**/ };", getLLVMStyleWithColumns(39)); - verifyFormat("return { { aaaaaaaaaaaaaaaaaaaaa }, { aaaaaaaaaaaaaaaaaaa },\n" - " { aaaaaaaaaaaaaaaaaaaaa }, { aaaaaaaaaaaaaaaaa } };", + verifyFormat("return { { aaaaaaaaaaaaaaaaaaaaa },\n" + " { aaaaaaaaaaaaaaaaaaa },\n" + " { aaaaaaaaaaaaaaaaaaaaa },\n" + " { aaaaaaaaaaaaaaaaa } };", getLLVMStyleWithColumns(60)); } -- 2.40.0