r198869 - clang-format: Some tweaks to braces list formatting:
Daniel Jasper
djasper at google.com
Thu Jan 9 05:42:56 PST 2014
Author: djasper
Date: Thu Jan 9 07:42:56 2014
New Revision: 198869
URL: http://llvm.org/viewvc/llvm-project?rev=198869&view=rev
Log:
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).
Modified:
cfe/trunk/lib/Format/ContinuationIndenter.cpp
cfe/trunk/lib/Format/FormatToken.cpp
cfe/trunk/lib/Format/FormatToken.h
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=198869&r1=198868&r2=198869&view=diff
==============================================================================
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Thu Jan 9 07:42:56 2014
@@ -721,13 +721,15 @@ unsigned ContinuationIndenter::moveState
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;
}
Modified: cfe/trunk/lib/Format/FormatToken.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatToken.cpp?rev=198869&r1=198868&r2=198869&view=diff
==============================================================================
--- cfe/trunk/lib/Format/FormatToken.cpp (original)
+++ cfe/trunk/lib/Format/FormatToken.cpp Thu Jan 9 07:42:56 2014
@@ -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(Line
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
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::precomputeForma
// trailing comments which are otherwise ignored for column alignment.
SmallVector<unsigned, 8> 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::precomputeForma
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::precomputeForma
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);
}
}
Modified: cfe/trunk/lib/Format/FormatToken.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatToken.h?rev=198869&r1=198868&r2=198869&view=diff
==============================================================================
--- cfe/trunk/lib/Format/FormatToken.h (original)
+++ cfe/trunk/lib/Format/FormatToken.h Thu Jan 9 07:42:56 2014
@@ -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<ColumnFormat, 4> Formats;
+
+ bool HasNestedBracedList;
};
} // namespace format
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=198869&r1=198868&r2=198869&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Thu Jan 9 07:42:56 2014
@@ -1913,23 +1913,29 @@ TEST_F(FormatTest, NestedStaticInitializ
" } };");
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, FormatsBracedListsInC
" { 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) {
More information about the cfe-commits
mailing list