From: Daniel Jasper Date: Thu, 9 Jan 2014 13:42:56 +0000 (+0000) Subject: clang-format: Some tweaks to braces list formatting: X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=3a93edef5bd0f5adfdd81cd458b88cb9f84690b4;p=clang clang-format: Some tweaks to braces list formatting: - Format a braced list with one element per line if it has nested braced lists. - Use a column layout only when the list has 6+ elements (instead of the current 4+ elements). git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@198869 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Format/ContinuationIndenter.cpp b/lib/Format/ContinuationIndenter.cpp index 2812a405b8..acd670b945 100644 --- a/lib/Format/ContinuationIndenter.cpp +++ b/lib/Format/ContinuationIndenter.cpp @@ -721,13 +721,15 @@ unsigned ContinuationIndenter::moveStateToNextToken(LineState &State, Penalty += Style.PenaltyExcessCharacter * ExcessCharacters; } + if (Current.Role) + Current.Role->formatFromToken(State, this, DryRun); // If the previous has a special role, let it consume tokens as appropriate. // It is necessary to start at the previous token for the only implemented // role (comma separated list). That way, the decision whether or not to break // after the "{" is already done and both options are tried and evaluated. // FIXME: This is ugly, find a better way. if (Previous && Previous->Role) - Penalty += Previous->Role->format(State, this, DryRun); + Penalty += Previous->Role->formatAfterToken(State, this, DryRun); return Penalty; } diff --git a/lib/Format/FormatToken.cpp b/lib/Format/FormatToken.cpp index 16600f24ee..0816668b57 100644 --- a/lib/Format/FormatToken.cpp +++ b/lib/Format/FormatToken.cpp @@ -26,11 +26,10 @@ TokenRole::~TokenRole() {} void TokenRole::precomputeFormattingInfos(const FormatToken *Token) {} -unsigned CommaSeparatedList::format(LineState &State, - ContinuationIndenter *Indenter, - bool DryRun) { - if (!State.NextToken->Previous || !State.NextToken->Previous->Previous || - Commas.size() <= 2) +unsigned CommaSeparatedList::formatAfterToken(LineState &State, + ContinuationIndenter *Indenter, + bool DryRun) { + if (!State.NextToken->Previous || !State.NextToken->Previous->Previous) return 0; // Ensure that we start on the opening brace. @@ -82,6 +81,14 @@ unsigned CommaSeparatedList::format(LineState &State, return Penalty; } +unsigned CommaSeparatedList::formatFromToken(LineState &State, + ContinuationIndenter *Indenter, + bool DryRun) { + if (HasNestedBracedList) + State.Stack.back().AvoidBinPacking = true; + return 0; +} + // Returns the lengths in code points between Begin and End (both included), // assuming that the entire sequence is put on a single line. static unsigned CodePointsBetween(const FormatToken *Begin, @@ -92,8 +99,7 @@ static unsigned CodePointsBetween(const FormatToken *Begin, void CommaSeparatedList::precomputeFormattingInfos(const FormatToken *Token) { // FIXME: At some point we might want to do this for other lists, too. - if (!Token->MatchingParen || Token->isNot(tok::l_brace) || - Token->NestingLevel != 0) + if (!Token->MatchingParen || Token->isNot(tok::l_brace)) return; FormatToken *ItemBegin = Token->Next; @@ -103,7 +109,6 @@ 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()) @@ -143,6 +148,13 @@ void CommaSeparatedList::precomputeFormattingInfos(const FormatToken *Token) { ItemBegin = ItemEnd->Next; } + // If this doesn't have a nested list, we require at least 6 elements in order + // create a column layout. If it has a nested list, column layout ensures one + // list element per line. + if (HasNestedBracedList || Commas.size() < 5 || + Token->NestingLevel != 0) + return; + // We can never place more than ColumnLimit / 3 items in a row (because of the // spaces and the comma). for (unsigned Columns = 1; Columns <= Style.ColumnLimit / 3; ++Columns) { @@ -179,11 +191,6 @@ 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/lib/Format/FormatToken.h b/lib/Format/FormatToken.h index 391f9ee800..dff2942c00 100644 --- a/lib/Format/FormatToken.h +++ b/lib/Format/FormatToken.h @@ -398,10 +398,21 @@ public: /// \brief Apply the special formatting that the given role demands. /// + /// Assumes that the token having this role is already formatted. + /// /// Continues formatting from \p State leaving indentation to \p Indenter and /// returns the total penalty that this formatting incurs. - virtual unsigned format(LineState &State, ContinuationIndenter *Indenter, - bool DryRun) { + virtual unsigned formatFromToken(LineState &State, + ContinuationIndenter *Indenter, + bool DryRun) { + return 0; + } + + /// \brief Same as \c formatFromToken, but assumes that the first token has + /// already been set thereby deciding on the first line break. + virtual unsigned formatAfterToken(LineState &State, + ContinuationIndenter *Indenter, + bool DryRun) { return 0; } @@ -414,12 +425,17 @@ protected: class CommaSeparatedList : public TokenRole { public: - CommaSeparatedList(const FormatStyle &Style) : TokenRole(Style) {} + CommaSeparatedList(const FormatStyle &Style) + : TokenRole(Style), HasNestedBracedList(false) {} virtual void precomputeFormattingInfos(const FormatToken *Token); - virtual unsigned format(LineState &State, ContinuationIndenter *Indenter, - bool DryRun); + virtual unsigned formatAfterToken(LineState &State, + ContinuationIndenter *Indenter, + bool DryRun); + + virtual unsigned formatFromToken(LineState &State, + ContinuationIndenter *Indenter, bool DryRun); /// \brief Adds \p Token as the next comma to the \c CommaSeparated list. virtual void CommaFound(const FormatToken *Token) { Commas.push_back(Token); } @@ -454,6 +470,8 @@ private: /// \brief Precomputed formats that can be used for this list. SmallVector Formats; + + bool HasNestedBracedList; }; } // namespace format diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp index ea215ab757..432caf4774 100644 --- a/unittests/Format/FormatTest.cpp +++ b/unittests/Format/FormatTest.cpp @@ -1913,23 +1913,29 @@ TEST_F(FormatTest, NestedStaticInitializers) { " } };"); verifyFormat( - "SomeArrayOfSomeType a = { { { 1, 2, 3 }, { 1, 2, 3 },\n" + "SomeArrayOfSomeType a = { { { 1, 2, 3 },\n" + " { 1, 2, 3 },\n" " { 111111111111111111111111111111,\n" " 222222222222222222222222222222,\n" " 333333333333333333333333333333 },\n" - " { 1, 2, 3 }, { 1, 2, 3 } } };"); + " { 1, 2, 3 },\n" + " { 1, 2, 3 } } };"); verifyFormat( - "SomeArrayOfSomeType a = { { { 1, 2, 3 } }, { { 1, 2, 3 } },\n" + "SomeArrayOfSomeType a = { { { 1, 2, 3 } },\n" + " { { 1, 2, 3 } },\n" " { { 111111111111111111111111111111,\n" " 222222222222222222222222222222,\n" " 333333333333333333333333333333 } },\n" - " { { 1, 2, 3 } }, { { 1, 2, 3 } } };"); + " { { 1, 2, 3 } },\n" + " { { 1, 2, 3 } } };"); verifyGoogleFormat( "SomeArrayOfSomeType a = {\n" - " {{1, 2, 3}}, {{1, 2, 3}},\n" + " {{1, 2, 3}},\n" + " {{1, 2, 3}},\n" " {{111111111111111111111111111111, 222222222222222222222222222222,\n" " 333333333333333333333333333333}},\n" - " {{1, 2, 3}}, {{1, 2, 3}}};"); + " {{1, 2, 3}},\n" + " {{1, 2, 3}}};"); verifyFormat( "struct {\n" @@ -4938,16 +4944,23 @@ TEST_F(FormatTest, FormatsBracedListsInColumnLayout) { " { aaaaaaaaaaaaaaaaa } };", getLLVMStyleWithColumns(60)); - // No column layout for nested lists. + // With nested lists, we should either format one item per line or all nested + // lists one one line. // FIXME: For some nested lists, we can do better. verifyFormat( "SomeStruct my_struct_array = {\n" " { aaaaaa, aaaaaaaa, aaaaaaaaaa, aaaaaaaaa, aaaaaaaaa, aaaaaaaaaa,\n" " aaaaaaaaaaaaa, aaaaaaa, aaa },\n" + " { aaa, aaa },\n" + " { aaa, aaa },\n" " { aaaa, aaaa, aaaa, aaaa, aaaa, aaaa, aaaa, aaa },\n" " { aaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaa,\n" " aaaaaaaaaaaa, a, aaaaaaaaaa, aaaaaaaaa, aaa },\n" "};"); + + // No column layout should be used here. + verifyFormat("aaaaaaaaaaaaaaa = { aaaaaaaaaaaaaaaaaaaaaaaaaaa, 0, 0,\n" + " bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb };"); } TEST_F(FormatTest, PullTrivialFunctionDefinitionsIntoSingleLine) {