Update Mozilla Style

[• kentuckyfriedtakahe]

Summary

Update Mozilla style to more closely match the official Mozilla guidelines

[• kentuckyfriedtakahe]

Discussion in https://bugzilla.mozilla.org/show_bug.cgi?id=961541 supports leaving IndentCaseLabels as is but having ContinuationIdentWidth set to 2.

[• kentuckyfriedtakahe]

This allows breaking after the type in top level function definitions.

int
MyClass::MyFunction();

I tried a few different ways to implement this feature but this is what it took to make it work. I tried implementing it in ContinuationIndenter::mustBreak but State.Line->MightBeFunctionDecl seems to be false for functions with no arguments.

[djasper]

include/clang/Format/Format.h

252

We are trying to move towards fewer and more generic options. So my initial question would be, are you sure that this applies only for top-level functions? I couldn't find anything in the style guide.. Also, I didn't find anything in the GNU style guide. The same is true for the question: Does this apply to both declarations and definitions? So, I think we should at least make it easy to enable this also for non-top-level functions.

So, more specifically, I would make this an enum:

enum BreakReturnType { // Name might not be ideal.
  BRT_Always,
  BRT_TopLevel,
  BRT_Default,
  BRT_LastResort
};

And this can then also replace the current PenaltyReturnTypeOnItsOwnLine (using 60 for BRT_TopLevel, and BRT_Default and 200 for BRT_LastResort).

Then I would probably call the style option:

BreakReturnType BreakFunctionsAfterType;

lib/Format/Format.cpp

339

Shouldn't this contain your other modifications?

342

Whatever this parameter ends up being called, please also set it for GNU style (llvm.org/PR 17851).

lib/Format/TokenAnnotator.cpp

1087

The name 'FirstFunctionDecl' doesn't make much sense to me as there aren't multiple function declarations in one line. I guess you'd want to exclude trailing annotations? Can you add tests testing this and the other branches (top-level vs. not-top-level) in unittests/Format/FormatTests.cpp?

1105

No line break after "}".. (Use clang-format :-) )

[djasper]

Ping? Any update on this?

[• kentuckyfriedtakahe]

Daniel,

Thanks for reminding me. I've just brought up clang-format for further
bike shedding on dev-platform to see what people think. I'll get back to
you next week about it.

Anthony

[djasper]

Any updates?

include/clang/Format/Format.h

struct FormatStyle {
   /// the parentheses of a function call with that name. If there is no name,
   /// a zero-length name is assumed.
   bool Cpp11BracedListStyle;
 
   /// \brief If \c true, indent when breaking function declarations which
   /// are not also definitions after the type.
   bool IndentFunctionDeclarationAfterType;
 
+  /// \brief If \c true, breaks top function definitions after the type
+  /// declaration.
+  bool AlwaysBreakTopLevelFunctionsAfterType;
+
   /// \brief If \c true, spaces will be inserted after '(' and before ')'.
   bool SpacesInParentheses;
 
   /// \brief If \c true, spaces will be inserted after '<' and before '>' in
   /// template argument lists
   bool SpacesInAngles;
 
   /// \brief If \c false, spaces may be inserted into '()'.
return AccessModifierOffset == R.AccessModifierOffset &&
            AlignTrailingComments == R.AlignTrailingComments &&
            AllowAllParametersOfDeclarationOnNextLine ==
                R.AllowAllParametersOfDeclarationOnNextLine &&
            AllowShortFunctionsOnASingleLine ==
                R.AllowShortFunctionsOnASingleLine &&
            AllowShortIfStatementsOnASingleLine ==
                R.AllowShortIfStatementsOnASingleLine &&
            AllowShortLoopsOnASingleLine == R.AllowShortLoopsOnASingleLine &&
-           AlwaysBreakTemplateDeclarations ==
-               R.AlwaysBreakTemplateDeclarations &&
            AlwaysBreakBeforeMultilineStrings ==
                R.AlwaysBreakBeforeMultilineStrings &&
+           AlwaysBreakTopLevelFunctionsAfterType ==
+               R.AlwaysBreakTopLevelFunctionsAfterType &&
+           AlwaysBreakTemplateDeclarations ==
+               R.AlwaysBreakTemplateDeclarations &&
            BinPackParameters == R.BinPackParameters &&
            BreakBeforeBinaryOperators == R.BreakBeforeBinaryOperators &&
            BreakBeforeTernaryOperators == R.BreakBeforeTernaryOperators &&
            BreakBeforeBraces == R.BreakBeforeBraces &&
            BreakConstructorInitializersBeforeComma ==
                R.BreakConstructorInitializersBeforeComma &&
            ColumnLimit == R.ColumnLimit &&
            ConstructorInitializerAllOnOneLineOrOnePerLine ==

lib/Format/Format.cpp

static void mapping(IO &IO, FormatStyle &Style) {
     IO.mapOptional("AllowAllParametersOfDeclarationOnNextLine",
                    Style.AllowAllParametersOfDeclarationOnNextLine);
     IO.mapOptional("AllowShortIfStatementsOnASingleLine",
                    Style.AllowShortIfStatementsOnASingleLine);
     IO.mapOptional("AllowShortLoopsOnASingleLine",
                    Style.AllowShortLoopsOnASingleLine);
     IO.mapOptional("AllowShortFunctionsOnASingleLine",
                    Style.AllowShortFunctionsOnASingleLine);
+    IO.mapOptional("AlwaysBreakTopLevelFunctionsAfterType",
+                   Style.AlwaysBreakTopLevelFunctionsAfterType);
     IO.mapOptional("AlwaysBreakTemplateDeclarations",
                    Style.AlwaysBreakTemplateDeclarations);
     IO.mapOptional("AlwaysBreakBeforeMultilineStrings",
                    Style.AlwaysBreakBeforeMultilineStrings);
     IO.mapOptional("BreakBeforeBinaryOperators",
                    Style.BreakBeforeBinaryOperators);
     IO.mapOptional("BreakBeforeTernaryOperators",
                    Style.BreakBeforeTernaryOperators);
FormatStyle getLLVMStyle() {
   LLVMStyle.AccessModifierOffset = -2;
   LLVMStyle.AlignEscapedNewlinesLeft = false;
   LLVMStyle.AlignTrailingComments = true;
   LLVMStyle.AllowAllParametersOfDeclarationOnNextLine = true;
   LLVMStyle.AllowShortFunctionsOnASingleLine = true;
   LLVMStyle.AllowShortIfStatementsOnASingleLine = false;
   LLVMStyle.AllowShortLoopsOnASingleLine = false;
   LLVMStyle.AlwaysBreakBeforeMultilineStrings = false;
+  LLVMStyle.AlwaysBreakTopLevelFunctionsAfterType = false;
   LLVMStyle.AlwaysBreakTemplateDeclarations = false;
   LLVMStyle.BinPackParameters = true;
   LLVMStyle.BreakBeforeBinaryOperators = false;
   LLVMStyle.BreakBeforeTernaryOperators = true;
   LLVMStyle.BreakBeforeBraces = FormatStyle::BS_Attach;
   LLVMStyle.BreakConstructorInitializersBeforeComma = false;
   LLVMStyle.ColumnLimit = 80;
   LLVMStyle.ConstructorInitializerAllOnOneLineOrOnePerLine = false;
FormatStyle getChromiumStyle() {
   ChromiumStyle.AllowShortIfStatementsOnASingleLine = false;
   ChromiumStyle.AllowShortLoopsOnASingleLine = false;
   ChromiumStyle.BinPackParameters = false;
   ChromiumStyle.DerivePointerBinding = false;
   ChromiumStyle.Standard = FormatStyle::LS_Cpp03;
   return ChromiumStyle;
 }
 
 FormatStyle getMozillaStyle() {
   FormatStyle MozillaStyle = getLLVMStyle();
   MozillaStyle.AllowAllParametersOfDeclarationOnNextLine = false;
+  MozillaStyle.AlwaysBreakTopLevelFunctionsAfterType = true;
   MozillaStyle.BreakConstructorInitializersBeforeComma = true;
   MozillaStyle.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
   MozillaStyle.ConstructorInitializerIndentWidth = 2;
   MozillaStyle.DerivePointerBinding = true;
   MozillaStyle.IndentCaseLabels = true;
   MozillaStyle.ObjCSpaceBeforeProtocolList = false;
   MozillaStyle.PenaltyReturnTypeOnItsOwnLine = 200;
   MozillaStyle.PointerBindsToType = true;

lib/Format/TokenAnnotator.cpp

 
 void TokenAnnotator::calculateFormattingInformation(AnnotatedLine &Line) {
   Line.First->TotalLength =
       Line.First->IsMultiline ? Style.ColumnLimit : Line.First->ColumnWidth;
   if (!Line.First->Next)
     return;
   FormatToken *Current = Line.First->Next;
   bool InFunctionDecl = Line.MightBeFunctionDecl;
+  bool FirstFunctionDecl = Line.MightBeFunctionDecl && Style.AlwaysBreakTopLevelFunctionsAfterType;
   while (Current != NULL) {
     if (Current->Type == TT_LineComment) {
       if (Current->Previous->BlockKind == BK_BracedInit)
         Current->SpacesRequiredBefore = Style.Cpp11BracedListStyle ? 0 : 1;
       else
         Current->SpacesRequiredBefore = Style.SpacesBeforeTrailingComments;
     } else if (Current->SpacesRequiredBefore == 0 &&
              spaceRequiredBefore(Line, *Current)) {
       Current->SpacesRequiredBefore = 1;
     }
 
     Current->MustBreakBefore =
         Current->MustBreakBefore || mustBreakBefore(Line, *Current);
 
+    if (FirstFunctionDecl) {
+      if (Current->Type == TT_StartOfName) {
+        Current->MustBreakBefore = Line.Level == 0;
+      }
+      else if (Current->is(tok::l_paren)) {
+        FirstFunctionDecl = false;
+      }
+    }
+
     Current->CanBreakBefore =
         Current->MustBreakBefore || canBreakBefore(Line, *Current);
     if (Current->MustBreakBefore || !Current->Children.empty() ||
         Current->IsMultiline)
       Current->TotalLength = Current->Previous->TotalLength + Style.ColumnLimit;
     else
       Current->TotalLength = Current->Previous->TotalLength +
                              Current->ColumnWidth +

unittests/Format/FormatTest.cpp

TEST_F(FormatTest, ParsesConfiguration) {
   CHECK_PARSE_BOOL(DerivePointerBinding);
   CHECK_PARSE_BOOL(IndentCaseLabels);
   CHECK_PARSE_BOOL(ObjCSpaceBeforeProtocolList);
   CHECK_PARSE_BOOL(PointerBindsToType);
   CHECK_PARSE_BOOL(Cpp11BracedListStyle);
   CHECK_PARSE_BOOL(IndentFunctionDeclarationAfterType);
   CHECK_PARSE_BOOL(SpacesInParentheses);
   CHECK_PARSE_BOOL(SpacesInAngles);
+  CHECK_PARSE_BOOL(AlwaysBreakTopLevelFunctionsAfterType);
   CHECK_PARSE_BOOL(SpaceInEmptyParentheses);
   CHECK_PARSE_BOOL(SpacesInCStyleCastParentheses);
   CHECK_PARSE_BOOL(SpaceBeforeAssignmentOperators);
 
   CHECK_PARSE("AccessModifierOffset: -1234", AccessModifierOffset, -1234);
   CHECK_PARSE("ConstructorInitializerIndentWidth: 1234",
               ConstructorInitializerIndentWidth, 1234u);
   CHECK_PARSE("ColumnLimit: 1234", ColumnLimit, 1234u);