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 ↗

(On Diff #6628)

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

336

Shouldn't this contain your other modifications?

339

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

lib/Format/TokenAnnotator.cpp

1087 ↗

(On Diff #6628)

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 ↗

(On Diff #6628)

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?

lib/Format/Format.cpp

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.BreakConstructorInitializersBeforeComma = true;
   MozillaStyle.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
+  MozillaStyle.ConstructorInitializerIndentWidth = 2;
   MozillaStyle.DerivePointerBinding = true;
   MozillaStyle.IndentCaseLabels = true;
   MozillaStyle.ObjCSpaceBeforeProtocolList = false;
   MozillaStyle.PenaltyReturnTypeOnItsOwnLine = 200;
   MozillaStyle.PointerBindsToType = true;
+  MozillaStyle.ContinuationIndentWidth = 2;
+  MozillaStyle.BreakBeforeBraces = FormatStyle::BS_Linux;
   return MozillaStyle;
 }
 
 FormatStyle getWebKitStyle() {
   FormatStyle Style = getLLVMStyle();
   Style.AccessModifierOffset = -4;
   Style.AlignTrailingComments = false;
   Style.BreakBeforeBinaryOperators = true;