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.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@186003 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/include/clang/Format/Format.h b/include/clang/Format/Format.h
index ab3c1aa..f8786c8 100644
--- a/include/clang/Format/Format.h
+++ b/include/clang/Format/Format.h
@@ -77,6 +77,18 @@
/// 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 @@
ConstructorInitializerAllOnOneLineOrOnePerLine ==
R.ConstructorInitializerAllOnOneLineOrOnePerLine &&
DerivePointerBinding == R.DerivePointerBinding &&
+ ExperimentalAutoDetectBinPacking ==
+ R.ExperimentalAutoDetectBinPacking &&
IndentCaseLabels == R.IndentCaseLabels &&
IndentWidth == R.IndentWidth &&
MaxEmptyLinesToKeep == R.MaxEmptyLinesToKeep &&
diff --git a/lib/Format/Format.cpp b/lib/Format/Format.cpp
index b6b726c..ad390c2 100644
--- a/lib/Format/Format.cpp
+++ b/lib/Format/Format.cpp
@@ -94,6 +94,8 @@
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 @@
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 @@
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 @@
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 @@
} 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 @@
// to create a deterministic order independent of the container.
unsigned Count;
encoding::Encoding Encoding;
+ bool BinPackInconclusiveFunctions;
};
class FormatTokenLexer {
@@ -1363,7 +1374,8 @@
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 @@
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 @@
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 @@
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 @@
std::vector<AnnotatedLine> AnnotatedLines;
encoding::Encoding Encoding;
+ bool BinPackInconclusiveFunctions;
};
} // end anonymous namespace
diff --git a/lib/Format/FormatToken.h b/lib/Format/FormatToken.h
index 1de4b89..03f0970 100644
--- a/lib/Format/FormatToken.h
+++ b/lib/Format/FormatToken.h
@@ -64,6 +64,13 @@
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 @@
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 @@
/// 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;
diff --git a/lib/Format/TokenAnnotator.cpp b/lib/Format/TokenAnnotator.cpp
index ce2db6d..f5b6fbd 100644
--- a/lib/Format/TokenAnnotator.cpp
+++ b/lib/Format/TokenAnnotator.cpp
@@ -99,6 +99,8 @@
}
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 @@
}
}
+ 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 @@
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 @@
}
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 @@
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";
diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp
index 0192a07..b87393a 100644
--- a/unittests/Format/FormatTest.cpp
+++ b/unittests/Format/FormatTest.cpp
@@ -2644,6 +2644,30 @@
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"
