Index: include/clang/Format/Format.h =================================================================== --- include/clang/Format/Format.h +++ include/clang/Format/Format.h @@ -693,16 +693,35 @@ /// \endcode bool BreakBeforeTernaryOperators; - /// \brief Always break constructor initializers before commas and align - /// the commas with the colon. - /// \code - /// true: false: - /// SomeClass::Constructor() vs. SomeClass::Constructor() : a(a), - /// : a(a) b(b), - /// , b(b) c(c) {} - /// , c(c) {} - /// \endcode - bool BreakConstructorInitializersBeforeComma; + /// \brief Different ways to break initializers. + enum BreakConstructorInitializersStyle + { + /// Break constructor initializers before the colon and after the commas. + /// \code + /// Constructor() + /// : initializer1(), + /// initializer2() + /// \endcode + BCIS_BeforeColon, + /// Break constructor initializers before the colon and commas, and align + /// the commas with the colon. + /// \code + /// Constructor() + /// : initializer1() + /// , initializer2() + /// \endcode + BCIS_BeforeComma, + /// Break constructor initializers after the colon and commas. + /// \code + /// Constructor() : + /// initializer1(), + /// initializer2() + /// \endcode + BCIS_AfterColon + }; + + /// \brief The constructor initializers style to use.. + BreakConstructorInitializersStyle BreakConstructorInitializers; /// \brief Break after each annotation on a field in Java files. /// \code{.java} @@ -1370,8 +1389,7 @@ BreakBeforeBinaryOperators == R.BreakBeforeBinaryOperators && BreakBeforeBraces == R.BreakBeforeBraces && BreakBeforeTernaryOperators == R.BreakBeforeTernaryOperators && - BreakConstructorInitializersBeforeComma == - R.BreakConstructorInitializersBeforeComma && + BreakConstructorInitializers == R.BreakConstructorInitializers && BreakAfterJavaFieldAnnotations == R.BreakAfterJavaFieldAnnotations && BreakStringLiterals == R.BreakStringLiterals && ColumnLimit == R.ColumnLimit && CommentPragmas == R.CommentPragmas && Index: lib/Format/ContinuationIndenter.cpp =================================================================== --- lib/Format/ContinuationIndenter.cpp +++ lib/Format/ContinuationIndenter.cpp @@ -54,13 +54,14 @@ const FormatStyle &Style) { const FormatToken &Previous = *Current.Previous; if (Current.is(TT_CtorInitializerComma) && - Style.BreakConstructorInitializersBeforeComma) + Style.BreakConstructorInitializers == FormatStyle::BCIS_BeforeComma) return true; return Previous.is(tok::comma) && !Current.isTrailingComment() && ((Previous.isNot(TT_CtorInitializerComma) || - !Style.BreakConstructorInitializersBeforeComma) && + Style.BreakConstructorInitializers != + FormatStyle::BCIS_BeforeComma) && (Previous.isNot(TT_InheritanceComma) || - !Style.BreakBeforeInheritanceComma)); + !Style.BreakBeforeInheritanceComma)); } ContinuationIndenter::ContinuationIndenter(const FormatStyle &Style, @@ -179,11 +180,19 @@ getColumnLimit(State)) return true; if (Current.is(TT_CtorInitializerColon) && + Style.BreakConstructorInitializers != FormatStyle::BCIS_AfterColon && (State.Column + State.Line->Last->TotalLength - Current.TotalLength + 2 > getColumnLimit(State) || State.Stack.back().BreakBeforeParameter) && - ((Style.AllowShortFunctionsOnASingleLine != FormatStyle::SFS_All) || - Style.BreakConstructorInitializersBeforeComma || Style.ColumnLimit != 0)) + (Style.AllowShortFunctionsOnASingleLine != FormatStyle::SFS_All || + Style.BreakConstructorInitializers == FormatStyle::BCIS_BeforeComma || + Style.ColumnLimit != 0)) + return true; + if (Previous.is(TT_CtorInitializerColon) && + Style.BreakConstructorInitializers == FormatStyle::BCIS_AfterColon && + (State.Column + State.Line->Last->TotalLength - Previous.TotalLength > + getColumnLimit(State) || + State.Stack.back().BreakBeforeParameter)) return true; if (Current.is(TT_ObjCMethodExpr) && !Previous.is(TT_SelectorName) && State.Line->startsWith(TT_ObjCMethodSpecifier)) @@ -455,6 +464,11 @@ !Previous.is(TT_OverloadedOperator)) || (Previous.is(tok::colon) && Previous.is(TT_ObjCMethodExpr)))) { State.Stack.back().LastSpace = State.Column; + } else if (Previous.is(TT_CtorInitializerColon) && + Style.BreakConstructorInitializers == + FormatStyle::BCIS_AfterColon) { + State.Stack.back().Indent = State.Column; + State.Stack.back().LastSpace = State.Column; } else if ((Previous.isOneOf(TT_BinaryOperator, TT_ConditionalExpr, TT_CtorInitializerColon)) && ((Previous.getPrecedence() != prec::Assignment && @@ -612,7 +626,7 @@ State.Stack[i].BreakBeforeParameter = true; if (PreviousNonComment && - !PreviousNonComment->isOneOf(tok::comma, tok::semi) && + !PreviousNonComment->isOneOf(tok::comma, tok::colon, tok::semi) && (PreviousNonComment->isNot(TT_TemplateCloser) || Current.NestingLevel != 0) && !PreviousNonComment->isOneOf( @@ -746,6 +760,9 @@ return ContinuationIndent; if (NextNonComment->is(TT_CtorInitializerComma)) return State.Stack.back().Indent; + if (PreviousNonComment && PreviousNonComment->is(TT_CtorInitializerColon) && + Style.BreakConstructorInitializers == FormatStyle::BCIS_AfterColon) + return State.Stack.back().Indent; if (NextNonComment->isOneOf(TT_CtorInitializerColon, TT_InheritanceColon, TT_InheritanceComma)) return State.FirstIndent + Style.ConstructorInitializerIndentWidth; @@ -805,19 +822,29 @@ State.FirstIndent + Style.ContinuationIndentWidth; } } - if (Current.is(TT_CtorInitializerColon)) { + if (Current.is(TT_CtorInitializerColon) && + Style.BreakConstructorInitializers != FormatStyle::BCIS_AfterColon) { // Indent 2 from the column, so: // SomeClass::SomeClass() // : First(...), ... // Next(...) // ^ line up here. State.Stack.back().Indent = - State.Column + (Style.BreakConstructorInitializersBeforeComma ? 0 : 2); + State.Column + (Style.BreakConstructorInitializers == + FormatStyle::BCIS_BeforeComma ? 0 : 2); State.Stack.back().NestedBlockIndent = State.Stack.back().Indent; if (Style.ConstructorInitializerAllOnOneLineOrOnePerLine) State.Stack.back().AvoidBinPacking = true; State.Stack.back().BreakBeforeParameter = false; } + if (Current.is(TT_CtorInitializerColon) && + Style.BreakConstructorInitializers == FormatStyle::BCIS_AfterColon) { + State.Stack.back().Indent = + State.FirstIndent + Style.ConstructorInitializerIndentWidth; + State.Stack.back().NestedBlockIndent = State.Stack.back().Indent; + if (Style.ConstructorInitializerAllOnOneLineOrOnePerLine) + State.Stack.back().AvoidBinPacking = true; + } if (Current.is(TT_InheritanceColon)) State.Stack.back().Indent = State.FirstIndent + Style.ContinuationIndentWidth; Index: lib/Format/Format.cpp =================================================================== --- lib/Format/Format.cpp +++ lib/Format/Format.cpp @@ -123,6 +123,14 @@ } }; +template <> struct ScalarEnumerationTraits { + static void enumeration(IO &IO, FormatStyle::BreakConstructorInitializersStyle &Value) { + IO.enumCase(Value, "BeforeColon", FormatStyle::BCIS_BeforeColon); + IO.enumCase(Value, "BeforeComma", FormatStyle::BCIS_BeforeComma); + IO.enumCase(Value, "AfterColon", FormatStyle::BCIS_AfterColon); + } +}; + template <> struct ScalarEnumerationTraits { static void enumeration(IO &IO, FormatStyle::ReturnTypeBreakingStyle &Value) { @@ -291,8 +299,19 @@ IO.mapOptional("BreakBeforeBraces", Style.BreakBeforeBraces); IO.mapOptional("BreakBeforeTernaryOperators", Style.BreakBeforeTernaryOperators); + + bool BreakConstructorInitializersBeforeComma = false; IO.mapOptional("BreakConstructorInitializersBeforeComma", - Style.BreakConstructorInitializersBeforeComma); + BreakConstructorInitializersBeforeComma); + IO.mapOptional("BreakConstructorInitializers", + Style.BreakConstructorInitializers); + // If BreakConstructorInitializersBeforeComma was specified but + // BreakConstructorInitializers was not, initialize the latter from the + // former for backwards compatibility. + if (BreakConstructorInitializersBeforeComma && + Style.BreakConstructorInitializers == FormatStyle::BCIS_BeforeColon) + Style.BreakConstructorInitializers = FormatStyle::BCIS_BeforeComma; + IO.mapOptional("BreakAfterJavaFieldAnnotations", Style.BreakAfterJavaFieldAnnotations); IO.mapOptional("BreakStringLiterals", Style.BreakStringLiterals); @@ -522,7 +541,7 @@ LLVMStyle.BraceWrapping = {false, false, false, false, false, false, false, false, false, false, false}; LLVMStyle.BreakAfterJavaFieldAnnotations = false; - LLVMStyle.BreakConstructorInitializersBeforeComma = false; + LLVMStyle.BreakConstructorInitializers = FormatStyle::BCIS_BeforeColon; LLVMStyle.BreakBeforeInheritanceComma = false; LLVMStyle.BreakStringLiterals = true; LLVMStyle.ColumnLimit = 80; @@ -677,7 +696,7 @@ MozillaStyle.BinPackParameters = false; MozillaStyle.BinPackArguments = false; MozillaStyle.BreakBeforeBraces = FormatStyle::BS_Mozilla; - MozillaStyle.BreakConstructorInitializersBeforeComma = true; + MozillaStyle.BreakConstructorInitializers = FormatStyle::BCIS_BeforeComma; MozillaStyle.BreakBeforeInheritanceComma = true; MozillaStyle.ConstructorInitializerIndentWidth = 2; MozillaStyle.ContinuationIndentWidth = 2; @@ -700,7 +719,7 @@ Style.AlignTrailingComments = false; Style.BreakBeforeBinaryOperators = FormatStyle::BOS_All; Style.BreakBeforeBraces = FormatStyle::BS_WebKit; - Style.BreakConstructorInitializersBeforeComma = true; + Style.BreakConstructorInitializers = FormatStyle::BCIS_BeforeComma; Style.Cpp11BracedListStyle = false; Style.ColumnLimit = 0; Style.FixNamespaceComments = false; Index: lib/Format/TokenAnnotator.cpp =================================================================== --- lib/Format/TokenAnnotator.cpp +++ lib/Format/TokenAnnotator.cpp @@ -1984,7 +1984,7 @@ if (Left.is(tok::comment)) return 1000; - if (Left.isOneOf(TT_RangeBasedForLoopColon, TT_InheritanceColon)) + if (Left.isOneOf(TT_RangeBasedForLoopColon, TT_InheritanceColon, TT_CtorInitializerColon)) return 2; if (Right.isMemberAccess()) { @@ -2490,8 +2490,12 @@ Right.Previous->MatchingParen->NestingLevel == 0 && Style.AlwaysBreakTemplateDeclarations) return true; - if ((Right.isOneOf(TT_CtorInitializerComma, TT_CtorInitializerColon)) && - Style.BreakConstructorInitializersBeforeComma && + if (Right.is(TT_CtorInitializerComma) && + Style.BreakConstructorInitializers == FormatStyle::BCIS_BeforeComma && + !Style.ConstructorInitializerAllOnOneLineOrOnePerLine) + return true; + if (Right.is(TT_CtorInitializerColon) && + Style.BreakConstructorInitializers == FormatStyle::BCIS_BeforeComma && !Style.ConstructorInitializerAllOnOneLineOrOnePerLine) return true; // Break only if we have multiple inheritance. @@ -2599,7 +2603,10 @@ // The first comment in a braced lists is always interpreted as belonging to // the first list element. Otherwise, it should be placed outside of the // list. - return Left.BlockKind == BK_BracedInit; + return Left.BlockKind == BK_BracedInit || + (Left.is(TT_CtorInitializerColon) && + Style.BreakConstructorInitializers == + FormatStyle::BCIS_AfterColon); if (Left.is(tok::question) && Right.is(tok::colon)) return false; if (Right.is(TT_ConditionalExpr) || Right.is(tok::question)) @@ -2672,11 +2679,15 @@ if (Right.is(tok::identifier) && Right.Next && Right.Next->is(TT_DictLiteral)) return true; + if (Left.is(TT_CtorInitializerColon)) + return Style.BreakConstructorInitializers == FormatStyle::BCIS_AfterColon; + if (Right.is(TT_CtorInitializerColon)) + return Style.BreakConstructorInitializers != FormatStyle::BCIS_AfterColon; if (Left.is(TT_CtorInitializerComma) && - Style.BreakConstructorInitializersBeforeComma) + Style.BreakConstructorInitializers == FormatStyle::BCIS_BeforeComma) return false; if (Right.is(TT_CtorInitializerComma) && - Style.BreakConstructorInitializersBeforeComma) + Style.BreakConstructorInitializers == FormatStyle::BCIS_BeforeComma) return true; if (Left.is(TT_InheritanceComma) && Style.BreakBeforeInheritanceComma) return false; Index: unittests/Format/FormatTest.cpp =================================================================== --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -56,16 +56,17 @@ return *Result; } - FormatStyle getLLVMStyleWithColumns(unsigned ColumnLimit) { - FormatStyle Style = getLLVMStyle(); + FormatStyle getStyleWithColumns(FormatStyle Style, unsigned ColumnLimit) { Style.ColumnLimit = ColumnLimit; return Style; } + FormatStyle getLLVMStyleWithColumns(unsigned ColumnLimit) { + return getStyleWithColumns(getLLVMStyle(), ColumnLimit); + } + FormatStyle getGoogleStyleWithColumns(unsigned ColumnLimit) { - FormatStyle Style = getGoogleStyle(); - Style.ColumnLimit = ColumnLimit; - return Style; + return getStyleWithColumns(getGoogleStyle(), ColumnLimit); } void verifyFormat(llvm::StringRef Code, @@ -2699,6 +2700,165 @@ " aaaa(aaaa) {}")); } +TEST_F(FormatTest, BreakConstructorInitializersAfterColon) { + FormatStyle Style = getLLVMStyle(); + Style.BreakConstructorInitializers = FormatStyle::BCIS_AfterColon; + + verifyFormat("Constructor() : Initializer(FitsOnTheLine) {}"); + verifyFormat("Constructor() : Inttializer(FitsOnTheLine) {}", + getStyleWithColumns(Style, 45)); + verifyFormat("Constructor() :\n" + " Inttializer(FitsOnTheLine) {}", + getStyleWithColumns(Style, 44)); + verifyFormat("Constructor() :\n" + " Inttializer(FitsOnTheLine) {}", + getStyleWithColumns(Style, 43)); + + verifyFormat("template \n" + "Constructor() : Initializer(FitsOnTheLine) {}", + getStyleWithColumns(Style, 50)); + + verifyFormat( + "SomeClass::Constructor() :\n" + " aaaaaaaaaaaaa(aaaaaaaaaaaaaa), aaaaaaaaaaaaaaa(aaaaaaaaaaaa) {}", + Style); + + verifyFormat( + "SomeClass::Constructor() :\n" + " aaaaaaaaaaaaa(aaaaaaaaaaaaaa), aaaaaaaaaaaaa(aaaaaaaaaaaaaa),\n" + " aaaaaaaaaaaaa(aaaaaaaaaaaaaa) {}", + Style); + verifyFormat( + "SomeClass::Constructor() :\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa),\n" + " aaaaaaaaaaaaaaa(aaaaaaaaaaaa) {}", + Style); + verifyFormat("Constructor(aaaaaa aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,\n" + " aaaaaa aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) :\n" + " aaaaaaaaaa(aaaaaa) {}", + Style); + + verifyFormat("Constructor() :\n" + " aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaa),\n" + " aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaa,\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaa),\n" + " aaaaaaaaaaaaaaaaaaaaaaa() {}", + Style); + + verifyFormat("Constructor() :\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) {}", + Style); + + verifyFormat("Constructor(int Parameter = 0) :\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaa),\n" + " aaaaaaaaaaaa(aaaaaaaaaaaaaaaaa) {}", + Style); + verifyFormat("Constructor() :\n" + " aaaaaaaaaaaaaaaaaaaaaa(a), bbbbbbbbbbbbbbbbbbbbbbbb(b) {\n" + "}", + getStyleWithColumns(Style, 60)); + verifyFormat("Constructor() :\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n" + " aaaaaaaaaaaaaaaaaaaaaaaaa(aaaa, aaaa)) {}", + Style); + + // Here a line could be saved by splitting the second initializer onto two + // lines, but that is not desirable. + verifyFormat("Constructor() :\n" + " aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaa),\n" + " aaaaaaaaaaa(aaaaaaaaaaa),\n" + " aaaaaaaaaaaaaaaaaaaaat(aaaaaaaaaaaaaaaaaaaaaaaaaaaa) {}", + Style); + + FormatStyle OnePerLine = Style; + OnePerLine.ConstructorInitializerAllOnOneLineOrOnePerLine = true; + OnePerLine.AllowAllParametersOfDeclarationOnNextLine = false; + verifyFormat("SomeClass::Constructor() :\n" + " aaaaaaaaaaaaa(aaaaaaaaaaaaaa),\n" + " aaaaaaaaaaaaa(aaaaaaaaaaaaaa),\n" + " aaaaaaaaaaaaa(aaaaaaaaaaaaaa) {}", + OnePerLine); + verifyFormat("SomeClass::Constructor() :\n" + " aaaaaaaaaaaaa(aaaaaaaaaaaaaa), // Some comment\n" + " aaaaaaaaaaaaa(aaaaaaaaaaaaaa),\n" + " aaaaaaaaaaaaa(aaaaaaaaaaaaaa) {}", + OnePerLine); + verifyFormat("MyClass::MyClass(int var) :\n" + " some_var_(var), // 4 space indent\n" + " some_other_var_(var + 1) { // lined up\n" + "}", + OnePerLine); + verifyFormat("Constructor() :\n" + " aaaaa(aaaaaa),\n" + " aaaaa(aaaaaa),\n" + " aaaaa(aaaaaa),\n" + " aaaaa(aaaaaa),\n" + " aaaaa(aaaaaa) {}", + OnePerLine); + verifyFormat("Constructor() :\n" + " aaaaa(aaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaa,\n" + " aaaaaaaaaaaaaaaaaaaaaa) {}", + OnePerLine); + OnePerLine.BinPackParameters = false; + verifyFormat( + "Constructor() :\n" + " aaaaaaaaaaaaaaaaaaaaaaaa(\n" + " aaaaaaaaaaa().aaa(),\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) {}", + OnePerLine); + OnePerLine.ColumnLimit = 60; + verifyFormat("Constructor() :\n" + " aaaaaaaaaaaaaaaaaaaa(a),\n" + " bbbbbbbbbbbbbbbbbbbbbbbb(b) {}", + OnePerLine); + + EXPECT_EQ("Constructor() :\n" + " // Comment forcing unwanted break.\n" + " aaaa(aaaa) {}", + format("Constructor() :\n" + " // Comment forcing unwanted break.\n" + " aaaa(aaaa) {}", + Style)); + + Style.ColumnLimit = 0; + verifyFormat("SomeClass::Constructor() :\n" + " a(a) {}", + Style); + verifyFormat("SomeClass::Constructor() noexcept :\n" + " a(a) {}", + Style); + verifyFormat("SomeClass::Constructor() :\n" + " a(a), b(b), c(c) {}", + Style); + verifyFormat("SomeClass::Constructor() :\n" + " a(a) {\n" + " foo();\n" + " bar();\n" + "}", + Style); + + Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None; + verifyFormat("SomeClass::Constructor() :\n" + " a(a), b(b), c(c) {\n" + "}", + Style); + verifyFormat("SomeClass::Constructor() :\n" + " a(a) {\n" + "}", + Style); + + Style.ColumnLimit = 80; + Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All; + Style.ConstructorInitializerIndentWidth = 2; + verifyFormat("SomeClass::Constructor() : a(a), b(b), c(c) {}", + Style); + verifyFormat("SomeClass::Constructor() :\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,\n" + " bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb {}", + Style); +} + TEST_F(FormatTest, MemoizationTests) { // This breaks if the memoization lookup does not take \c Indent and // \c LastSpace into account. @@ -8679,7 +8839,6 @@ CHECK_PARSE_BOOL(BinPackParameters); CHECK_PARSE_BOOL(BreakAfterJavaFieldAnnotations); CHECK_PARSE_BOOL(BreakBeforeTernaryOperators); - CHECK_PARSE_BOOL(BreakConstructorInitializersBeforeComma); CHECK_PARSE_BOOL(BreakStringLiterals); CHECK_PARSE_BOOL(BreakBeforeInheritanceComma) CHECK_PARSE_BOOL(ConstructorInitializerAllOnOneLineOrOnePerLine); @@ -8774,6 +8933,17 @@ CHECK_PARSE("BreakBeforeBinaryOperators: true", BreakBeforeBinaryOperators, FormatStyle::BOS_All); + Style.BreakConstructorInitializers = FormatStyle::BCIS_BeforeColon; + CHECK_PARSE("BreakConstructorInitializers: BeforeComma", + BreakConstructorInitializers, FormatStyle::BCIS_BeforeComma); + CHECK_PARSE("BreakConstructorInitializers: AfterColon", + BreakConstructorInitializers, FormatStyle::BCIS_AfterColon); + CHECK_PARSE("BreakConstructorInitializers: BeforeColon", + BreakConstructorInitializers, FormatStyle::BCIS_BeforeColon); + // For backward compatibility: + CHECK_PARSE("BreakConstructorInitializersBeforeComma: true", + BreakConstructorInitializers, FormatStyle::BCIS_BeforeComma); + Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak; CHECK_PARSE("AlignAfterOpenBracket: Align", AlignAfterOpenBracket, FormatStyle::BAS_Align); @@ -9193,7 +9363,7 @@ TEST_F(FormatTest, BreakConstructorInitializersBeforeComma) { FormatStyle Style = getLLVMStyle(); - Style.BreakConstructorInitializersBeforeComma = true; + Style.BreakConstructorInitializers = FormatStyle::BCIS_BeforeComma; Style.ConstructorInitializerIndentWidth = 4; verifyFormat("SomeClass::Constructor()\n" " : a(a)\n"