Add experimental flag for adaptive parameter bin-packing.

This is not activated for any style, might change or go away
completely.

For those that want to play around with it, set
ExperimentalAutoDetectBinPacking to true.

clang-format will then:
Look at whether function calls/declarations/definitions are currently
formatted with one parameter per line (on a case-by-case basis). If so,
clang-format will avoid bin-packing the parameters. If all parameters
are on one line (thus that line is "inconclusive"), clang-format will
make the choice dependent on whether there are other bin-packed
calls/declarations in the same file.

The reason for this change is that bin-packing in some situations can be
really bad and an author might opt to put one parameter on each line. If
the author does that, he might want clang-format not to mess with that.
If the author is unhappy with the one-per-line formatting, clang-format
can easily be convinced to bin-pack by putting any two parameters on the
same line.

llvm-svn: 186003
This commit is contained in:
Daniel Jasper 2013-07-10 14:02:49 +00:00
parent 123fdb3413
commit b10cbc45ad
5 changed files with 100 additions and 12 deletions

View File

@ -77,6 +77,18 @@ struct FormatStyle {
/// will either all be on the same line or will have one line each.
bool BinPackParameters;
/// \brief If true, clang-format detects whether function calls and
/// definitions are formatted with one parameter per line.
///
/// Each call can be bin-packed, one-per-line or inconclusive. If it is
/// inconclusive, e.g. completely on one line, but a decision needs to be
/// made, clang-format analyzes whether there are other bin-packed cases in
/// the input file and act accordingly.
///
/// NOTE: This is an experimental flag, that might go away or be renamed. Do
/// not use this in config files, etc. Use at your own risk.
bool ExperimentalAutoDetectBinPacking;
/// \brief Allow putting all parameters of a function declaration onto
/// the next line even if \c BinPackParameters is \c false.
bool AllowAllParametersOfDeclarationOnNextLine;
@ -155,6 +167,8 @@ struct FormatStyle {
ConstructorInitializerAllOnOneLineOrOnePerLine ==
R.ConstructorInitializerAllOnOneLineOrOnePerLine &&
DerivePointerBinding == R.DerivePointerBinding &&
ExperimentalAutoDetectBinPacking ==
R.ExperimentalAutoDetectBinPacking &&
IndentCaseLabels == R.IndentCaseLabels &&
IndentWidth == R.IndentWidth &&
MaxEmptyLinesToKeep == R.MaxEmptyLinesToKeep &&

View File

@ -94,6 +94,8 @@ template <> struct MappingTraits<clang::format::FormatStyle> {
IO.mapOptional("ConstructorInitializerAllOnOneLineOrOnePerLine",
Style.ConstructorInitializerAllOnOneLineOrOnePerLine);
IO.mapOptional("DerivePointerBinding", Style.DerivePointerBinding);
IO.mapOptional("ExperimentalAutoDetectBinPacking",
Style.ExperimentalAutoDetectBinPacking);
IO.mapOptional("IndentCaseLabels", Style.IndentCaseLabels);
IO.mapOptional("MaxEmptyLinesToKeep", Style.MaxEmptyLinesToKeep);
IO.mapOptional("ObjCSpaceBeforeProtocolList",
@ -134,6 +136,7 @@ FormatStyle getLLVMStyle() {
LLVMStyle.ColumnLimit = 80;
LLVMStyle.ConstructorInitializerAllOnOneLineOrOnePerLine = false;
LLVMStyle.DerivePointerBinding = false;
LLVMStyle.ExperimentalAutoDetectBinPacking = false;
LLVMStyle.IndentCaseLabels = false;
LLVMStyle.MaxEmptyLinesToKeep = 1;
LLVMStyle.ObjCSpaceBeforeProtocolList = true;
@ -165,6 +168,7 @@ FormatStyle getGoogleStyle() {
GoogleStyle.ColumnLimit = 80;
GoogleStyle.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
GoogleStyle.DerivePointerBinding = true;
GoogleStyle.ExperimentalAutoDetectBinPacking = false;
GoogleStyle.IndentCaseLabels = true;
GoogleStyle.MaxEmptyLinesToKeep = 1;
GoogleStyle.ObjCSpaceBeforeProtocolList = false;
@ -260,10 +264,12 @@ public:
const AnnotatedLine &Line, unsigned FirstIndent,
const FormatToken *RootToken,
WhitespaceManager &Whitespaces,
encoding::Encoding Encoding)
encoding::Encoding Encoding,
bool BinPackInconclusiveFunctions)
: Style(Style), SourceMgr(SourceMgr), Line(Line),
FirstIndent(FirstIndent), RootToken(RootToken),
Whitespaces(Whitespaces), Count(0), Encoding(Encoding) {}
Whitespaces(Whitespaces), Count(0), Encoding(Encoding),
BinPackInconclusiveFunctions(BinPackInconclusiveFunctions) {}
/// \brief Formats an \c UnwrappedLine.
void format(const AnnotatedLine *NextLine) {
@ -782,7 +788,11 @@ private:
} else {
NewIndent =
4 + std::max(LastSpace, State.Stack.back().StartOfFunctionCall);
AvoidBinPacking = !Style.BinPackParameters;
AvoidBinPacking = !Style.BinPackParameters ||
(Style.ExperimentalAutoDetectBinPacking &&
(Current.PackingKind == PPK_OnePerLine ||
(!BinPackInconclusiveFunctions &&
Current.PackingKind == PPK_Inconclusive)));
}
State.Stack.push_back(ParenState(NewIndent, LastSpace, AvoidBinPacking,
@ -1157,6 +1167,7 @@ private:
// to create a deterministic order independent of the container.
unsigned Count;
encoding::Encoding Encoding;
bool BinPackInconclusiveFunctions;
};
class FormatTokenLexer {
@ -1363,7 +1374,8 @@ public:
1;
}
UnwrappedLineFormatter Formatter(Style, SourceMgr, TheLine, Indent,
TheLine.First, Whitespaces, Encoding);
TheLine.First, Whitespaces, Encoding,
BinPackInconclusiveFunctions);
Formatter.format(I + 1 != E ? &*(I + 1) : NULL);
IndentForLevel[TheLine.Level] = LevelIndent;
PreviousLineWasTouched = true;
@ -1408,6 +1420,8 @@ private:
unsigned CountBoundToVariable = 0;
unsigned CountBoundToType = 0;
bool HasCpp03IncompatibleFormat = false;
bool HasBinPackedFunction = false;
bool HasOnePerLineFunction = false;
for (unsigned i = 0, e = AnnotatedLines.size(); i != e; ++i) {
if (!AnnotatedLines[i].First->Next)
continue;
@ -1428,6 +1442,12 @@ private:
Tok->Previous->Type == TT_TemplateCloser &&
Tok->WhitespaceRange.getBegin() == Tok->WhitespaceRange.getEnd())
HasCpp03IncompatibleFormat = true;
if (Tok->PackingKind == PPK_BinPacked)
HasBinPackedFunction = true;
if (Tok->PackingKind == PPK_OnePerLine)
HasOnePerLineFunction = true;
Tok = Tok->Next;
}
}
@ -1441,6 +1461,8 @@ private:
Style.Standard = HasCpp03IncompatibleFormat ? FormatStyle::LS_Cpp11
: FormatStyle::LS_Cpp03;
}
BinPackInconclusiveFunctions =
HasBinPackedFunction || !HasOnePerLineFunction;
}
/// \brief Get the indent of \p Level from \p IndentForLevel.
@ -1691,6 +1713,7 @@ private:
std::vector<AnnotatedLine> AnnotatedLines;
encoding::Encoding Encoding;
bool BinPackInconclusiveFunctions;
};
} // end anonymous namespace

View File

@ -64,6 +64,13 @@ enum BraceBlockKind {
BK_BracedInit
};
// The packing kind of a function's parameters.
enum ParameterPackingKind {
PPK_BinPacked,
PPK_OnePerLine,
PPK_Inconclusive
};
/// \brief A wrapper around a \c Token storing information about the
/// whitespace characters preceeding it.
struct FormatToken {
@ -72,9 +79,9 @@ struct FormatToken {
CodePointCount(0), IsFirst(false), MustBreakBefore(false),
BlockKind(BK_Unknown), Type(TT_Unknown), SpacesRequiredBefore(0),
CanBreakBefore(false), ClosesTemplateDeclaration(false),
ParameterCount(0), TotalLength(0), UnbreakableTailLength(0),
BindingStrength(0), SplitPenalty(0), LongestObjCSelectorName(0),
FakeRParens(0), LastInChainOfCalls(false),
ParameterCount(0), PackingKind(PPK_Inconclusive), TotalLength(0),
UnbreakableTailLength(0), BindingStrength(0), SplitPenalty(0),
LongestObjCSelectorName(0), FakeRParens(0), LastInChainOfCalls(false),
PartOfMultiVariableDeclStmt(false), MatchingParen(NULL), Previous(NULL),
Next(NULL) {}
@ -143,6 +150,9 @@ struct FormatToken {
/// the number of commas.
unsigned ParameterCount;
/// \brief If this is an opening parenthesis, how are the parameters packed?
ParameterPackingKind PackingKind;
/// \brief The total length of the line up to and including this token.
unsigned TotalLength;

View File

@ -99,6 +99,8 @@ private:
}
bool MightBeFunctionType = CurrentToken->is(tok::star);
bool HasMultipleLines = false;
bool HasMultipleParametersOnALine = false;
while (CurrentToken != NULL) {
// LookForDecls is set when "if (" has been seen. Check for
// 'identifier' '*' 'identifier' followed by not '=' -- this
@ -133,6 +135,13 @@ private:
}
}
if (!HasMultipleLines)
Left->PackingKind = PPK_Inconclusive;
else if (HasMultipleParametersOnALine)
Left->PackingKind = PPK_BinPacked;
else
Left->PackingKind = PPK_OnePerLine;
next();
return true;
}
@ -143,8 +152,14 @@ private:
tok::coloncolon))
MightBeFunctionType = true;
updateParameterCount(Left, CurrentToken);
if (CurrentToken->is(tok::comma) && CurrentToken->Next &&
!CurrentToken->Next->HasUnescapedNewline &&
!CurrentToken->Next->isTrailingComment())
HasMultipleParametersOnALine = true;
if (!consumeToken())
return false;
if (CurrentToken && CurrentToken->HasUnescapedNewline)
HasMultipleLines = true;
}
return false;
}
@ -245,10 +260,11 @@ private:
}
void updateParameterCount(FormatToken *Left, FormatToken *Current) {
if (Current->is(tok::comma))
if (Current->is(tok::comma)) {
++Left->ParameterCount;
else if (Left->ParameterCount == 0 && Current->isNot(tok::comment))
} else if (Left->ParameterCount == 0 && Current->isNot(tok::comment)) {
Left->ParameterCount = 1;
}
}
bool parseConditional() {
@ -1283,9 +1299,10 @@ void TokenAnnotator::printDebugInfo(const AnnotatedLine &Line) {
const FormatToken *Tok = Line.First;
while (Tok) {
llvm::errs() << " M=" << Tok->MustBreakBefore
<< " C=" << Tok->CanBreakBefore << " T=" << Tok->Type << " S="
<< Tok->SpacesRequiredBefore << " P=" << Tok->SplitPenalty
<< " Name=" << Tok->Tok.getName() << " FakeLParens=";
<< " C=" << Tok->CanBreakBefore << " T=" << Tok->Type
<< " S=" << Tok->SpacesRequiredBefore
<< " P=" << Tok->SplitPenalty << " Name=" << Tok->Tok.getName()
<< " PPK=" << Tok->PackingKind << " FakeLParens=";
for (unsigned i = 0, e = Tok->FakeLParens.size(); i != e; ++i)
llvm::errs() << Tok->FakeLParens[i] << "/";
llvm::errs() << " FakeRParens=" << Tok->FakeRParens << "\n";

View File

@ -2644,6 +2644,30 @@ TEST_F(FormatTest, FormatsOneParameterPerLineIfNecessary) {
NoBinPacking);
}
TEST_F(FormatTest, AdaptiveOnePerLineFormatting) {
FormatStyle Style = getLLVMStyleWithColumns(15);
Style.ExperimentalAutoDetectBinPacking = true;
EXPECT_EQ("aaa(aaaa,\n"
" aaaa,\n"
" aaaa);\n"
"aaa(aaaa,\n"
" aaaa,\n"
" aaaa);",
format("aaa(aaaa,\n" // one-per-line
" aaaa,\n"
" aaaa );\n"
"aaa(aaaa, aaaa, aaaa);", // inconclusive
Style));
EXPECT_EQ("aaa(aaaa, aaaa,\n"
" aaaa);\n"
"aaa(aaaa, aaaa,\n"
" aaaa);",
format("aaa(aaaa, aaaa,\n" // bin-packed
" aaaa );\n"
"aaa(aaaa, aaaa, aaaa);", // inconclusive
Style));
}
TEST_F(FormatTest, FormatsBuilderPattern) {
verifyFormat(
"return llvm::StringSwitch<Reference::Kind>(name)\n"