[clang-format] Option for empty lines after an access modifier.

[Max_S]

Summary

The current logic for access modifiers in classes ignores the option 'MaxEmptyLinesToKeep=1'. It is therefore impossible to have a coding style that requests one empty line after an access modifier. The patch allows the user to configure how many empty lines clang-format should add after an access modifier. This will remove lines if there are to many and will add them if there are missing.

[HazardyKnusperkeks]

I would like additional tests:

• With at least 2 empty lines
• With 0-2 empty lines without verifyFormat to demonstrate what is kept, added, and removed.

clang/lib/Format/UnwrappedLineFormatter.cpp

1284

I don't know, I'm just asking:
Shouldn't this be Newlines = std::min(Newlines, Style.EmptyLinesAfterAccessModifier + 1u);?

[Max_S]

Updating D98237: [clang-format] Option for empty lines after an access modifier.

• Added additional verification tests
• Added expected formating tests
• Changed comment to reflect the new logic

[Max_S]

In D98237#2614844, @HazardyKnusperkeks wrote:

I would like additional tests:

• With at least 2 empty lines
• With 0-2 empty lines without verifyFormat to demonstrate what is kept, added, and removed.

Thank you for reviewing this, I added new tests, that demonstrate the formatting behavior.

[Max_S]

clang/lib/Format/UnwrappedLineFormatter.cpp

1284

This is also possible but then the logic would be how many lines should be kept at maximum after an access specifier.

The name would then be Style.KeepMaximumLinesAfterAccessModifier.

Currently the logic above:

if (Newlines == 0 && !RootToken.IsFirst)
  Newlines = 1;

forces Newlines to be always 1 or bigger. Therefore the old logic would always add one new line and I decided to implement the setting in the same way.

[MyDeveloperDay]

Just out of interest and we are supposed to ask for this but can you point to public style guide that uses this style. (actually I don't mind if other formatting tools have this capability and you highlight it, like astyle or editorConfig etc)

From my perspective this seems like a reasonable suggestion. (even if I'm unlikely to use it myself ;-))

clang/unittests/Format/FormatTest.cpp

9205

why can't you just verifyFormat them all?

9212

yeah I'm not a fan of this like this... sorry... just write the test out in long form, when it goes wrong I don't have to be a compiler to understand what is going wrong I can just see it.

[Max_S]

In D98237#2616193, @MyDeveloperDay wrote:

Just out of interest and we are supposed to ask for this but can you point to public style guide that uses this style. (actually I don't mind if other formatting tools have this capability and you highlight it, like astyle or editorConfig etc)

From my perspective this seems like a reasonable suggestion. (even if I'm unlikely to use it myself ;-))

I can not specify an other formater or a style guide that uses this style. The problem is that clang-format is rather destructive at this point. Other formatters like astyle or the one from CLion keep empty lines after an access modifier. The behavior of clang-format before this patch was to remove all empty lines after an access modifier. The option MaxEmptyLinesToKeep is currently ignored at this point.

After thinking a little bit about this:
If the removal of all lines after an access modifier is rather a bug than a feature, then the change:

   if (PreviousLine && PreviousLine->First->isAccessSpecifier() &&
       (!PreviousLine->InPPDirective || !RootToken.HasUnescapedNewline))
-    Newlines = std::min(1u, Newlines);
+    Newlines = std::max(1u, Newlines);

   if (Newlines)

would solve this problem.

The format test suite runs with this patch, the only error is:

[ RUN      ] FormatTest.SeparatesLogicalBlocks
/home/msagebaum/Kaiserslautern/Programms/temp/llvm-project/clang/unittests/Format/FormatTest.cpp:1852: Failure
      Expected: "class A {\n" "protected:\n" "public:\n" "  void f();\n" "};"
      Which is: "class A {\nprotected:\npublic:\n  void f();\n};"
To be equal to: format("class A {\n" "protected:\n" "\n" "public:\n" "\n" "  void f();\n" "};")
      Which is: "class A {\nprotected:\n\npublic:\n\n  void f();\n};"
With diff:
@@ +1,7 @@
 class A {
 protected:
+
 public:
+
   void f();
 };

/home/msagebaum/Kaiserslautern/Programms/temp/llvm-project/clang/unittests/Format/FormatTest.cpp:1866: Failure
      Expected: "#define B     \\\n" "  class A {   \\\n" "   protected: \\\n" "   public:    \\\n" "    void f(); \\\n" "  };"
      Which is: "#define B     \\\n  class A {   \\\n   protected: \\\n   public:    \\\n    void f(); \\\n  };"
To be equal to: format("#define B     \\\n" "  class A {   \\\n" "   protected: \\\n" "              \\\n" "   public:    \\\n" "              \\\n" "    void f(); \\\n" "  };", getGoogleStyle())
      Which is: "#define B     \\\n  class A {   \\\n   protected: \\\n              \\\n   public:    \\\n              \\\n    void f(); \\\n  };"
With diff:
@@ +2,7 @@
   class A {   \\
    protected: \\
+              \\
    public:    \\
+              \\
     void f(); \\
   };

[  FAILED  ] FormatTest.SeparatesLogicalBlocks (7 ms)

clang/unittests/Format/FormatTest.cpp

9205

Yes. I will change this in the next update.

9212

I can change this, but the current output of the tests is (I forced the error):

/<path>/llvm-project/clang/unittests/Format/FormatTest.cpp:72: Failure
      Expected: Expected.str()
      Which is: "class Foo {\nprivate:\n\n  int i;\n};"
To be equal to: format(Expected, Style)
      Which is: "class Foo {\nprivate:\n  int i;\n};"
With diff:
@@ -1,5 @@
 class Foo {
 private:
-
   int i;
 };

Which is actually human readable in this case. Shall I still change it?

[HazardyKnusperkeks]

Please also add an entry in the clang/doc/ReleaseNotes.rst.

clang/lib/Format/UnwrappedLineFormatter.cpp

1284

With your explanation everything is fine here.

clang/unittests/Format/FormatTest.cpp

9205

He verifyFormats them with the right style, doesn't he?

With handling of empty lines I think it is useful to add the EXPECT_EQ.

9212

I'm a fan of it. :)
Especially with the demonstration that the string is still expanded.

[curdeius]

I'm missing tests with both EmptyLineBeforeAccessModifier and EmptyLine(s)AfterAccessModifier options.
And possibly other options that could interfere with them.

clang/include/clang/Format/Format.h

1957

This option seems to be very different from EmptyLineBeforeAccessModifier. I don't mean in what it does, because this is analogical, but in the possible options.
Wouldn't it be less surprising to have (at least some) similar options here and there?
Is there any value in having more than one line after access modifiers? Couldn't that be achieved with Leave option?
How do the two options work together?

Also, the difference in singular vs. plural form of "Line(s)" in these two options is disconcerting.
From the user perspective, it's error-prone to have two options that are at the same time so similar and so different.

[Max_S]

Changed the option to EmptyLineAfterAccessModifier and allowed the options Never, Leave and Always. Updated the tests accordingly and also added an entry in the changelog.

[Max_S]

As already described in the update I changed the option name and how it behaves. It is now more in line with EmptyLineBeforeAccessModifier.

I hope this was the correct way to push the rewrite, since all line remarks are now off.

clang/include/clang/Format/Format.h

1957

I agree with you and changed it accordingly. I left out the option LogicalBlock.

The interaction between the two options is quite minimal. I can add extra tests, that would demonstrate this but I do not think that this is necessary.

The leave option would now applies MaxEmptyLinesToKeep in a correct way. See also my remarks in >>! In D98237#2616621.

clang/unittests/Format/FormatTest.cpp

9212

I changed all to EXPECT_EQ since the failer output there is better. It shows the variables names and not the arbitrary code in verfyFormat. Hope this is ok.

/home/msagebaum/Kaiserslautern/Programms/temp/llvm-project/clang/unittests/Format/FormatTest.cpp:9202: Failure
      Expected: test2NL
      Which is: "class Foo {\nprivate:\n\n  int i;\n};"
To be equal to: format(test1NL)
      Which is: "class Foo {\nprivate:\n  int i;\n};"
With diff:
@@ -1,5 @@
 class Foo {
 private:
-
   int i;
 };

`

[curdeius]

That looks really good. Please add tests as indicated in the inline comments and fix the formatting (see clang-format warnings).

clang/include/clang/Format/Format.h

1912

Shouldn't we put the same comment in ELBAMS_Leave?
That goes outside the scope of this patch if it ELBAMS doesn't behave this way.

1957

I like the new version. Thank you for the update.
I'd still love to see the tests with both Before/After options (and we probably want MaxEmptyLinesToKeep > 1 for these tests in order to check whether e.g. Always/Always keeps precisely one new line).

I mean something along these lines (amongst other tests):

  Style.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Always;
  Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_Always;
  Style.MaxEmptyLinesToKeep = 3;
  EXPECT_EQ(R"(
struct S {
private:

public:
};
)",
format(R"(
struct S {
private:



public:
};
)", Style));

[HazardyKnusperkeks]

clang/include/clang/Format/Format.h

1914

It does not always add, it does enforces one empty line, or am I mistaken?

[Max_S]

I added more tests for the interaction between MaxEmptyLinesToKeep, EmptyLineBeforeAccessModifier and EmptyLineAfterAccessModifier. During these tests I recognized, that EmptyLineBeforeAccessModifier = Always,Leave will (should adhere) to MaxEmptyLinesToKeep. So the option does not force the new line it only applies it if it is missing. I changed the logic for EmptyLineAfterAccessModifier accordingly.

Four of the new tests fail, two of them are probably bugs in the implementation of EmptyLineBeforeAccessModifier.

The test in line 9354 is set to:

Style.MaxEmptyLinesToKeep = 0u; // Both force one new line
Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_Always;
Style.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Always;
EXPECT_EQ(NL_B_1_A_1_I_1, format(NL_B_3_A_3_I_3, Style));

So both options should force here the newline, that are removed by MaxEmptyLinesToKeep. EmptyLineBeforeAccessModifier removes all lines, so it seems that MaxEmptyLinesToKeep has the higher rank here. Is that correct behavior?

The test in line 9368 is set to:

Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_Leave;
Style.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Leave;
Style.MaxEmptyLinesToKeep = 1u; 
EXPECT_EQ(NL_B_1_A_1_I_1, format(NL_B_3_A_3_I_3, Style));

So both options should leave the new lines in place but should adhere to MaxEmptyLinesToKeep. EmptyLineBeforeAccessModifier keeps all three lines, so it seems that MaxEmptyLinesToKeep has the lower rank here. Is that correct behavior?

The tests in line 9462 and 9466 require some additional implementation. EmptyLineAfterAccessModifier is adding the three lines since it is configured to do so, but EmptyLineBeforeAccessModifier would remove these lines. Since EmptyLineBeforeAccessModifier can only access the old file and not the new one. It does not know that three lines have been written and probably could not remove them.

I would like to implement it in such a way, that EmptyLineAfterAccessModifier looks ahead and skips its logic if the next token is a also an access modifier such that the logic of EmptyLineBeforeAccessModifier takes precedence. I could not find a way to get the next line. Is this possible somehow? Does there exist some helper functionality?

[Max_S]

clang/include/clang/Format/Format.h

1914

It adds it if it is missing, but leaves additional lines. All in all MaxEmptyLinesToKeep has precedence here. If MaxEmptyLinesToKeep is zero, then no line is added.

[MyDeveloperDay]

clang/unittests/Format/FormatTest.cpp

9212

why would we break with convention?

[MyDeveloperDay]

clang/unittests/Format/FormatTest.cpp

9392

I can't read this, I can't tell if you've fat fingered a test, I feel these are better done systematically one by one with the text in the verifyFormat("...") it makes it easier when they fail to understand what is going on.

Also its ok to have more than one TEST_F if you want to handle the different states in small batches

I think you undetersand what NL_B_3_A_0_I_0 means and how it differs form NL_B_0_A_3_I_0 but others won't

[MyDeveloperDay]

could you please mark your comments done when they are done.

[MyDeveloperDay]

clang/docs/ClangFormatStyleOptions.rst

2132

2159

if MaxEmptyLineToKeep is 0 then surely ELAAMS_Always should win, as this is the lesser i.e. if people never uses extra lines except after access modifiers this is what they would set? did I missunderstand?

otherwise doesn't MaxEmptyLinesToKeep just nullify the whole option? meaning its useless for anyone using it?

I agree in the ELAAMS_Leave case that MaxEmptyLinesToKeep should win.

[Max_S]

Fixed interaction between Before and After configurations options. Before handles the case
of two access modifiers specification following each other.

[Max_S]

The tests in line 9462 and 9466 require some additional implementation. EmptyLineAfterAccessModifier is adding the three lines since it is configured to do so, but EmptyLineBeforeAccessModifier would remove these lines. Since EmptyLineBeforeAccessModifier can only access the old file and not the new one. It does not know that three lines have been written and probably could not remove them.

I would like to implement it in such a way, that EmptyLineAfterAccessModifier looks ahead and skips its logic if the next token is a also an access modifier such that the logic of EmptyLineBeforeAccessModifier takes precedence. I could not find a way to get the next line. Is this possible somehow? Does there exist some helper functionality?

I fixed this in the newest change set. EmptyLineBeforeAccessModifier handles the case when two access modifiers follow each other. I updated the documentation accordingly.

Four of the new tests fail, two of them are probably bugs in the implementation of EmptyLineBeforeAccessModifier.

The test in line 9354 is set to:

Style.MaxEmptyLinesToKeep = 0u; // Both force one new line
Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_Always;
Style.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Always;
EXPECT_EQ(NL_B_1_A_1_I_1, format(NL_B_3_A_3_I_3, Style));

So both options should force here the newline, that are removed by MaxEmptyLinesToKeep. EmptyLineBeforeAccessModifier removes all lines, so it seems that MaxEmptyLinesToKeep has the higher rank here. Is that correct behavior?

The test in line 9368 is set to:

Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_Leave;
Style.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Leave;
Style.MaxEmptyLinesToKeep = 1u; 
EXPECT_EQ(NL_B_1_A_1_I_1, format(NL_B_3_A_3_I_3, Style));

So both options should leave the new lines in place but should adhere to MaxEmptyLinesToKeep. EmptyLineBeforeAccessModifier keeps all three lines, so it seems that MaxEmptyLinesToKeep has the lower rank here. Is that correct behavior?

I still need some advice on how to handle this. Should I fix the test so that it captures the current behavior. Should I fix the logic or should I open a bug report?

In D98237#2637832, @MyDeveloperDay wrote:

could you please mark your comments done when they are done.

I usually try to do this. How can I see which ones are still open?

clang/docs/ClangFormatStyleOptions.rst

2159

if MaxEmptyLineToKeep is 0 then surely ELAAMS_Always should win, as this is the lesser i.e. if people never uses extra lines except after access modifiers this is what they would set? did I missunderstand?

You understood correctly and that is the way it is implemented.

I agree in the ELAAMS_Leave case that MaxEmptyLinesToKeep should win.

That is also the way it is implemented.

I fixed the problem with two access modifiers in a row.

clang/unittests/Format/FormatTest.cpp

9212

verifyFormat errors look like:

/<path>/llvm-project/clang/unittests/Format/FormatTest.cpp:72: Failure
      Expected: Expected.str()
      Which is: "class Foo {\nprivate:\n\n  int i;\n};"
To be equal to: format(Expected, Style)
      Which is: "class Foo {\nprivate:\n  int i;\n};"
With diff:
@@ -1,5 @@
 class Foo {
 private:
-
   int i;
 };

EXPECT_EQ errors look like:

/home/msagebaum/Kaiserslautern/Programms/temp/llvm-project/clang/unittests/Format/FormatTest.cpp:9202: Failure
      Expected: test2NL
      Which is: "class Foo {\nprivate:\n\n  int i;\n};"
To be equal to: format(test1NL)
      Which is: "class Foo {\nprivate:\n  int i;\n};"
With diff:
@@ -1,5 @@
 class Foo {
 private:
-
   int i;
 };

In the first approach you do not get the actual line which the error created (just the line in a helper function). The stacktrace afterwards tells you wehre the error origniated.
I can change everything to verifyFormat, personally I think that EXPECT_EQ has the improved output.

Please give me a go / no go to change everything to verifyFormat.

9392

I can't read this,

I may have missed here a comment. The convention is:

NewLines_Before_Access_Modifier_3_After_Access_Modifier_3_Inbetween_Access_Modifier_3
N     L_       B_                                       3_A_                                    3_I_                                               3
NL_B_3_A_3_I_3

I can't tell if you've fat fingered a test

What do you mean by that?

I feel these are better done systematically

Thats what I have actually done. First tests only concerning EmptyLineAfterAccessModifier afterwards the combinations with EmptyLineAfterAccessModifier and EmptyLineBeforeAccessModifier.
You have the matrix:

Before\After | Never | Leave | Always
Never        |     x     |    x      |     x
Leave        |     x     |    x      |     x
Always       |     x     |    x      |     x
LogicalBlock |     x     |    x      |     x

with the text in the verifyFormat("...") it makes it easier when they fail to understand what is going on.

I do not think that verifyFormat is the better option. Both provide you with the actual string and the diff. verifyFormat gives you some internal variable names, EXPECT_EQ the names or strings you have provided which is IMHO more readable.

As mentioned in the comment to line 9207 I can gladly change this if requested.
If I expand the string the test will expand from 289 lines to approx. 500 lines.

Also its ok to have more than one TEST_F if you want to handle the different states in small batches

I will split it into two tests on the next change. One that tests EmptyLineAfterAccessModifier and one that tests the interaction EmptyLineAfterAccessModifier and EmptyLineBeforeAccessModifier.

For the interaction I can reduce it to a smaller code sample, that just has the interaction. That is:

struct foo {
private:



protected:
};

Please give me a go / no go on:

• Adding comment for naming convention
• A more systematic approach (please specify what you have in your mind, what would be a better approach.)
• Change everything to verifyFormat
• Remove the variables and expand everything to the arguments
• Reduce the interaction test to a smaller code sample. (This would also change the naming convention to NL_Between_3.)

I will defenetly do:

• Split it into two tests.

[MyDeveloperDay]

If you follow people tweeting about clang-format (as I do) and you look through the bug tracking system, one major criticism of clang-format is that the second clang-format can be different from the first, sometimes an equilibrium can be found sometimes not.

When I started working on clang-format I was encouraged to use verifyFormat() as it tests that scenario and also tries to mess up the format and ensure it returns to the desired state. It found bugs in my code which would have made clang-format worse.

Apart from it being the convention I believe it makes for much more readable code, even if there is repetition as I don't need to keep cross referencing variables with rather obscure names NL_B_3_A_0_I_0 this is unnecessary noise and makes the code overly verbose.

No you'll need to check out what the messUp() function is actually doing but I think by and large IMHO we should stick with verifyFormat.

[MyDeveloperDay]

I'd be quite interested to understand what the impact (if any) would be on javascript and C# formatting

[Max_S]

In D98237#2643815, @MyDeveloperDay wrote:

If you follow people tweeting about clang-format (as I do) and you look through the bug tracking system, one major criticism of clang-format is that the second clang-format can be different from the first, sometimes an equilibrium can be found sometimes not.

When I started working on clang-format I was encouraged to use verifyFormat() as it tests that scenario and also tries to mess up the format and ensure it returns to the desired state. It found bugs in my code which would have made clang-format worse.

Apart from it being the convention I believe it makes for much more readable code, even if there is repetition as I don't need to keep cross referencing variables with rather obscure names NL_B_3_A_0_I_0 this is unnecessary noise and makes the code overly verbose.

No you'll need to check out what the messUp() function is actually doing but I think by and large IMHO we should stick with verifyFormat.

Ok then I will change the tests accordingly. This reasoning should be written down somewhere.

In D98237#2643880, @MyDeveloperDay wrote:

I'd be quite interested to understand what the impact (if any) would be on javascript and C# formatting

I have not done anything in javascript a quick google search showed no such modifiers in classes. Do you have an example? C# seems to handle it like java and then the modifiers become properties of the functions member, there should be no influence.

[Max_S]

Cleanup of tests and additonal logic for modifiers at the end of a class.

[Max_S]

In the last update I fixed now everything and changed the tests.

The tests use now verify format when possible. A bug here, kept me from doing it in a few places. I submitted a bug report for this: https://bugs.llvm.org/show_bug.cgi?id=49772

I hope this is now fine and can be merged into the master.

clang/include/clang/Format/Format.h

1912

I submitted a bug report about this, since the behavior is different.

https://bugs.llvm.org/show_bug.cgi?id=49757

[MyDeveloperDay]

LGTM thank you for trying where you could to use verifyFormat

[curdeius]

LGTM, but please clean up the comments before landing.

clang/docs/ClangFormatStyleOptions.rst

2132

Please apply this when landing.

clang/unittests/Format/FormatTest.cpp

9289

Nit. Here and elsewhere.

9546

Please rephrase/move the comment to be clear that it concerns ELBAMS/ELAAMS.

9598

Nit.

9605

Please end comments with full stops. Here and elsewhere.

[Max_S]

Thank you for the reviews.

I addressed last minor remarks.

Do I need to worry about the build failures. (I do not think, that they are related to the changes.)

Do I need to do something to start the final merge?

[Max_S]

Just wanted to ask if there is something missing for the merge?

[curdeius]

I think you can land it. The issues seem indeed unrelated.
You might want to rebase and retest before landing to be sure.

[Max_S]

Thank you for the answer. It did not know that I have to land it myself.

Now I read up on https://llvm.org/docs/Phabricator.html

Tried to land via 'arc land' but I do not have the access rights.

So: Can someone please land the change for me. :)

[curdeius]

I'll land it for you. Could you please provide "Name Surname <email@org>" for commit attribution unless the info associated with your phabricator user is okay?

[Max_S]

Thank you for landing it. The information should be ok. But anyway: Max Sagebaum <[e-mail]>

[MyDeveloperDay]

When you made this commit you didn't regenerate the ClangFormatStyleOptions.rst from the Format.h using clang/docs/tool/dump_format_style.py now anyone else using the tool hit issues with code which is incorrect, we need to update Format.h to match what you expect in ClangFormatStyleOptions.rst

[Max_S]

In D98237#2775514, @MyDeveloperDay wrote:

When you made this commit you didn't regenerate the ClangFormatStyleOptions.rst from the Format.h using clang/docs/tool/dump_format_style.py now anyone else using the tool hit issues with code which is incorrect, we need to update Format.h to match what you expect in ClangFormatStyleOptions.rst

I am sorry about this. I provided a bugfix in D102989, can you please have a look if this is enough?

Is there a documentation I forget to read?

clang/docs/ClangFormatStyleOptions.rst

**DerivePointerAlignment** (``bool``)
   Pointer and reference alignment styles are going to be updated according
   to the preferences found in the file.
   ``PointerAlignment`` is then used only as fallback.
 
 **DisableFormat** (``bool``)
   Disables formatting completely.
 
 **EmptyLineBeforeAccessModifier** (``EmptyLineBeforeAccessModifierStyle``)
   Defines in which cases to put empty line before access modifiers.
 
   Possible values:
 
   * ``ELBAMS_Never`` (in configuration: ``Never``)
     Remove all empty lines before access modifiers.
 
     .. code-block:: c++
 
.. code-block:: c++
       };
 
   * ``ELBAMS_Leave`` (in configuration: ``Leave``)
     Keep existing empty lines before access modifiers.
 
   * ``ELBAMS_LogicalBlock`` (in configuration: ``LogicalBlock``)
     Add empty line only when access modifier starts a new logical block.
     Logical block is a group of one or more member fields or functions.
 
     .. code-block:: c++
 
       struct foo {
       private:
         int i;
 
       protected:
         int j;
.. code-block:: c++
       public:
         foo() {}
 
       private:
 
       protected:
       };
 
+**EmptyLinesAfterAccessModifier** (``unsigned``)
+  Defines how many lines are put after access modifiers.
 
+  .. code-block:: c++
+
+     0:                                     1:
+     struct Foo {                   vs.     struct Foo {
+     private:                               private:
+       int x;
+     };                                       int x;
+                                            };
 
 **ExperimentalAutoDetectBinPacking** (``bool``)
   If ``true``, clang-format detects whether function calls and
   definitions are formatted with one parameter per line.
 
   Each call can be bin-packed, one-per-line or inconclusive. If it is
   inconclusive, e.g. completely on one line, but a decision needs to be
   made, clang-format analyzes whether there are other bin-packed cases in

clang/include/clang/Format/Format.h

enum EmptyLineBeforeAccessModifierStyle : unsigned char {
     ///   public:
     ///     foo() {}
     ///   private:
     ///   protected:
     ///   };
     /// \endcode
     ELBAMS_Never,
     /// Keep existing empty lines before access modifiers.
     ELBAMS_Leave,
     /// Add empty line only when access modifier starts a new logical block.
     /// Logical block is a group of one or more member fields or functions.
     /// \code
     ///   struct foo {
     ///   private:
     ///     int i;
     ///
     ///   protected:
     ///     int j;
     ///     /* comment */
enum EmptyLineBeforeAccessModifierStyle : unsigned char {
     ///   };
     /// \endcode
     ELBAMS_Always,
   };
 
   /// Defines in which cases to put empty line before access modifiers.
   EmptyLineBeforeAccessModifierStyle EmptyLineBeforeAccessModifier;
 
+  /// Defines how many lines are put after access modifiers.
+  unsigned EmptyLinesAfterAccessModifier;
+
   /// If ``true``, clang-format detects whether function calls and
   /// definitions are formatted with one parameter per line.
   ///
   /// Each call can be bin-packed, one-per-line or inconclusive. If it is
   /// inconclusive, e.g. completely on one line, but a decision needs to be
   /// made, clang-format analyzes whether there are other bin-packed cases in
   /// the input file and act accordingly.
   ///
return AccessModifierOffset == R.AccessModifierOffset &&
            ConstructorInitializerIndentWidth ==
                R.ConstructorInitializerIndentWidth &&
            ContinuationIndentWidth == R.ContinuationIndentWidth &&
            Cpp11BracedListStyle == R.Cpp11BracedListStyle &&
            DeriveLineEnding == R.DeriveLineEnding &&
            DerivePointerAlignment == R.DerivePointerAlignment &&
            DisableFormat == R.DisableFormat &&
            EmptyLineBeforeAccessModifier == R.EmptyLineBeforeAccessModifier &&
+           EmptyLinesAfterAccessModifier == R.EmptyLinesAfterAccessModifier &&
            ExperimentalAutoDetectBinPacking ==
                R.ExperimentalAutoDetectBinPacking &&
            FixNamespaceComments == R.FixNamespaceComments &&
            ForEachMacros == R.ForEachMacros &&
            IncludeStyle.IncludeBlocks == R.IncludeStyle.IncludeBlocks &&
            IncludeStyle.IncludeCategories == R.IncludeStyle.IncludeCategories &&
            IncludeStyle.IncludeIsMainRegex ==
                R.IncludeStyle.IncludeIsMainRegex &&

clang/lib/Format/Format.cpp

IO.mapOptional("ConstructorInitializerIndentWidth",
                    Style.ConstructorInitializerIndentWidth);
     IO.mapOptional("ContinuationIndentWidth", Style.ContinuationIndentWidth);
     IO.mapOptional("Cpp11BracedListStyle", Style.Cpp11BracedListStyle);
     IO.mapOptional("DeriveLineEnding", Style.DeriveLineEnding);
     IO.mapOptional("DerivePointerAlignment", Style.DerivePointerAlignment);
     IO.mapOptional("DisableFormat", Style.DisableFormat);
     IO.mapOptional("EmptyLineBeforeAccessModifier",
                    Style.EmptyLineBeforeAccessModifier);
+    IO.mapOptional("EmptyLinesAfterAccessModifier",
+                   Style.EmptyLinesAfterAccessModifier);
     IO.mapOptional("ExperimentalAutoDetectBinPacking",
                    Style.ExperimentalAutoDetectBinPacking);
     IO.mapOptional("FixNamespaceComments", Style.FixNamespaceComments);
     IO.mapOptional("ForEachMacros", Style.ForEachMacros);
     IO.mapOptional("StatementAttributeLikeMacros",
                    Style.StatementAttributeLikeMacros);
     IO.mapOptional("IncludeBlocks", Style.IncludeStyle.IncludeBlocks);
     IO.mapOptional("IncludeCategories", Style.IncludeStyle.IncludeCategories);
FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
   LLVMStyle.CompactNamespaces = false;
   LLVMStyle.ConstructorInitializerAllOnOneLineOrOnePerLine = false;
   LLVMStyle.ConstructorInitializerIndentWidth = 4;
   LLVMStyle.ContinuationIndentWidth = 4;
   LLVMStyle.Cpp11BracedListStyle = true;
   LLVMStyle.DeriveLineEnding = true;
   LLVMStyle.DerivePointerAlignment = false;
   LLVMStyle.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_LogicalBlock;
+  LLVMStyle.EmptyLinesAfterAccessModifier = 0u;
   LLVMStyle.ExperimentalAutoDetectBinPacking = false;
   LLVMStyle.FixNamespaceComments = true;
   LLVMStyle.ForEachMacros.push_back("foreach");
   LLVMStyle.ForEachMacros.push_back("Q_FOREACH");
   LLVMStyle.ForEachMacros.push_back("BOOST_FOREACH");
   LLVMStyle.IncludeStyle.IncludeCategories = {
       {"^\"(llvm|llvm-c|clang|clang-c)/", 2, 0, false},
       {"^(<|\"(gtest|gmock|isl|json)/)", 3, 0, false},

clang/lib/Format/UnwrappedLineFormatter.cpp

case FormatStyle::ELBAMS_Always: {
         Newlines = 2;
     } break;
     }
   }
 
   // Remove empty lines after access specifiers.
   if (PreviousLine && PreviousLine->First->isAccessSpecifier() &&
       (!PreviousLine->InPPDirective || !RootToken.HasUnescapedNewline))
-    Newlines = std::min(1u, Newlines);
+    Newlines = Style.EmptyLinesAfterAccessModifier + 1u;
 
   if (Newlines)
     Indent = NewlineIndent;
 
   // Preprocessor directives get indented before the hash only if specified
   if (Style.IndentPPDirectives != FormatStyle::PPDIS_BeforeHash &&
       (Line.Type == LT_PreprocessorDirective ||
        Line.Type == LT_ImportStatement))

clang/unittests/Format/FormatTest.cpp

verifyFormat("struct foo {\n"
                "#ifdef FOO\n"
                "private:\n"
                "#endif\n"
                "  int j;\n"
                "};\n",
                Style);
 }
 
+TEST_F(FormatTest, FormatsAfterAccessModifiers) {
+  verifyFormat("class Foo {\n"
+               "private:\n"
+               "  int i;\n"
+               "};");
+
+  FormatStyle StyleWithLine = getLLVMStyle();
+  StyleWithLine.EmptyLinesAfterAccessModifier = 1u;
+  verifyFormat("class Foo {\n"
+               "private:\n"
+               "\n"
+               "  int i;\n"
+               "};", StyleWithLine);
+}
+
 TEST_F(FormatTest, FormatsArrays) {
   verifyFormat("aaaaaaaaaaaaaaaaaaaaaaaaa[aaaaaaaaaaaaaaaaaaaaaaaaa]\n"
                "                         [bbbbbbbbbbbbbbbbbbbbbbbbb] = c;");
   verifyFormat("aaaaaaaaaaaaaaaaaaaaaaaaa[aaaaaaaaaaa(aaaaaaaaaaaa)]\n"
                "                         [bbbbbbbbbbb(bbbbbbbbbbbb)] = c;");
   verifyFormat("if (aaaaaaaaaaaaaaaaaaaaaaaa &&\n"
                "    aaaaaaaaaaaaaaaaaaa[aaaaaaaaaaaaa][aaaaaaaaaaaaa]) {\n}");
   verifyFormat("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
                "    [bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb] = ccccccccccc;");
   verifyFormat("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
                "    [a][bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb] = cccccccc;");
   verifyFormat("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
                "    [aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa]\n"
                "    [bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb] = ccccccccccc;");
   verifyFormat(
       "llvm::outs() << \"aaaaaaaaaaaa: \"\n"
       "             << (*aaaaaaaiaaaaaaa)[aaaaaaaaaaaaaaaaaaaaaaaaa]\n"
       "                                  [aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa];");
   verifyFormat("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa[aaaaaaaaaaaaaaaaa][a]\n"
                "    .aaaaaaaaaaaaaaaaaaaaaa();");
 
   verifyGoogleFormat("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa<int>\n"
                      "    aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa[aaaaaaaaaaaa];");
   verifyFormat(
verifyFormat("import::SomeFunction(aaaaaaaaaaaaaaaaaaaaaaaaaaa,\n"
                "                     aaaaaaaaaaaaaaaaaaaaaaaaaaaaa);");
 }
 
 //===----------------------------------------------------------------------===//
 // Error recovery tests.
 //===----------------------------------------------------------------------===//
 
 TEST_F(FormatTest, IncompleteParameterLists) {
   FormatStyle NoBinPacking = getLLVMStyle();
   NoBinPacking.BinPackParameters = false;
   verifyFormat("void aaaaaaaaaaaaaaaaaa(int level,\n"
                "                        double *min_x,\n"
                "                        double *max_x,\n"
                "                        double *min_y,\n"
                "                        double *max_y,\n"
                "                        double *min_z,\n"
                "                        double *max_z, ) {}",
verifyFormat("do {\n}\n"
                "wheeee(fun);");
   verifyFormat("do {\n"
                "  f();\n"
                "}");
 }
 
 TEST_F(FormatTest, IncorrectCodeMissingParens) {
   verifyFormat("if {\n  foo;\n  foo();\n}");
   verifyFormat("switch {\n  foo;\n  foo();\n}");
   verifyIncompleteFormat("for {\n  foo;\n  foo();\n}");
   verifyFormat("while {\n  foo;\n  foo();\n}");
   verifyFormat("do {\n  foo;\n  foo();\n} while;");
 }
 
 TEST_F(FormatTest, DoesNotTouchUnwrappedLinesWithErrors) {
   verifyIncompleteFormat("namespace {\n"
                          "class Foo { Foo (\n"
verifyFormat("const Aaaaaa aaaaa = {\n"
                "    bbbbb,\n"
                "    ccccc,\n"
                "    ddddd,\n"
                "    eeeee,\n"
                "    ffffff,\n"
                "    ggggg,\n"
                "    hhhhhh,\n"
                "    iiiiii,\n"
                "    jjjjjj,\n"
                "    kkkkkk,\n"
                "};",
                NoBinPacking);
   verifyFormat(
       "const Aaaaaa aaaaa = {\n"
       "    aaaaa,  bbbbb,  ccccc,  ddddd,  eeeee,  ffffff, ggggg, hhhhhh,\n"
       "    iiiiii, jjjjjj, kkkkkk, aaaaa,  bbbbb,  ccccc,  ddddd, eeeee,\n"
       "    ffffff, ggggg,  hhhhhh, iiiiii, jjjjjj, kkkkkk,\n"
EXPECT_EQ("std::vector<int> MyNumbers{\n"
             "    // Second element:\n"
             "    2};",
             format("std::vector<int> MyNumbers{// First element:\n"
                    "                           1,\n"
                    "                           // Second element:\n"
                    "                           2};",
                    getLLVMStyleWithColumns(30)));
   // A trailing comma should still lead to an enforced line break and no
   // binpacking.
   EXPECT_EQ("vector<int> SomeVector = {\n"
             "    // aaa\n"
             "    1,\n"
             "    2,\n"
             "};",
             format("vector<int> SomeVector = { // aaa\n"
                    "    1, 2, };"));
 
   // C++11 brace initializer list l-braces should not be treated any differently
   // when breaking before lambda bodies is enabled
   FormatStyle BreakBeforeLambdaBody = getLLVMStyle();
   BreakBeforeLambdaBody.BreakBeforeBraces = FormatStyle::BS_Custom;
   BreakBeforeLambdaBody.BraceWrapping.BeforeLambdaBody = true;
   BreakBeforeLambdaBody.AlwaysBreakBeforeMultilineStrings = true;
   verifyFormat(