ClangFormat - Add option to break before inheritance separation operator in class declaration

[Abpostelnicu]

Summary

This modification addresses a coding style requirement that we have at Mozilla when it comes to he way how code should be written when we are dealing with class inheritance.

• When deriving from a single base, the inherited class should be on the same line as the the inherited class.

class MyClass : public A
{
  ...
};

• When deriving from multiple base classes, each base class should be on it's own line, the inheritance operators should also be on the same line as the base class

class MyClass
  : public X  // When deriving from more than one class, put each on its own line.
  , public Y
{

The above rules can be found in our coding style guideline.

This patch adds a special flag: "BreakBeforeInhertianceDelimiter" that by default is false and is only set with true on our coding style.

The corresponding bug for this patch is here.

I must mention that clang-format with this patch has been applied to the entire fx repo from Mozilla and no regression were triggered. You can see the result here.

[sylvestre.ledru]

tests are coming

[Abpostelnicu]

Also added tests.

[Abpostelnicu]

added patch against the updated repo.

[sylvestre.ledru]

docs/ClangFormatStyleOptions.rst

430 ↗

(On Diff #90176)

s/is/are/

431 ↗

(On Diff #90176)

s/inheritance/ inheritances/

include/clang/Format/Format.h

307 ↗

(On Diff #90176)

same typo

[djasper]

Could you please upload a diff with the entire file as context? That makes reviewing this easier.

docs/ClangFormatStyleOptions.rst

428 ↗

(On Diff #90176)

Auto-generate this with docs/tools/dump_format_style.py

include/clang/Format/Format.h

309 ↗

(On Diff #90176)

s/Delimiter/Colon/

Not because it is better, but just because it's more consistent with much of the rest of clang-format.

lib/Format/TokenAnnotator.cpp

950 ↗

(On Diff #90176)

Maybe "InInheritanceList"?

1012 ↗

(On Diff #90176)

Why would we be in an expression here?

2398 ↗

(On Diff #90176)

I don't understand this comment or what the function is supposed to do. It seems to search whether there is an inheritance comma somewhere in the rest of the line.

2490 ↗

(On Diff #90176)

nit: Use periods of the end of sentences and a space after //.

unittests/Format/FormatTest.cpp

1023 ↗

(On Diff #90176)

I am missing tests that show the behavior when there are multiple base classes, but they do fit on one line.

1033 ↗

(On Diff #90176)

What is this supposed to test?

1039 ↗

(On Diff #90176)

Sure, but the comment is forcing that, so I don't know what this test does.

[Abpostelnicu]

lib/Format/TokenAnnotator.cpp

2398 ↗

(On Diff #90176)

I've corrected the comment, if we have single inheritance we don't want to break onto the next line before the inheritance operand, that's why i'm looking for a TT_InheritanceComma, because if there is one then we are dealing with multiple inheritance.

[Abpostelnicu]

Updated patch with the proposed modifications.

[djasper]

include/clang/Format/Format.h

309 ↗

(On Diff #90300)

Hm. I am still not sure about this flag and it's name. Fundamentally, this is currently controlling two different things:

• Whether to wrap before or after colon and comma
• Whether or not to force wraps for multiple inheritance

That's going to get us in trouble if at some point people also want other combinations of these two things. I have to alternative suggestions:

• Add a single flag (e.g. InheritanceListWrapping) with multiple enum values describing the various choices.
• Don't add a flag at all, but instead break inheritance lists exactly like constructor initializers. I *think* that should solve all the know use cases for now, is easier to configure for users and we can cross the bridge of naming additional flags when we have to.

lib/Format/TokenAnnotator.cpp

2400 ↗

(On Diff #90300)

I don't think you need this. If you set MustBreakBefore to true for the InheritanceCommas and set NoLineBreak to true when adding the InheritanceColon on the same line, then clang-format will already do the right thing.

Fundamentally, this seems to be identical to how we wrap constructor initializer lists in Mozilla style. So I think we should also implement this the same way (if not even reusing the same implementation).

[Abpostelnicu]

include/clang/Format/Format.h

309 ↗

(On Diff #90300)

I see your worries here and indeed the current flags does two things. I'm more inclined to use the num that you suggested since in this way it's easier to separate the coding style differences that somebody uses.

lib/Format/TokenAnnotator.cpp

2400 ↗

(On Diff #90300)

Now that i've seen the behaviour of NoLineBreak thanks for pointing it out to me, but still correct me if i'm wrong but shouldn't i use: `NoLineBreakInOperand`.
My guess is if i use NoLineBreak, than breaking on the current line where we would have : or , would be prohibited.

[djasper]

include/clang/Format/Format.h

309 ↗

(On Diff #90300)

The trouble is that adding more style options should be done very carefully. Some of the reasons here:
https://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options

So unless we know that people would want the one style for constructor initializers and the other style for inheritance lists, I'd keep both the flags and the implementation simple.

lib/Format/TokenAnnotator.cpp

2400 ↗

(On Diff #90300)

Yes, that would be prohibited and that is intended. Remember that I'd set this after placing the colon on the same line (and I should have written explicitly) as the class name. After that, there must not be any further line breaks.

But again, I think we should just have the exact same behavior and implementation as for constructor initializer lists.

[Abpostelnicu]

lib/Format/TokenAnnotator.cpp

2400 ↗

(On Diff #90300)

Yes we should have the exact behaviour like the constructor initialisation list, but that one is also controlled by this flag: `BreakConstructorInitializersBeforeComma`, that on our coding style is set to true.
But still the actual behaviour of initialiser list still breaks before `'` for only one item.

[djasper]

Before '? Can you give an example?

[Abpostelnicu]

In D30487#691506, @djasper wrote:

Before '? Can you give an example?

MY mistake, i wanted to write `:

[djasper]

Do you know whether that is intentional? The style guide isn't really conclusive.

[djasper]

lib/Format/TokenAnnotator.cpp

2486 ↗

(On Diff #90300)

At any rate, I think this is what makes single-item ctor init lists be split. So everything except for this spot could still share the implementation.

[Abpostelnicu]

In D30487#691517, @djasper wrote:

Do you know whether that is intentional? The style guide isn't really conclusive.

Well i'm not sure, because as you said the document is not clear but i think that when we have a single initialiser it should be on the same line as the ctor declaration. In this way it would be consistent with the style for inheritance list. The actual implementation for inheritance list breaking was tailored from the BreakConstructorInitializersBeforeComma flag.

[djasper]

Hm. Unfortunately, this seems to have been implemented to support Webkit style and Webkit style is explicit about wanting this (https://webkit.org/code-style-guidelines/) :(.

But maybe the solution to that is to add an extra flag like AlwaysWrapInitializerList. Really not sure how best to organize this. Any thoughts? (I personally care about neither of these styles, so maybe I am not the best to judge)

At any rate, to move forward, could you remove the hasMultipleInheritance function and instead use the alternative approach discussed?

[Abpostelnicu]

In D30487#691535, @djasper wrote:

Hm. Unfortunately, this seems to have been implemented to support Webkit style and Webkit style is explicit about wanting this (https://webkit.org/code-style-guidelines/) :(.

But maybe the solution to that is to add an extra flag like AlwaysWrapInitializerList. Really not sure how best to organize this. Any thoughts? (I personally care about neither of these styles, so maybe I am not the best to judge)

At any rate, to move forward, could you remove the hasMultipleInheritance function and instead use the alternative approach discussed?

Sure will drop hasMultipleInheritance and use the variable that you mentioned and i won't touch the rest of the patch. After that i will repost it here.

[Abpostelnicu]

corrected some format issues.

[Abpostelnicu]

Daniel what do you think about the last version of the patch? I don't want to have this stalled since we really need this modifications in order to be able to run clang-format on our repository.

[djasper]

I think the patch is fine, except for the name of the flag. It is not breaking inheritance ;).

Maybe BreakBeforeInhertianceColonAndComma, but that's pretty long still. I think maybe we can shorten this to BreakBeforeInhertianceComma, as it never makes sense to break before the comma if we keep the ":" on the old line. What do you think?

[Abpostelnicu]

Yes that's a better name :)

[djasper]

A few nits, otherwise looks good.

include/clang/Format/Format.h

426 ↗

(On Diff #91174)

Please remove "operands" and "only". I think they can be confusing.

852 ↗

(On Diff #91174)

Looks like it might fit on one line now :)

lib/Format/ContinuationIndenter.cpp

355 ↗

(On Diff #91174)

Can you leave a comment here:

// Don't break within the inheritance declaration unless the ":" is on a new line.

750 ↗

(On Diff #91174)

Please merge these into the one about TT_CtorInitializerComma, i.e.:

if (NextNonComment->isOneOf(TT_CtorInitializerColon, TT_InheritanceColon,
                            TT_InheritanceComma))
  return State.FirstIndent + Style.ConstructorInitializerIndentWidth;

[kimgr]

Oops, the flag is called BreakBeforeInhertianceComma with a misspelling throughout. May want to search for inhertiance throughout and see if there are more cases.

docs/ClangFormatStyleOptions.rst

531 ↗

(On Diff #91174)

Tyop: Inhertiance

[djasper]

lib/Format/TokenAnnotator.cpp

2666 ↗

(On Diff #91330)

Do these now fit on one line?

[Abpostelnicu]

lib/Format/ContinuationIndenter.cpp

355 ↗

(On Diff #91174)

I've modified the comment since this should be valid only if Style.BreakBeforeInheritanceComma is set to true otherwise we would be in the strange case where the input where the input is:

class A : public B,
              public C {}

And this would be translated to something like:

class A 
  : public B,
    public C {}

[kimgr]

A few more 'inhertiance' left, otherwise spelling looks good to me :-)

lib/Format/TokenAnnotator.cpp

679 ↗

(On Diff #91330)

This still says 'Inhertiance'

950 ↗

(On Diff #91330)

'Inhertiance'

[Abpostelnicu]

Fixed two spell errors.

[sylvestre.ledru]

Added to the release notes in r297721

cfe/trunk/docs/ClangFormatStyleOptions.rst

**BreakBeforeBraces** (``BraceBreakingStyle``)
   * ``BS_WebKit`` (in configuration: ``WebKit``)
     Like ``Attach``, but break before functions.
 
   * ``BS_Custom`` (in configuration: ``Custom``)
     Configure each individual brace in `BraceWrapping`.
 
 
 
+**BreakBeforeInheritanceComma** (``bool``)
+  If ``true``, in the class inheritance expression clang-format will
+  break before ``:`` and ``,`` if there is multiple inheritance.
+
 **BreakBeforeTernaryOperators** (``bool``)
   If ``true``, ternary operators will be placed after line breaks.
 
 **BreakConstructorInitializersBeforeComma** (``bool``)
   Always break constructor initializers before commas and align
   the commas with the colon.
 
   .. code-block:: c++

cfe/trunk/include/clang/Format/Format.h

struct FormatStyle {
   /// clang-format will respect the input's line breaking decisions within
   /// statements unless they contradict other rules.
   unsigned ColumnLimit;
 
   /// \brief A regular expression that describes comments with special meaning,
   /// which should not be split into lines or otherwise changed.
   std::string CommentPragmas;
 
+  /// \brief If ``true``, in the class inheritance expression clang-format will
+  /// break before ``:`` and ``,`` if there is multiple inheritance.
+  bool BreakBeforeInheritanceComma;
+
   /// \brief If the constructor initializers don't fit on a line, put each
   /// initializer on its own line.
   bool ConstructorInitializerAllOnOneLineOrOnePerLine;
 
   /// \brief The number of characters to use for indentation of constructor
   /// initializer lists.
   unsigned ConstructorInitializerIndentWidth;
 
return AccessModifierOffset == R.AccessModifierOffset &&
            BreakBeforeBinaryOperators == R.BreakBeforeBinaryOperators &&
            BreakBeforeBraces == R.BreakBeforeBraces &&
            BreakBeforeTernaryOperators == R.BreakBeforeTernaryOperators &&
            BreakConstructorInitializersBeforeComma ==
                R.BreakConstructorInitializersBeforeComma &&
            BreakAfterJavaFieldAnnotations == R.BreakAfterJavaFieldAnnotations &&
            BreakStringLiterals == R.BreakStringLiterals &&
            ColumnLimit == R.ColumnLimit && CommentPragmas == R.CommentPragmas &&
+           BreakBeforeInheritanceComma == R.BreakBeforeInheritanceComma &&
            ConstructorInitializerAllOnOneLineOrOnePerLine ==
                R.ConstructorInitializerAllOnOneLineOrOnePerLine &&
            ConstructorInitializerIndentWidth ==
                R.ConstructorInitializerIndentWidth &&
            ContinuationIndentWidth == R.ContinuationIndentWidth &&
            Cpp11BracedListStyle == R.Cpp11BracedListStyle &&
            DerivePointerAlignment == R.DerivePointerAlignment &&
            DisableFormat == R.DisableFormat &&

cfe/trunk/lib/Format/ContinuationIndenter.cpp

 // Returns \c true if \c Current starts a new parameter.
 static bool startsNextParameter(const FormatToken &Current,
                                 const FormatStyle &Style) {
   const FormatToken &Previous = *Current.Previous;
   if (Current.is(TT_CtorInitializerComma) &&
       Style.BreakConstructorInitializersBeforeComma)
     return true;
   return Previous.is(tok::comma) && !Current.isTrailingComment() &&
-         (Previous.isNot(TT_CtorInitializerComma) ||
-          !Style.BreakConstructorInitializersBeforeComma);
+         ((Previous.isNot(TT_CtorInitializerComma) ||
+          !Style.BreakConstructorInitializersBeforeComma) &&
+          (Previous.isNot(TT_InheritanceComma) ||
+          !Style.BreakBeforeInheritanceComma));
 }
 
 ContinuationIndenter::ContinuationIndenter(const FormatStyle &Style,
                                            const AdditionalKeywords &Keywords,
                                            const SourceManager &SourceMgr,
                                            WhitespaceManager &Whitespaces,
                                            encoding::Encoding Encoding,
                                            bool BinPackInconclusiveFunctions)
void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
   }
 
   unsigned Spaces = Current.SpacesRequiredBefore + ExtraSpaces;
 
   if (!DryRun)
     Whitespaces.replaceWhitespace(Current, /*Newlines=*/0, Spaces,
                                   State.Column + Spaces);
 
+  // If "BreakBeforeInheritanceComma" mode, don't break within the inheritance
+  // declaration unless there is multiple inheritance.
+  if (Style.BreakBeforeInheritanceComma && Current.is(TT_InheritanceColon))
+    State.Stack.back().NoLineBreak = true;
+
   if (Current.is(TT_SelectorName) &&
       !State.Stack.back().ObjCSelectorNameFound) {
     unsigned MinIndent =
         std::max(State.FirstIndent + Style.ContinuationIndentWidth,
                  State.Stack.back().Indent);
     unsigned FirstColonPos = State.Column + Spaces + Current.ColumnWidth;
     if (Current.LongestObjCSelectorName == 0)
       State.Stack.back().AlignColons = false;
if (NextNonComment->is(tok::identifier) && NextNonComment->FakeRParens == 0 &&
     return State.Stack.back().Indent;
 
   if (NextNonComment->isOneOf(TT_StartOfName, TT_PointerOrReference) ||
       Previous.isOneOf(tok::coloncolon, tok::equal, TT_JsTypeColon))
     return ContinuationIndent;
   if (PreviousNonComment && PreviousNonComment->is(tok::colon) &&
       PreviousNonComment->isOneOf(TT_ObjCMethodExpr, TT_DictLiteral))
     return ContinuationIndent;
-  if (NextNonComment->is(TT_CtorInitializerColon))
-    return State.FirstIndent + Style.ConstructorInitializerIndentWidth;
   if (NextNonComment->is(TT_CtorInitializerComma))
     return State.Stack.back().Indent;
+  if (NextNonComment->isOneOf(TT_CtorInitializerColon, TT_InheritanceColon,
+                              TT_InheritanceComma))
+    return State.FirstIndent + Style.ConstructorInitializerIndentWidth;
   if (Previous.is(tok::r_paren) && !Current.isBinaryOperator() &&
       !Current.isOneOf(tok::colon, tok::comment))
     return ContinuationIndent;
   if (State.Stack.back().Indent == State.FirstIndent && PreviousNonComment &&
       PreviousNonComment->isNot(tok::r_brace))
     // Ensure that we fall back to the continuation indent width instead of
     // just flushing continuations left.
     return State.Stack.back().Indent + Style.ContinuationIndentWidth;
if (Current.is(TT_CtorInitializerColon)) {
     //       ^ line up here.
     State.Stack.back().Indent =
         State.Column + (Style.BreakConstructorInitializersBeforeComma ? 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_InheritanceColon))
+    State.Stack.back().Indent =
+        State.FirstIndent + Style.ContinuationIndentWidth;
   if (Current.isOneOf(TT_BinaryOperator, TT_ConditionalExpr) && Newline)
     State.Stack.back().NestedBlockIndent =
         State.Column + Current.ColumnWidth + 1;
   if (Current.isOneOf(TT_LambdaLSquare, TT_LambdaArrow))
     State.Stack.back().LastSpace = State.Column;
 
   // Insert scopes created by fake parenthesis.
   const FormatToken *Previous = Current.getPreviousNonComment();

cfe/trunk/lib/Format/Format.cpp

IO.mapOptional("BreakBeforeTernaryOperators",
                    Style.BreakBeforeTernaryOperators);
     IO.mapOptional("BreakConstructorInitializersBeforeComma",
                    Style.BreakConstructorInitializersBeforeComma);
     IO.mapOptional("BreakAfterJavaFieldAnnotations",
                    Style.BreakAfterJavaFieldAnnotations);
     IO.mapOptional("BreakStringLiterals", Style.BreakStringLiterals);
     IO.mapOptional("ColumnLimit", Style.ColumnLimit);
     IO.mapOptional("CommentPragmas", Style.CommentPragmas);
+    IO.mapOptional("BreakBeforeInheritanceComma",
+                   Style.BreakBeforeInheritanceComma);
     IO.mapOptional("ConstructorInitializerAllOnOneLineOrOnePerLine",
                    Style.ConstructorInitializerAllOnOneLineOrOnePerLine);
     IO.mapOptional("ConstructorInitializerIndentWidth",
                    Style.ConstructorInitializerIndentWidth);
     IO.mapOptional("ContinuationIndentWidth", Style.ContinuationIndentWidth);
     IO.mapOptional("Cpp11BracedListStyle", Style.Cpp11BracedListStyle);
     IO.mapOptional("DerivePointerAlignment", Style.DerivePointerAlignment);
     IO.mapOptional("DisableFormat", Style.DisableFormat);
FormatStyle getLLVMStyle() {
   LLVMStyle.BinPackArguments = true;
   LLVMStyle.BreakBeforeBinaryOperators = FormatStyle::BOS_None;
   LLVMStyle.BreakBeforeTernaryOperators = true;
   LLVMStyle.BreakBeforeBraces = FormatStyle::BS_Attach;
   LLVMStyle.BraceWrapping = {false, false, false, false, false, false,
                              false, false, false, false, false};
   LLVMStyle.BreakAfterJavaFieldAnnotations = false;
   LLVMStyle.BreakConstructorInitializersBeforeComma = false;
+  LLVMStyle.BreakBeforeInheritanceComma = false;
   LLVMStyle.BreakStringLiterals = true;
   LLVMStyle.ColumnLimit = 80;
   LLVMStyle.CommentPragmas = "^ IWYU pragma:";
   LLVMStyle.ConstructorInitializerAllOnOneLineOrOnePerLine = false;
   LLVMStyle.ConstructorInitializerIndentWidth = 4;
   LLVMStyle.ContinuationIndentWidth = 4;
   LLVMStyle.Cpp11BracedListStyle = true;
   LLVMStyle.DerivePointerAlignment = false;
MozillaStyle.AlwaysBreakAfterReturnType =
       FormatStyle::RTBS_TopLevel;
   MozillaStyle.AlwaysBreakAfterDefinitionReturnType =
       FormatStyle::DRTBS_TopLevel;
   MozillaStyle.AlwaysBreakTemplateDeclarations = true;
   MozillaStyle.BinPackParameters = false;
   MozillaStyle.BinPackArguments = false;
   MozillaStyle.BreakBeforeBraces = FormatStyle::BS_Mozilla;
   MozillaStyle.BreakConstructorInitializersBeforeComma = true;
+  MozillaStyle.BreakBeforeInheritanceComma = true;
   MozillaStyle.ConstructorInitializerIndentWidth = 2;
   MozillaStyle.ContinuationIndentWidth = 2;
   MozillaStyle.Cpp11BracedListStyle = false;
   MozillaStyle.FixNamespaceComments = false;
   MozillaStyle.IndentCaseLabels = true;
   MozillaStyle.ObjCSpaceAfterProperty = true;
   MozillaStyle.ObjCSpaceBeforeProtocolList = false;
   MozillaStyle.PenaltyReturnTypeOnItsOwnLine = 200;

cfe/trunk/lib/Format/FormatToken.h

#define LIST_TOKEN_TYPES \
   TYPE(DictLiteral) \
   TYPE(ForEachMacro) \
   TYPE(FunctionAnnotationRParen) \
   TYPE(FunctionDeclarationName) \
   TYPE(FunctionLBrace) \
   TYPE(FunctionTypeLParen) \
   TYPE(ImplicitStringLiteral) \
   TYPE(InheritanceColon) \
+  TYPE(InheritanceComma) \
   TYPE(InlineASMBrace) \
   TYPE(InlineASMColon) \
   TYPE(JavaAnnotation) \
   TYPE(JsComputedPropertyName) \
   TYPE(JsFatArrow) \
   TYPE(JsTypeColon) \
   TYPE(JsTypeOperator) \
   TYPE(JsTypeOptionalQuestion) \

cfe/trunk/lib/Format/TokenAnnotator.cpp

case tok::question:
       parseConditional();
       break;
     case tok::kw_template:
       parseTemplateDeclaration();
       break;
     case tok::comma:
       if (Contexts.back().InCtorInitializer)
         Tok->Type = TT_CtorInitializerComma;
+      else if (Contexts.back().InInheritanceList)
+        Tok->Type = TT_InheritanceComma;
       else if (Contexts.back().FirstStartOfName &&
                (Contexts.size() == 1 || Line.startsWith(tok::kw_for))) {
         Contexts.back().FirstStartOfName->PartOfMultiVariableDeclStmt = true;
         Line.IsMultiVariableDeclStmt = true;
       }
       if (Contexts.back().IsForEachMacro)
         Contexts.back().IsExpression = true;
       break;
struct Context {
     bool ColonIsForRangeExpr = false;
     bool ColonIsDictLiteral = false;
     bool ColonIsObjCMethodExpr = false;
     FormatToken *FirstObjCSelectorName = nullptr;
     FormatToken *FirstStartOfName = nullptr;
     bool CanBeExpression = true;
     bool InTemplateArgument = false;
     bool InCtorInitializer = false;
+    bool InInheritanceList = false;
     bool CaretFound = false;
     bool IsForEachMacro = false;
   };
 
   /// \brief Puts a new \c Context onto the stack \c Contexts for the lifetime
   /// of each instance.
   struct ScopedContextCreator {
     AnnotatingParser &P;
void modifyContext(const FormatToken &Current) {
     } else if (Current.is(TT_TrailingReturnArrow)) {
       Contexts.back().IsExpression = false;
     } else if (Current.is(TT_LambdaArrow) || Current.is(Keywords.kw_assert)) {
       Contexts.back().IsExpression = Style.Language == FormatStyle::LK_Java;
     } else if (Current.Previous &&
                Current.Previous->is(TT_CtorInitializerColon)) {
       Contexts.back().IsExpression = true;
       Contexts.back().InCtorInitializer = true;
+    } else if (Current.Previous &&
+               Current.Previous->is(TT_InheritanceColon)) {
+      Contexts.back().InInheritanceList = true;
     } else if (Current.isOneOf(tok::r_paren, tok::greater, tok::comma)) {
       for (FormatToken *Previous = Current.Previous;
            Previous && Previous->isOneOf(tok::star, tok::amp);
            Previous = Previous->Previous)
         Previous->Type = TT_PointerOrReference;
       if (Line.MustBeDeclaration && !Contexts.front().InCtorInitializer)
         Contexts.back().IsExpression = false;
     } else if (Current.is(tok::kw_new)) {
if (Right.Previous->ClosesTemplateDeclaration &&
       Right.Previous->MatchingParen &&
       Right.Previous->MatchingParen->NestingLevel == 0 &&
       Style.AlwaysBreakTemplateDeclarations)
     return true;
   if ((Right.isOneOf(TT_CtorInitializerComma, TT_CtorInitializerColon)) &&
       Style.BreakConstructorInitializersBeforeComma &&
       !Style.ConstructorInitializerAllOnOneLineOrOnePerLine)
     return true;
+  // Break only if we have multiple inheritance.
+  if (Style.BreakBeforeInheritanceComma &&
+      Right.is(TT_InheritanceComma))
+   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
     // deliberate choice and might have aligned the contents of the string
     // literal accordingly. Thus, we try keep existing line breaks.
     return Right.NewlinesBefore > 0;
   if (Right.Previous->is(tok::l_brace) && Right.NestingLevel == 1 &&
       Style.Language == FormatStyle::LK_Proto)
     // Don't put enums onto single lines in protocol buffers.
if (Right.is(tok::identifier) && Right.Next && Right.Next->is(TT_DictLiteral))
     return true;
 
   if (Left.is(TT_CtorInitializerComma) &&
       Style.BreakConstructorInitializersBeforeComma)
     return false;
   if (Right.is(TT_CtorInitializerComma) &&
       Style.BreakConstructorInitializersBeforeComma)
     return true;
+  if (Left.is(TT_InheritanceComma) && Style.BreakBeforeInheritanceComma)
+    return false;
+  if (Right.is(TT_InheritanceComma) && Style.BreakBeforeInheritanceComma)
+    return true;
   if ((Left.is(tok::greater) && Right.is(tok::greater)) ||
       (Left.is(tok::less) && Right.is(tok::less)))
     return false;
   if (Right.is(TT_BinaryOperator) &&
       Style.BreakBeforeBinaryOperators != FormatStyle::BOS_None &&
       (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_All ||
        Right.getPrecedence() != prec::Assignment))
     return true;

cfe/trunk/unittests/Format/FormatTest.cpp

verifyFormat("struct aaaaaaaaaaaaaaaaaaaa\n"
                "    : public aaaaaaaaaaaaaaaaaaa<aaaaaaaaaaaaaaaaaaaaa,\n"
                "                                 aaaaaaaaaaaaaaaaaaaaaa> {};");
   verifyFormat("template <class R, class C>\n"
                "struct Aaaaaaaaaaaaaaaaa<R (C::*)(int) const>\n"
                "    : Aaaaaaaaaaaaaaaaa<R (C::*)(int)> {};");
   verifyFormat("class ::A::B {};");
 }
 
+TEST_F(FormatTest, BreakBeforeInheritanceComma) {
+  FormatStyle StyleWithInheritanceBreak = getLLVMStyle();
+  StyleWithInheritanceBreak.BreakBeforeInheritanceComma = true;
+
+  verifyFormat("class MyClass : public X {};", StyleWithInheritanceBreak);
+  verifyFormat("class MyClass\n"
+               "    : public X\n"
+               "    , public Y {};",
+               StyleWithInheritanceBreak);
+}
+
 TEST_F(FormatTest, FormatsVariableDeclarationsAfterStructOrClass) {
   verifyFormat("class A {\n} a, b;");
   verifyFormat("struct A {\n} a, b;");
   verifyFormat("union A {\n} a;");
 }
 
 TEST_F(FormatTest, FormatsEnum) {
   verifyFormat("enum {\n"
TEST_F(FormatTest, ParsesConfigurationBools) {
   CHECK_PARSE_BOOL(AllowShortLoopsOnASingleLine);
   CHECK_PARSE_BOOL(AlwaysBreakTemplateDeclarations);
   CHECK_PARSE_BOOL(BinPackArguments);
   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);
   CHECK_PARSE_BOOL(DerivePointerAlignment);
   CHECK_PARSE_BOOL_FIELD(DerivePointerAlignment, "DerivePointerBinding");
   CHECK_PARSE_BOOL(DisableFormat);
   CHECK_PARSE_BOOL(IndentCaseLabels);
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
   CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);