JVApen created this revision.
JVApen added a reviewer: djasper.
JVApen added a subscriber: cfe-commits.
JVApen set the repository for this revision to rL LLVM.
Herald added a subscriber: klimek.
Hi all,
I've been playing around with the clang for a while now and really enjoy it.
Unfortunately clang-format does not yet do what I like it to do, so I started
hacking it. So here is my first successful attempt to get something working.
The issue: ConstructorInitializerAllOnOneLineOrOnePerLine only works if 'If the
constructor initializers don’t fit on a line', while I prefer it to always
work. In other words, I use the following formatting:
```
Constructor()
: a(a)
, b(b)
```
Since everyone can benefit from upstreaming, I like to share my changes and get
some feedback.
Here is already some of the stuff which I was uncertain about:
- Should I keep ConstructorInitializerAllOnOneLineOrOnePerLine or rename it
(currently the second one)
- How to name the values, currently: Compact (old: false), BestFit (old:
true), OnePerLine (new)
- Is the back-ward compatibility in ScalarEnumerationTraits a good idea? (On
rename most likely not)
JVApen
Repository:
rL LLVM
http://reviews.llvm.org/D14484
Files:
docs/ClangFormatStyleOptions.rst
include/clang/Format/Format.h
lib/Format/ContinuationIndenter.cpp
lib/Format/Format.cpp
lib/Format/TokenAnnotator.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -3586,8 +3586,51 @@
" aaaaaaaaaaa(aaaaaaaaaaa),\n"
" aaaaaaaaaaaaaaaaaaaaat(aaaaaaaaaaaaaaaaaaaaaaaaaaaa) {}");
+ FormatStyle BestFit = getLLVMStyle();
+ BestFit.ConstructorInitializer = FormatStyle::CI_BestFit;
+ verifyFormat("SomeClass::Constructor()\n"
+ " : aaaaaaaaaaaaa(aaaaaaaaaaaaaa),\n"
+ " aaaaaaaaaaaaa(aaaaaaaaaaaaaa),\n"
+ " aaaaaaaaaaaaa(aaaaaaaaaaaaaa) {}",
+ BestFit);
+ verifyFormat("SomeClass::Constructor()\n"
+ " : aaaaaaaaaaaaa(aaaaaaaaaaaaaa), // Some comment\n"
+ " aaaaaaaaaaaaa(aaaaaaaaaaaaaa),\n"
+ " aaaaaaaaaaaaa(aaaaaaaaaaaaaa) {}",
+ BestFit);
+ verifyFormat("MyClass::MyClass(int var)\n"
+ " : some_var_(var), // 4 space indent\n"
+ " some_other_var_(var + 1) { // lined up\n"
+ "}",
+ BestFit);
+ verifyFormat("Constructor() : a(a), a(a) {}\n",
+ BestFit);
+ verifyFormat("Constructor()\n"
+ " : aaaaa(aaaaaa),\n"
+ " aaaaa(aaaaaa),\n"
+ " aaaaa(aaaaaa),\n"
+ " aaaaa(aaaaaa),\n"
+ " aaaaa(aaaaaa) {}",
+ BestFit);
+ verifyFormat("Constructor()\n"
+ " : aaaaa(aaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaa,\n"
+ " aaaaaaaaaaaaaaaaaaaaaa) {}",
+ BestFit);
+ BestFit.ColumnLimit = 60;
+ verifyFormat("Constructor()\n"
+ " : aaaaaaaaaaaaaaaaaaaa(a),\n"
+ " bbbbbbbbbbbbbbbbbbbbbbbb(b) {}",
+ BestFit);
+
+ EXPECT_EQ("Constructor()\n"
+ " : // Comment forcing unwanted break.\n"
+ " aaaa(aaaa) {}",
+ format("Constructor() :\n"
+ " // Comment forcing unwanted break.\n"
+ " aaaa(aaaa) {}"));
+
FormatStyle OnePerLine = getLLVMStyle();
- OnePerLine.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
+ OnePerLine.ConstructorInitializer = FormatStyle::CI_OnePerLine;
verifyFormat("SomeClass::Constructor()\n"
" : aaaaaaaaaaaaa(aaaaaaaaaaaaaa),\n"
" aaaaaaaaaaaaa(aaaaaaaaaaaaaa),\n"
@@ -3604,6 +3647,10 @@
"}",
OnePerLine);
verifyFormat("Constructor()\n"
+ " : a(a),\n"
+ " a(a) {}",
+ OnePerLine);
+ verifyFormat("Constructor()\n"
" : aaaaa(aaaaaa),\n"
" aaaaa(aaaaaa),\n"
" aaaaa(aaaaaa),\n"
@@ -3696,7 +3743,7 @@
// This test takes VERY long when memoization is broken.
FormatStyle OnePerLine = getLLVMStyle();
- OnePerLine.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
+ OnePerLine.ConstructorInitializer = FormatStyle::CI_BestFit;
OnePerLine.BinPackParameters = false;
std::string input = "Constructor()\n"
" : aaaa(a,\n";
@@ -9560,7 +9607,6 @@
CHECK_PARSE_BOOL(BinPackParameters);
CHECK_PARSE_BOOL(BreakBeforeTernaryOperators);
CHECK_PARSE_BOOL(BreakConstructorInitializersBeforeComma);
- CHECK_PARSE_BOOL(ConstructorInitializerAllOnOneLineOrOnePerLine);
CHECK_PARSE_BOOL(DerivePointerAlignment);
CHECK_PARSE_BOOL_FIELD(DerivePointerAlignment, "DerivePointerBinding");
CHECK_PARSE_BOOL(IndentCaseLabels);
@@ -9696,6 +9742,19 @@
CHECK_PARSE("SpaceAfterControlStatementKeyword: true", SpaceBeforeParens,
FormatStyle::SBPO_ControlStatements);
+ Style.ConstructorInitializer = FormatStyle::CI_BestFit;
+ CHECK_PARSE("ConstructorInitializer: Compact", ConstructorInitializer,
+ FormatStyle::CI_Compact);
+ CHECK_PARSE("ConstructorInitializer: BestFit", ConstructorInitializer,
+ FormatStyle::CI_BestFit);
+ CHECK_PARSE("ConstructorInitializer: OnePerLine", ConstructorInitializer,
+ FormatStyle::CI_OnePerLine);
+ // For backward compatibility:
+ CHECK_PARSE("ConstructorInitializer: false", ConstructorInitializer,
+ FormatStyle::CI_Compact);
+ CHECK_PARSE("ConstructorInitializer: true", ConstructorInitializer,
+ FormatStyle::CI_BestFit);
+
Style.ColumnLimit = 123;
FormatStyle BaseStyle = getLLVMStyle();
CHECK_PARSE("BasedOnStyle: LLVM", ColumnLimit, BaseStyle.ColumnLimit);
@@ -10094,7 +10153,7 @@
", c(c) {}",
Style);
- Style.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
+ Style.ConstructorInitializer = FormatStyle::CI_BestFit;
Style.ConstructorInitializerIndentWidth = 4;
verifyFormat("SomeClass::Constructor() : aaaaaaaa(aaaaaaaa) {}", Style);
verifyFormat(
@@ -10111,6 +10170,29 @@
" , aaaaaaaa(aaaaaaaa)\n"
" , aaaaaaaa(aaaaaaaa) {}",
Style);
+
+ Style.ConstructorInitializer = FormatStyle::CI_OnePerLine;
+ Style.ConstructorInitializerIndentWidth = 4;
+ verifyFormat("SomeClass::Constructor()\n"
+ " : aaaaaaaa(aaaaaaaa) {}",
+ Style);
+ verifyFormat("SomeClass::Constructor()\n"
+ " : aaaaa(aaaaa)\n"
+ " , aaaaa(aaaaa)\n"
+ " , aaaaa(aaaaa)\n",
+ Style);
+ verifyFormat("SomeClass::Constructor()\n"
+ " : aaaaaaaa(aaaaaaaa)\n"
+ " , aaaaaaaa(aaaaaaaa)\n"
+ " , aaaaaaaa(aaaaaaaa) {}",
+ Style);
+ Style.ConstructorInitializerIndentWidth = 4;
+ Style.ColumnLimit = 60;
+ verifyFormat("SomeClass::Constructor()\n"
+ " : aaaaaaaa(aaaaaaaa)\n"
+ " , aaaaaaaa(aaaaaaaa)\n"
+ " , aaaaaaaa(aaaaaaaa) {}",
+ Style);
}
TEST_F(FormatTest, Destructors) {
Index: lib/Format/TokenAnnotator.cpp
===================================================================
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2146,9 +2146,12 @@
Right.Previous->MatchingParen->NestingLevel == 0 &&
Style.AlwaysBreakTemplateDeclarations)
return true;
+ if (Right.is(TT_CtorInitializerColon) &&
+ Style.ConstructorInitializer == FormatStyle::CI_OnePerLine)
+ return true;
if ((Right.isOneOf(TT_CtorInitializerComma, TT_CtorInitializerColon)) &&
- Style.BreakConstructorInitializersBeforeComma &&
- !Style.ConstructorInitializerAllOnOneLineOrOnePerLine)
+ (Style.BreakConstructorInitializersBeforeComma &&
+ Style.ConstructorInitializer == FormatStyle::CI_Compact))
return true;
if (Right.is(tok::string_literal) && Right.TokenText.startswith("R\""))
// Raw string literals are special wrt. line breaks. The author has made a
Index: lib/Format/Format.cpp
===================================================================
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -167,6 +167,21 @@
}
};
+
+template <>
+struct ScalarEnumerationTraits<FormatStyle::ConstructorInitializerKind> {
+ static void enumeration(IO &IO,
+ FormatStyle::ConstructorInitializerKind &Value) {
+ IO.enumCase(Value, "Compact", FormatStyle::CI_Compact);
+ IO.enumCase(Value, "BestFit", FormatStyle::CI_BestFit);
+ IO.enumCase(Value, "OnePerLine", FormatStyle::CI_OnePerLine);
+
+ // For backward compatibility.
+ IO.enumCase(Value, "false", FormatStyle::CI_Compact);
+ IO.enumCase(Value, "true", FormatStyle::CI_BestFit);
+ }
+};
+
template <> struct MappingTraits<FormatStyle> {
static void mapping(IO &IO, FormatStyle &Style) {
// When reading, read the language first, we need it for getPredefinedStyle.
@@ -249,8 +264,7 @@
Style.BreakConstructorInitializersBeforeComma);
IO.mapOptional("ColumnLimit", Style.ColumnLimit);
IO.mapOptional("CommentPragmas", Style.CommentPragmas);
- IO.mapOptional("ConstructorInitializerAllOnOneLineOrOnePerLine",
- Style.ConstructorInitializerAllOnOneLineOrOnePerLine);
+ IO.mapOptional("ConstructorInitializer", Style.ConstructorInitializer);
IO.mapOptional("ConstructorInitializerIndentWidth",
Style.ConstructorInitializerIndentWidth);
IO.mapOptional("ContinuationIndentWidth", Style.ContinuationIndentWidth);
@@ -463,7 +477,7 @@
LLVMStyle.BreakAfterJavaFieldAnnotations = false;
LLVMStyle.ColumnLimit = 80;
LLVMStyle.CommentPragmas = "^ IWYU pragma:";
- LLVMStyle.ConstructorInitializerAllOnOneLineOrOnePerLine = false;
+ LLVMStyle.ConstructorInitializer = FormatStyle::CI_Compact;
LLVMStyle.ConstructorInitializerIndentWidth = 4;
LLVMStyle.ContinuationIndentWidth = 4;
LLVMStyle.Cpp11BracedListStyle = true;
@@ -521,7 +535,7 @@
GoogleStyle.AllowShortLoopsOnASingleLine = true;
GoogleStyle.AlwaysBreakBeforeMultilineStrings = true;
GoogleStyle.AlwaysBreakTemplateDeclarations = true;
- GoogleStyle.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
+ GoogleStyle.ConstructorInitializer = FormatStyle::CI_BestFit;
GoogleStyle.DerivePointerAlignment = true;
GoogleStyle.IncludeCategories = {{"^<.*\\.h>", 1}, {"^<.*", 2}, {".*", 3}};
GoogleStyle.IndentCaseLabels = true;
Index: lib/Format/ContinuationIndenter.cpp
===================================================================
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -708,9 +708,10 @@
State.Stack.back().Indent =
State.Column + (Style.BreakConstructorInitializersBeforeComma ? 0 : 2);
State.Stack.back().NestedBlockIndent = State.Stack.back().Indent;
- if (Style.ConstructorInitializerAllOnOneLineOrOnePerLine)
+ if (Style.ConstructorInitializer != FormatStyle::CI_Compact)
State.Stack.back().AvoidBinPacking = true;
- State.Stack.back().BreakBeforeParameter = false;
+ State.Stack.back().BreakBeforeParameter =
+ (Style.ConstructorInitializer == FormatStyle::CI_OnePerLine);
}
if (Current.isOneOf(TT_BinaryOperator, TT_ConditionalExpr) && Newline)
State.Stack.back().NestedBlockIndent =
Index: include/clang/Format/Format.h
===================================================================
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -269,10 +269,16 @@
/// which should not be split into lines or otherwise changed.
std::string CommentPragmas;
- /// \brief If the constructor initializers don't fit on a line, put each
- /// initializer on its own line.
- bool ConstructorInitializerAllOnOneLineOrOnePerLine;
+ enum ConstructorInitializerKind {
+ CI_Compact, ///< Put all initializers in a compact block
+ CI_BestFit, ///< Use the compact if all fits on a single line,
+ ///< otherwise put one per line.
+ CI_OnePerLine, ///< Put all initializers on their own line
+ };
+ /// \brief The placement of the constructor initializers.
+ ConstructorInitializerKind ConstructorInitializer;
+
/// \brief The number of characters to use for indentation of constructor
/// initializer lists.
unsigned ConstructorInitializerIndentWidth;
@@ -584,8 +590,7 @@
R.BreakConstructorInitializersBeforeComma &&
BreakAfterJavaFieldAnnotations == R.BreakAfterJavaFieldAnnotations &&
ColumnLimit == R.ColumnLimit && CommentPragmas == R.CommentPragmas &&
- ConstructorInitializerAllOnOneLineOrOnePerLine ==
- R.ConstructorInitializerAllOnOneLineOrOnePerLine &&
+ ConstructorInitializer == R.ConstructorInitializer &&
ConstructorInitializerIndentWidth ==
R.ConstructorInitializerIndentWidth &&
ContinuationIndentWidth == R.ContinuationIndentWidth &&
Index: docs/ClangFormatStyleOptions.rst
===================================================================
--- docs/ClangFormatStyleOptions.rst
+++ docs/ClangFormatStyleOptions.rst
@@ -369,10 +369,21 @@
A regular expression that describes comments with special meaning,
which should not be split into lines or otherwise changed.
-**ConstructorInitializerAllOnOneLineOrOnePerLine** (``bool``)
- If the constructor initializers don't fit on a line, put each
- initializer on its own line.
+**ConstructorInitializer** (``ConstructorInitializerKind``)
+ Formatting of the constructor initializer.
+ Possible values:
+
+ * ``CI_Compact`` (in configuration: ``Compact``)
+ Puts all the initializers in a compact block of multiple lines with
+ multiple initializers on the same line.
+ * ``CI_BestFit`` (in configuration: ``BestFit``)
+ Eiter all initializers are on a single line (it this fits),
+ otherwise every initializer has its own line.
+ * ``CI_OnePerLine`` (in configuration: ``OnePerLine``)
+ Always put the initializers on their own line, regardless if they would fit
+ on a single line.
+
**ConstructorInitializerIndentWidth** (``unsigned``)
The number of characters to use for indentation of constructor
initializer lists.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
