[PATCH] Update Mozilla Style

[Daniel Jasper]

================
Comment at: lib/Format/Format.cpp:339
@@ -335,3 +338,3 @@
 
 FormatStyle getMozillaStyle() {
   FormatStyle MozillaStyle = getLLVMStyle();
----------------
Shouldn't this contain your other modifications?

================
Comment at: lib/Format/Format.cpp:342
@@ -338,2 +341,3 @@
   MozillaStyle.AllowAllParametersOfDeclarationOnNextLine = false;
+  MozillaStyle.AlwaysBreakTopLevelFunctionsAfterType = true;
   MozillaStyle.BreakConstructorInitializersBeforeComma = true;
----------------
Whatever this parameter ends up being called, please also set it for GNU style (llvm.org/PR 17851).

================
Comment at: include/clang/Format/Format.h:252
@@ +251,3 @@
+  /// declaration.
+  bool AlwaysBreakTopLevelFunctionsAfterType;
+
----------------
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;


================
Comment at: lib/Format/TokenAnnotator.cpp:1087
@@ -1086,2 +1086,3 @@
   bool InFunctionDecl = Line.MightBeFunctionDecl;
+  bool FirstFunctionDecl = Line.MightBeFunctionDecl && Style.AlwaysBreakTopLevelFunctionsAfterType;
   while (Current != NULL) {
----------------
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?

================
Comment at: lib/Format/TokenAnnotator.cpp:1105
@@ +1104,3 @@
+        Current->MustBreakBefore = Line.Level == 0;
+      }
+      else if (Current->is(tok::l_paren)) {
----------------
No line break after "}".. (Use clang-format :-) )


http://llvm-reviews.chandlerc.com/D2610