[clang-format] Add style to separate definition blocks

[ksyx]

Summary

This commit resolves GitHub issue #45895 (Bugzilla #46550), to add empty line between definition blocks including namespaces, classes, enums, and structs.

[ksyx]

Test code:

// test.cpp
#include <algorithm>
#include <cstdio>
#include <cstring>
struct Foo {
  int a,b,c;
};
namespace Ns {
class Bar {
public:
  struct Foobar {
    int a;
    int b;
  };
private:
  int t;
  int method1()
  {
    // Intentional test for different
    // line breaking mode of brackets
  }
  template<typename T>
  int method2(T x) {
    // ...
  }
  int method3(int par) {}
};
class C {
};
}
namespace Ns2 {

}

Tests ran:

# pwd = build directory
bin/clang-format test.cpp # LLVM Style
bin/clang-format -style='{SeparateDefinitionBlocks: false}' test.cpp # Disable functionality
bin/clang-format -style='{BasedOnStyle: Google}' test.cpp # Disable by default
bin/clang-format -style='{BasedOnStyle: Google, SeparateDefinitionBlocks: true}' test.cpp # Override default
bin/clang-format -style='{BasedOnStyle: WebKit}' test.cpp # Opening bracket in new line (WebKit style has this functionality enabled)

[ksyx]

Apply clang-format suggestions

[ksyx]

clang/lib/Format/Format.cpp

1207

Not so sure about this as it would break some tests.

[ksyx]

clang/lib/Format/Format.cpp

1207

As CI shows some original tests does not expect this empty line, but it looks like LLVM is generally adding empty lines between blocks.

cpp
 template <typename T> class B;
+
 class A {
 public:
   int f();
-};
+};\n

[ksyx]

Fix missing semicolon in sample/test code

[MyDeveloperDay]

clang/lib/Format/Format.cpp

1207

This needs to be an enumeration where leave is the default

[ksyx]

• Change bool type of the option to enum type
• Generate empty line for enum block

Testfile:

// test2.cpp
#include <algorithm>
#include <cstdio>
#include <cstring>
struct Foo {
  int a,b,c;
};
namespace Ns {
class Bar {
public:
  struct Foobar {
    int a;
    int b;
  };
private:
  int t;
  int method1()
  {
    // Intentional test for different
    // line breaking mode of brackets
  }
  template<typename T>
  int method2(T x) {
    // ...
  }
  enum Foo {
    FOO,
    BAR
  };
  int method3(int par) {}
};
class C {
};
}
namespace Ns2 {

}

Tests:

bin/clang-format test2.cpp # Test default (leave as it is)
bin/clang-format -style='{SeparateDefinitionBlocks: Between}' test2.cpp # Test normal functionality
bin/clang-format -style='{SeparateDefinitionBlocks: Leave}' test2.cpp # Test turning off
bin/clang-format -style='{SeparateDefinitionBlocks: Between, BasedOnStyle: WebKit}' test2.cpp # Case: opening bracket { has its own line

[curdeius]

Thanks for working on this.
Please add unit tests that verify the expected behaviour. Keep them simple. You can inspire yourself from existing tests.

clang/docs/ClangFormatStyleOptions.rst

3398

Unless it takes a lot of time to review this patch 🙂

clang/lib/Format/Format.cpp

2023

Please consider moving it to a different file.

[MyDeveloperDay]

clang/docs/ClangFormatStyleOptions.rst

3404

This enumeration surely needs 3 settings

Leave,     // don't touch the code, if I have a line leave it alone if not don't add one
Always,   // always add a line
Never,    // never add a line and remove the lint if one exists

[HazardyKnusperkeks]

clang/lib/Format/Format.cpp

2023

Yes please.

2110

Why only add the first replacement?

[ksyx]

• Change version from 15 to 14
• Add option SDS_Never, rename SDS_Between => SDS_Always
• Add unit tests
• Separate class into new file

[ksyx]

clang/lib/Format/Format.cpp

2110

This worked well as the Error class add method returned has overloaded boolean operator, returning true (nonzero) when failing, which simulates program exit code scheme? (ref: Doxygen/Error/operator bool)

[HazardyKnusperkeks]

clang/include/clang/Format/Format.h

3054

Add full stop at the end of sentences.

clang/lib/Format/DefinitionBlockSeparator.cpp

55

Here also full stop, please. And following comments.

clang/lib/Format/DefinitionBlockSeparator.h

38–39

I know you copied it. It is wrong where you copied it from. :)

clang/lib/Format/Format.cpp

2110

Okay, but maybe safe the result in Error to make it clearer.

[ksyx]

• Apply suggestions from clangfmt
• Add missing period of comments
• Fix namespace ending comment
• Add comment for notes on return value of WhitespaceManager's method

[ksyx]

Fix typo.

[curdeius]

Nice job. Some minor comments.
Please don't forget to reformat too.

clang/docs/ClangFormatStyleOptions.rst

3415

Keep it consistent please.

3418

Please reformat the code, e.g. on the left misses spaces (near braces and commas for instance). The difference between left and right should only be blank lines between blocks.
Of course, do it in Format.h and regenerate the RST.

clang/include/clang/Format/Format.h

3054

3063

Why not C and other similar languages?
Also, you don't seem to be limiting the pass to C++. Please add some tests to other languages.

3065

Actuay, do you need this line?

4080

4082

?

clang/unittests/Format/DefinitionBlockSeparatorTest.cpp

49

Maybe you can add verifyFormat helper as in other files? So that you don't need to repeat the code in some cases and we test the code formatting stability as well.

[HazardyKnusperkeks]

clang/lib/Format/DefinitionBlockSeparator.h

38–39

Not done.

[ksyx]

• Improve compatibility to other languages
• Improve unit test by test format stability, inverse result, and generate some input from expected output
• Reformat sample code
• Use more concise word for SDS_Leave

[ksyx]

Apply clangfmt's suggestions

[ksyx]

Apply suggestion from clangfmt

[HazardyKnusperkeks]

clang/include/clang/Format/Format.h

3104–3105

clang/lib/Format/DefinitionBlockSeparator.cpp

55

Not done. :)

[ksyx]

minor fixes.

[HazardyKnusperkeks]

Only some small changes, then it looks good to me. And I will definitely use it, once it is landed.

clang/include/clang/Format/Format.h

3056

Right?

4088

The only use of this function I found is in the tests and it sets the argument, or am I mistaken?

clang/lib/Format/DefinitionBlockSeparator.cpp

34–35

It should never be instantiated with that.

36

60

64

77

81

94

129

clang/lib/Format/Format.cpp

455

Please try to find a better place, regarding sorting (yeah the are some mismatches).

783

Please sort before ShortNamespaceLines

3064

clang/lib/Format/WhitespaceManager.h

48

[ksyx]

• Apply review suggestions.
• Assert the style is not SDS_Leave in private method, while return no change in public method if so.
• Rename loop variable Line to CurrentLine

clang/include/clang/Format/Format.h

4088

I suppose this function might also be called from some users of the library other than clangfmt itself? Besides, the declarations nearby are setting this default value.

clang/lib/Format/DefinitionBlockSeparator.cpp

36

Some other lambdas I found use lowercased leading letter, as it looks like a function (call).

[HazardyKnusperkeks]

clang/lib/Format/DefinitionBlockSeparator.cpp

26

Better, but I still think we should have the assert here. The class should not be instantiated at all, if you don't want to use it.

36

Okay, then I don't know what the LLVM Style is, probably there is none in this regard. I think the lambdas I saw in clang-format are with a capital letter.

[ksyx]

Change lambda variable names' leading letter to uppercase.

[ksyx]

Change lambda variable names' leading letter to uppercase.

clang/lib/Format/DefinitionBlockSeparator.cpp

26

My idea is that it would be too redundant to have every user of this method to check style once before it calls it?

36

Ok. Then I will make changes.

[HazardyKnusperkeks]

clang/lib/Format/DefinitionBlockSeparator.cpp

26

Who are the users? clang-format and maybe its tests. And the only one that really matters is clang-format, which does the check.

clang/unittests/Format/DefinitionBlockSeparatorTest.cpp

46

Maybe one should either use the normal reformat function, or additionally. Because right now we do not test any interferences of these two functions.

[ksyx]

Use reformat method instead of calling the single method only.

[ksyx]

clang/lib/Format/DefinitionBlockSeparator.cpp

26

Does there exist the possibility that some developer just include the header and link to library to obtain replacement analysis result for their other parts of program to use, instead of only clangfmt itself is using this class?

[HazardyKnusperkeks]

clang/lib/Format/DefinitionBlockSeparator.cpp

26

Of course the possibility exists, but I don't think there is anyone. And at least for me those aren't the audience. And even then, they can make sure that it's not SDS_Leave.

[ksyx]

Change the SDS_Leave style check to be assert not having this style.

[MyDeveloperDay]

LGTM thank you for this patch, I'll also definately use this myself.

clang/docs/ClangFormatStyleOptions.rst

3398

EmptyLinesBetweenBlocks?

[ksyx]

clang/docs/ClangFormatStyleOptions.rst

3398

It feel like for me that it is implying that there will also be empty lines surrounding blocks of, for example, for and if?

[MyDeveloperDay]

Can you add a release note to docs/ReleaseNotes.rst

[ksyx]

Add release note entry.

[curdeius]

LGTM.

[MyDeveloperDay]

I'm seeing some odd behaviour here..

void foo() {}
/* ABC */
void bar() {}

becomes

void foo() {}

/* ABC */

void bar() {}

If the ABC comment is a doxygen style comment it doesn't make sense to separate it from the function below

logged as https://github.com/llvm/llvm-project/issues/52976

[MyDeveloperDay]

@ksyx

Did you see this issue https://github.com/llvm/llvm-project/issues/52976

In its current form, this patch causes huge amounts of flux in projects
that document code like (anyone using doxygen likely, + most normal people
;-) )

...
}

// My function
<<<<-------- extra newline added
void
foo()
{
}

as it will place and undesirable additional newline between the function
and the return type.

We really need this to be resolved before we branch out v14 (which may be
imminent)

MyDeveloperDay

[ksyx]

In D116314#3221488, @MyDeveloperDay wrote:

@ksyx

Did you see this issue https://github.com/llvm/llvm-project/issues/52976

In its current form, this patch causes huge amounts of flux in projects
that document code like (anyone using doxygen likely, + most normal people
;-) )

...
}

// My function
<<<<-------- extra newline added
void
foo()
{
}

as it will place and undesirable additional newline between the function
and the return type.

We really need this to be resolved before we branch out v14 (which may be
imminent)

MyDeveloperDay

Thanks for the feedback! I have proposed some fix in D116663, and please see if this works.

clang/docs/ClangFormatStyleOptions.rst

.. code-block:: c++
      /* second veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of information */
 
      true:
      // veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of
      // information
      /* second veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of
       * information */
 
+**SeparateDefinitionBlocks** (``SeparateDefinitionStyle``) :versionbadge:`clang-format 14`
+  Specifies the use of empty lines to separate definition blocks, including classes,
+  structs, enum, and functions, for C++ language.
+
+  Possible values:
+
+  * ``SDS_Leave`` (in configuration: ``Leave``)
+    Leave definition blocks separated as they are, without changing anything.
+
+  * ``SDS_Always`` (in configuration: ``Always``)
+    Insert empty lines between definition blocks.
+
+  * ``SDS_Never`` (in configuration: ``Never``)
+    Remove any empty line between definition blocks.
+
+  .. code-block:: c++
+
+     SeparateDefinitions
+     Never                v.s.   Always
+     #include <cstring>          #include <cstring>
+     struct Foo{
+       int a,b,c;                struct Foo {
+     };                            int a, b, c;
+     namespace Ns {              };
+     class Bar {
+     public:                     namespace Ns {
+       struct Foobar {           class Bar {
+         int a;                  public:
+         int b;                    struct Foobar {
+       };                            int a;
+     private:                        int b;
+       int t;                      };
+       int method1() {
+         // ...                  private:
+       }                           int t;
+       template<typename T>
+       int method2(T x) {          int method1() {
+         // ...                      // ...
+       }                           }
+       int method3(int par) {}
+     };                            template <typename T> int method2(T x) {
+     class C {                       // ...
+     };                            }
+     }
+                                   int method3(int par) {}
+                                 };
+
+                                 class C {};
+                                 } // namespace Ns
+
 **ShortNamespaceLines** (``Unsigned``) :versionbadge:`clang-format 14`
   The maximal number of unwrapped lines that a short namespace spans.
   Defaults to 1.
 
   This determines the maximum length of short namespaces by counting
   unwrapped lines (i.e. containing neither opening nor closing
   namespace brace) and makes "FixNamespaceComments" omit adding
   end comments for those.

clang/include/clang/Format/Format.h

///    NS_TABLE_.*_END$"
   ///    // information
   ///    /* second veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of
   ///     * information */
   /// \endcode
   /// \version 4
   bool ReflowComments;
   // clang-format on
 
+  enum SeparateDefinitionStyle {
+    /// Leave definition blocks separated as they are without changing anything
+    SDS_Leave,
+    /// Insert empty lines between definition blocks
+    SDS_Always,
+    /// Remove any empty line between definition blocks
+    SDS_Never
+  };
+
+  /// Specifies the use of empty lines to separate definition blocks, including
+  /// classes, structs, enum, and functions, for C++ language.
+  /// \code
+  ///    SeparateDefinitions
+  ///    Never                v.s.   Always
+  ///    #include <cstring>          #include <cstring>
+  ///    struct Foo{
+  ///      int a,b,c;                struct Foo {
+  ///    };                            int a, b, c;
+  ///    namespace Ns {              };
+  ///    class Bar {
+  ///    public:                     namespace Ns {
+  ///      struct Foobar {           class Bar {
+  ///        int a;                  public:
+  ///        int b;                    struct Foobar {
+  ///      };                            int a;
+  ///    private:                        int b;
+  ///      int t;                      };
+  ///      int method1() {
+  ///        // ...                  private:
+  ///      }                           int t;
+  ///      template<typename T>
+  ///      int method2(T x) {          int method1() {
+  ///        // ...                      // ...
+  ///      }                           }
+  ///      int method3(int par) {}
+  ///    };                            template <typename T> int method2(T x) {
+  ///    class C {                       // ...
+  ///    };                            }
+  ///    }
+  ///                                  int method3(int par) {}
+  ///                                };
+  ///
+  ///                                class C {};
+  ///                                } // namespace Ns
+  /// \endcode
+  /// \version 14
+  SeparateDefinitionStyle SeparateDefinitionBlocks;
+
   /// The maximal number of unwrapped lines that a short namespace spans.
   /// Defaults to 1.
   ///
   /// This determines the maximum length of short namespaces by counting
   /// unwrapped lines (i.e. containing neither opening nor closing
   /// namespace brace) and makes "FixNamespaceComments" omit adding
   /// end comments for those.
   /// \code
   ///    ShortNamespaceLines: 1     vs.     ShortNamespaceLines: 0
   ///    namespace a {                      namespace a {
   ///      int foo;                           int foo;
   ///    }                                  } // namespace a
   ///

 ///
 /// Returns the ``Replacements`` that fix the namespace comments in all
 /// \p Ranges in \p Code.
 tooling::Replacements fixNamespaceEndComments(const FormatStyle &Style,
                                               StringRef Code,
                                               ArrayRef<tooling::Range> Ranges,
                                               StringRef FileName = "<stdin>");
 
+/// Use empty line to separate definition blocks including classes, structs,
+/// functions, namespaces, and enum in the given \p Ranges in \p Code.
+///
+/// Returns the ``Replacements`` that inserts empty lines to separate definition
+/// blocks in all \p Ranges in \p Code.
+tooling::Replacements separateDefinitionBlocks(const FormatStyle &Style,
+                                               StringRef Code,
+                                               ArrayRef<tooling::Range> Ranges,
+                                               StringRef FileName = "<stdin>");
+
 /// Sort consecutive using declarations in the given \p Ranges in
 /// \p Code.
 ///
 /// Returns the ``Replacements`` that sort the using declarations in all
 /// \p Ranges in \p Code.
 tooling::Replacements sortUsingDeclarations(const FormatStyle &Style,
                                             StringRef Code,
                                             ArrayRef<tooling::Range> Ranges,

clang/lib/Format/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangFormat
   AffectedRangeManager.cpp
   BreakableToken.cpp
   ContinuationIndenter.cpp
+  DefinitionBlockSeparator.cpp
   Format.cpp
   FormatToken.cpp
   FormatTokenLexer.cpp
   MacroExpander.cpp
   NamespaceEndCommentsFixer.cpp
   QualifierAlignmentFixer.cpp
   SortJavaScriptImports.cpp
   TokenAnalyzer.cpp

clang/lib/Format/DefinitionBlockSeparator.h

+//===--- DefinitionBlockSeparator.h -----------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// This file declares DefinitionBlockSeparator, a TokenAnalyzer that separates
+/// definition blocks like classes, structs, functions, and namespaces by
+/// inserting an empty line in between for C++ language.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_LIB_FORMAT_DEFINITIONBLOCKSEPARATOR_H
+#define LLVM_CLANG_LIB_FORMAT_DEFINITIONBLOCKSEPARATOR_H
+
+#include "TokenAnalyzer.h"
+#include "WhitespaceManager.h"
+
+namespace clang {
+namespace format {
+class DefinitionBlockSeparator : public TokenAnalyzer {
+public:
+  DefinitionBlockSeparator(const Environment &Env, const FormatStyle &Style)
+      : TokenAnalyzer(Env, Style) {}
+
+  std::pair<tooling::Replacements, unsigned>
+  analyze(TokenAnnotator &Annotator,
+          SmallVectorImpl<AnnotatedLine *> &AnnotatedLines,
+          FormatTokenLexer &Tokens) override;
+
+private:
+  void separateBlocks(SmallVectorImpl<AnnotatedLine *> &Lines,
+                      tooling::Replacements &Result);
+};
+} // end namespace format
+} // end namespace clang
+
+#endif

clang/lib/Format/DefinitionBlockSeparator.cpp

+//===--- DefinitionBlockSeparator.cpp ---------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// This file implements DefinitionBlockSeparator, a TokenAnalyzer that
+/// separates definition blocks like classes, structs, functions, and namespaces
+/// by inserting an empty line in between for C++ language.
+///
+//===----------------------------------------------------------------------===//
+
+#include "DefinitionBlockSeparator.h"
+#include "llvm/Support/Debug.h"
+#define DEBUG_TYPE "definition-block-separator"
+
+namespace clang {
+namespace format {
+  std::pair<tooling::Replacements, unsigned>
+  DefinitionBlockSeparator::analyze(TokenAnnotator &Annotator,
+          SmallVectorImpl<AnnotatedLine *> &AnnotatedLines,
+          FormatTokenLexer &Tokens) {
+    AffectedRangeMgr.computeAffectedLines(AnnotatedLines);
+    tooling::Replacements Result;
+    separateBlocks(AnnotatedLines, Result);
+    return {Result, 0};
+  }
+
+  void
+  DefinitionBlockSeparator::separateBlocks(SmallVectorImpl<AnnotatedLine *> &Lines,
+                                      tooling::Replacements &Result) {
+    if (Style.SeparateDefinitionBlocks == FormatStyle::SDS_Leave) return;
+    auto likelyDefinition = [](AnnotatedLine *Line) {
+      return (Line->MightBeFunctionDecl && Line->mightBeFunctionDefinition()) ||
+             Line->First->isOneOf(tok::kw_class, tok::kw_struct,
+                                  tok::kw_namespace, tok::kw_enum);
+    };
+    unsigned NewlineCount =
+        (Style.SeparateDefinitionBlocks == FormatStyle::SDS_Always ? 1 : 0) + 1;
+    WhitespaceManager Whitespaces(
+        Env.getSourceManager(), Style,
+        Style.DeriveLineEnding
+            ? WhitespaceManager::inputUsesCRLF(
+                  Env.getSourceManager().getBufferData(Env.getFileID()),
+                  Style.UseCRLF)
+            : Style.UseCRLF);
+    for (unsigned I = 0; I < Lines.size(); I++) {
+      auto Line = Lines[I];
+      FormatToken *TargetToken = nullptr;
+      auto OpeningLineIndex = Line->MatchingOpeningBlockLineIndex;
+      auto insertReplacement = [&]() {
+        // Do not handle EOF newlines
+        assert(TargetToken);
+        if (TargetToken->is(tok::eof)) return;
+        Whitespaces.replaceWhitespace(*TargetToken, NewlineCount,
+                                      TargetToken->SpacesRequiredBefore - 1,
+                                      TargetToken->StartsColumn);
+      };
+      auto followingOtherOpening = [&]() {
+        return OpeningLineIndex == 0 ||
+               Lines[OpeningLineIndex - 1]->Last->opensScope();
+      };
+
+      if (Line->First->closesScope()) {
+        // Case: Opening bracket has its own line
+        if (OpeningLineIndex > 0 &&
+            Lines[OpeningLineIndex]->First->TokenText == "{") {
+          OpeningLineIndex--;
+        }
+        AnnotatedLine *OpeningLine = Lines[OpeningLineIndex];
+        // Closing a function definition
+        if (likelyDefinition(OpeningLine)) {
+          AnnotatedLine *TargetLine;
+          // Not the first token
+          if (!followingOtherOpening()) {
+              TargetLine = OpeningLine;
+              TargetToken = TargetLine->First;
+
+              // Avoid duplicated replacement
+              if (TargetToken && !TargetToken->opensScope()) 
+                insertReplacement();
+          }
+
+          // Not the last token
+          if (I + 1 < Lines.size()) {
+            TargetLine = Lines[I + 1];
+            TargetToken = TargetLine->First;
+
+            // Not continuously closing scopes (e.g. function + class +
+            // namespace); The token will be handled in another case if
+            // it is a definition line
+            if (TargetLine->Affected && !TargetToken->closesScope() &&
+                !likelyDefinition(TargetLine)) {
+              insertReplacement();
+            }
+          }
+        }
+      }
+      else if (Line->First->is(tok::kw_enum)) {
+        OpeningLineIndex = I;
+        TargetToken = Line->First;
+        if (!followingOtherOpening())
+          insertReplacement();
+      }
+    }
+    for (const auto &R : Whitespaces.generateReplacements())
+      if (Result.add(R))
+        return;
+  }
+} // namespace format
+} // namespace clang

clang/lib/Format/Format.cpp

 /// split into separate files as we go.
 ///
 //===----------------------------------------------------------------------===//
 
 #include "clang/Format/Format.h"
 #include "AffectedRangeManager.h"
 #include "BreakableToken.h"
 #include "ContinuationIndenter.h"
+#include "DefinitionBlockSeparator.h"
 #include "FormatInternal.h"
 #include "FormatTokenLexer.h"
 #include "NamespaceEndCommentsFixer.h"
 #include "QualifierAlignmentFixer.h"
 #include "SortJavaScriptImports.h"
 #include "TokenAnalyzer.h"
 #include "TokenAnnotator.h"
 #include "UnwrappedLineFormatter.h"
static void enumeration(IO &IO, FormatStyle::ReferenceAlignmentStyle &Value) {
     IO.enumCase(Value, "Pointer", FormatStyle::RAS_Pointer);
     IO.enumCase(Value, "Middle", FormatStyle::RAS_Middle);
     IO.enumCase(Value, "Left", FormatStyle::RAS_Left);
     IO.enumCase(Value, "Right", FormatStyle::RAS_Right);
   }
 };
 
 template <>
+struct ScalarEnumerationTraits<FormatStyle::SeparateDefinitionStyle> {
+  static void enumeration(IO &IO, FormatStyle::SeparateDefinitionStyle &Value) {
+    IO.enumCase(Value, "Leave", FormatStyle::SDS_Leave);
+    IO.enumCase(Value, "Always", FormatStyle::SDS_Always);
+    IO.enumCase(Value, "Never", FormatStyle::SDS_Never);
+  }
+};
+
+template <>
 struct ScalarEnumerationTraits<FormatStyle::SpaceBeforeParensStyle> {
   static void enumeration(IO &IO, FormatStyle::SpaceBeforeParensStyle &Value) {
     IO.enumCase(Value, "Never", FormatStyle::SBPO_Never);
     IO.enumCase(Value, "ControlStatements",
                 FormatStyle::SBPO_ControlStatements);
     IO.enumCase(Value, "ControlStatementsExceptControlMacros",
                 FormatStyle::SBPO_ControlStatementsExceptControlMacros);
     IO.enumCase(Value, "NonEmptyParentheses",
static void mapping(IO &IO, FormatStyle &Style) {
     IO.mapOptional("PenaltyIndentedWhitespace",
                    Style.PenaltyIndentedWhitespace);
     IO.mapOptional("PointerAlignment", Style.PointerAlignment);
     IO.mapOptional("PPIndentWidth", Style.PPIndentWidth);
     IO.mapOptional("RawStringFormats", Style.RawStringFormats);
     IO.mapOptional("ReferenceAlignment", Style.ReferenceAlignment);
     IO.mapOptional("ReflowComments", Style.ReflowComments);
     IO.mapOptional("ShortNamespaceLines", Style.ShortNamespaceLines);
+    IO.mapOptional("SeparateDefinitionBlocks", Style.SeparateDefinitionBlocks);
     IO.mapOptional("SortIncludes", Style.SortIncludes);
     IO.mapOptional("SortJavaStaticImport", Style.SortJavaStaticImport);
     IO.mapOptional("SortUsingDeclarations", Style.SortUsingDeclarations);
     IO.mapOptional("SpaceAfterCStyleCast", Style.SpaceAfterCStyleCast);
     IO.mapOptional("SpaceAfterLogicalNot", Style.SpaceAfterLogicalNot);
     IO.mapOptional("SpaceAfterTemplateKeyword",
                    Style.SpaceAfterTemplateKeyword);
     IO.mapOptional("SpaceBeforeAssignmentOperators",
FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
   LLVMStyle.NamespaceIndentation = FormatStyle::NI_None;
   LLVMStyle.ObjCBinPackProtocolList = FormatStyle::BPS_Auto;
   LLVMStyle.ObjCBlockIndentWidth = 2;
   LLVMStyle.ObjCBreakBeforeNestedBlockParam = true;
   LLVMStyle.ObjCSpaceAfterProperty = false;
   LLVMStyle.ObjCSpaceBeforeProtocolList = true;
   LLVMStyle.PointerAlignment = FormatStyle::PAS_Right;
   LLVMStyle.ReferenceAlignment = FormatStyle::RAS_Pointer;
+  LLVMStyle.SeparateDefinitionBlocks = FormatStyle::SDS_Leave;
   LLVMStyle.ShortNamespaceLines = 1;
   LLVMStyle.SpacesBeforeTrailingComments = 1;
   LLVMStyle.Standard = FormatStyle::LS_Latest;
   LLVMStyle.UseCRLF = false;
   LLVMStyle.UseTab = FormatStyle::UT_Never;
   LLVMStyle.ReflowComments = true;
   LLVMStyle.SpacesInParentheses = false;
   LLVMStyle.SpacesInSquareBrackets = false;
analyze(TokenAnnotator &Annotator,
     for (unsigned i = 0, e = AnnotatedLines.size(); i != e; ++i) {
       Annotator.calculateFormattingInformation(*AnnotatedLines[i]);
     }
     Annotator.setCommentLineLevels(AnnotatedLines);
 
     WhitespaceManager Whitespaces(
         Env.getSourceManager(), Style,
         Style.DeriveLineEnding
-            ? inputUsesCRLF(
+            ? WhitespaceManager::inputUsesCRLF(
                   Env.getSourceManager().getBufferData(Env.getFileID()),
                   Style.UseCRLF)
             : Style.UseCRLF);
     ContinuationIndenter Indenter(Style, Tokens.getKeywords(),
                                   Env.getSourceManager(), Whitespaces, Encoding,
                                   BinPackInconclusiveFunctions);
     unsigned Penalty =
         UnwrappedLineFormatter(&Indenter, &Whitespaces, Style,
                                Tokens.getKeywords(), Env.getSourceManager(),
                                Status)
             .format(AnnotatedLines, /*DryRun=*/false,
                     /*AdditionalIndent=*/0,
                     /*FixBadIndentation=*/false,
                     /*FirstStartColumn=*/Env.getFirstStartColumn(),
                     /*NextStartColumn=*/Env.getNextStartColumn(),
                     /*LastStartColumn=*/Env.getLastStartColumn());
     for (const auto &R : Whitespaces.generateReplacements())
       if (Result.add(R))
         return std::make_pair(Result, 0);
     return std::make_pair(Result, Penalty);
   }
 
 private:
-  static bool inputUsesCRLF(StringRef Text, bool DefaultToCRLF) {
-    size_t LF = Text.count('\n');
-    size_t CR = Text.count('\r') * 2;
-    return LF == CR ? DefaultToCRLF : CR > LF;
-  }
-
   bool
   hasCpp03IncompatibleFormat(const SmallVectorImpl<AnnotatedLine *> &Lines) {
     for (const AnnotatedLine *Line : Lines) {
       if (hasCpp03IncompatibleFormat(Line->Children))
         return true;
       for (FormatToken *Tok = Line->First->Next; Tok; Tok = Tok->Next) {
         if (Tok->WhitespaceRange.getBegin() == Tok->WhitespaceRange.getEnd()) {
           if (Tok->is(tok::coloncolon) && Tok->Previous->is(TT_TemplateOpener))
for (AnnotatedLine *Line : Lines) {
             tooling::Replacement(Env.getSourceManager(), Start, 0, ",")));
       }
     }
   }
 };
 
 // This class clean up the erroneous/redundant code around the given ranges in
 // file.
 class Cleaner : public TokenAnalyzer {
 public:
   Cleaner(const Environment &Env, const FormatStyle &Style)
       : TokenAnalyzer(Env, Style),
         DeletedTokens(FormatTokenLess(Env.getSourceManager())) {}
 
   // FIXME: eliminate unused parameters.
   std::pair<tooling::Replacements, unsigned>
   analyze(TokenAnnotator &Annotator,
bool checkEmptyNamespace(SmallVectorImpl<AnnotatedLine *> &AnnotatedLines,
     unsigned InitLine = CurrentLine, End = AnnotatedLines.size();
     if (Style.BraceWrapping.AfterNamespace) {
       // If the left brace is in a new line, we should consume it first so that
       // it does not make the namespace non-empty.
       // FIXME: error handling if there is no left brace.
       if (!AnnotatedLines[++CurrentLine]->startsWith(tok::l_brace)) {
         NewLine = CurrentLine;
         return false;
       }
     } else if (!AnnotatedLines[CurrentLine]->endsWith(tok::l_brace)) {
       return false;
     }
     while (++CurrentLine < End) {
       if (AnnotatedLines[CurrentLine]->startsWith(tok::r_brace))
         break;
 
       if (AnnotatedLines[CurrentLine]->startsWithNamespace()) {
if (Style.FixNamespaceComments)
       Passes.emplace_back([&](const Environment &Env) {
         return NamespaceEndCommentsFixer(Env, Expanded).process();
       });
 
     if (Style.SortUsingDeclarations)
       Passes.emplace_back([&](const Environment &Env) {
         return UsingDeclarationsSorter(Env, Expanded).process();
       });
+
+    if (Style.SeparateDefinitionBlocks)
+      Passes.emplace_back([&](const Environment &Env) {
+        return DefinitionBlockSeparator(Env, Expanded).process();
+      });
   }
 
   if (Style.isJavaScript() && Style.JavaScriptQuotes != FormatStyle::JSQS_Leave)
     Passes.emplace_back([&](const Environment &Env) {
       return JavaScriptRequoter(Env, Expanded).process();
     });
 
   Passes.emplace_back([&](const Environment &Env) {
     return Formatter(Env, Expanded, Status).process();
   });
 
tooling::Replacements fixNamespaceEndComments(const FormatStyle &Style,
                                               ArrayRef<tooling::Range> Ranges,
                                               StringRef FileName) {
   auto Env = Environment::make(Code, FileName, Ranges);
   if (!Env)
     return {};
   return NamespaceEndCommentsFixer(*Env, Style).process().first;
 }
 
+tooling::Replacements separateDefinitionBlocks(const FormatStyle &Style,
+                                              StringRef Code,
+                                              ArrayRef<tooling::Range> Ranges,
+                                              StringRef FileName) {
+  auto Env = Environment::make(Code, FileName, Ranges);
+  if (!Env)
+    return {};
+  return DefinitionBlockSeparator(*Env, Style).process().first;
+}
+
 tooling::Replacements sortUsingDeclarations(const FormatStyle &Style,
                                             StringRef Code,
                                             ArrayRef<tooling::Range> Ranges,
                                             StringRef FileName) {
   auto Env = Environment::make(Code, FileName, Ranges);
   if (!Env)
     return {};
   return UsingDeclarationsSorter(*Env, Style).process().first;

clang/lib/Format/WhitespaceManager.h

 class WhitespaceManager {
 public:
   WhitespaceManager(const SourceManager &SourceMgr, const FormatStyle &Style,
                     bool UseCRLF)
       : SourceMgr(SourceMgr), Style(Style), UseCRLF(UseCRLF) {}
 
   bool useCRLF() const { return UseCRLF; }
 
+  /// Infers whether the input is using CRLF
+  static bool inputUsesCRLF(StringRef Text, bool DefaultToCRLF);
+
   /// Replaces the whitespace in front of \p Tok. Only call once for
   /// each \c AnnotatedToken.
   ///
   /// \p StartOfTokenColumn is the column at which the token will start after
   /// this replacement. It is needed for determining how \p Spaces is turned
   /// into tabs and spaces for some format styles.
   void replaceWhitespace(FormatToken &Tok, unsigned Newlines, unsigned Spaces,
                          unsigned StartOfTokenColumn, bool isAligned = false,

clang/lib/Format/WhitespaceManager.cpp

Changes.push_back(Change(Tok, /*CreateReplacement=*/false,
                            /*IsInsideToken=*/false));
 }
 
 llvm::Error
 WhitespaceManager::addReplacement(const tooling::Replacement &Replacement) {
   return Replaces.add(Replacement);
 }
 
+bool WhitespaceManager::inputUsesCRLF(StringRef Text, bool DefaultToCRLF) {
+  size_t LF = Text.count('\n');
+  size_t CR = Text.count('\r') * 2;
+  return LF == CR ? DefaultToCRLF : CR > LF;
+}
+
 void WhitespaceManager::replaceWhitespaceInToken(
     const FormatToken &Tok, unsigned Offset, unsigned ReplaceChars,
     StringRef PreviousPostfix, StringRef CurrentPrefix, bool InPPDirective,
     unsigned Newlines, int Spaces) {
   if (Tok.Finalized)
     return;
   SourceLocation Start = Tok.getStartOfNonWhitespace().getLocWithOffset(Offset);
   Changes.push_back(

clang/unittests/Format/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Support
   )
 
 add_clang_unittest(FormatTests
   CleanupTest.cpp
+  DefinitionBlockSeparatorTest.cpp
   FormatTest.cpp
   FormatTestComments.cpp
   FormatTestCSharp.cpp
   FormatTestJS.cpp
   FormatTestJava.cpp
   FormatTestJson.cpp
   FormatTestObjC.cpp
   FormatTestProto.cpp

clang/unittests/Format/DefinitionBlockSeparatorTest.cpp

+//===- DefinitionBlockSeparatorTest.cpp - Formatting unit tests -----------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Format/Format.h"
+
+#include "llvm/Support/Debug.h"
+#include "gtest/gtest.h"
+
+#define DEBUG_TYPE "definition-block-separator-test"
+
+namespace clang {
+namespace format {
+namespace {
+
+class DefinitionBlockSeparatorTest : public ::testing::Test {
+protected:
+  std::string
+  separateDefinitionBlocks(llvm::StringRef Code,
+                           const std::vector<tooling::Range> &Ranges,
+                           const FormatStyle &Style = getLLVMStyle()) {
+    LLVM_DEBUG(llvm::errs() << "---\n");
+    LLVM_DEBUG(llvm::errs() << Code << "\n\n");
+    tooling::Replacements Replaces =
+        clang::format::separateDefinitionBlocks(Style, Code, Ranges, "<stdin>");
+    auto Result = applyAllReplacements(Code, Replaces);
+    EXPECT_TRUE(static_cast<bool>(Result));
+    LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+    return *Result;
+  }
+
+  std::string
+  separateDefinitionBlocks(llvm::StringRef Code,
+                           const FormatStyle &Style = getLLVMStyle()) {
+    return separateDefinitionBlocks(
+        Code,
+        /*Ranges=*/{1, tooling::Range(0, Code.size())}, Style);
+  }
+};
+
+TEST_F(DefinitionBlockSeparatorTest, Basic) {
+  FormatStyle Style = getLLVMStyle();
+  Style.SeparateDefinitionBlocks = FormatStyle::SDS_Always;
+  EXPECT_EQ("int foo() {\n"
+            "  int i, j;\n"
+            "}\n"
+            "\n"
+            "int bar() {\n"
+            "  int j, k;\n"
+            "}",
+            separateDefinitionBlocks("int foo() {\n"
+                                     "  int i, j;\n"
+                                     "}\n"
+                                     "int bar() {\n"
+                                     "  int j, k;\n"
+                                     "}", Style));
+
+  EXPECT_EQ("struct foo {\n"
+            "  int i, j;\n"
+            "};\n"
+            "\n"
+            "struct bar {\n"
+            "  int j, k;\n"
+            "};",
+            separateDefinitionBlocks("struct foo {\n"
+                                     "  int i, j;\n"
+                                     "};\n"
+                                     "struct bar {\n"
+                                     "  int j, k;\n"
+                                     "};", Style));
+
+  EXPECT_EQ("class foo {\n"
+            "  int i, j;\n"
+            "};\n"
+            "\n"
+            "class bar {\n"
+            "  int j, k;\n"
+            "};",
+            separateDefinitionBlocks("class foo {\n"
+                                     "  int i, j;\n"
+                                     "};\n"
+                                     "class bar {\n"
+                                     "  int j, k;\n"
+                                     "};", Style));
+
+  EXPECT_EQ("namespace foo {\n"
+            "  int i, j;\n"
+            "}\n"
+            "\n"
+            "namespace bar {\n"
+            "  int j, k;\n"
+            "}",
+            separateDefinitionBlocks("namespace foo {\n"
+                                     "  int i, j;\n"
+                                     "}\n"
+                                     "namespace bar {\n"
+                                     "  int j, k;\n"
+                                     "}", Style));
+
+  EXPECT_EQ("enum Foo {\n"
+            "  FOO, BAR\n"
+            "};\n"
+            "\n"
+            "enum Bar {\n"
+            "  FOOBAR, BARFOO\n"
+            "};",
+            separateDefinitionBlocks("enum Foo {\n"
+                                     "  FOO, BAR\n"
+                                     "};\n"
+                                     "enum Bar {\n"
+                                     "  FOOBAR, BARFOO\n"
+                                     "};", Style));
+}
+
+TEST_F(DefinitionBlockSeparatorTest, Always) {
+  FormatStyle Style = getLLVMStyle();
+  Style.SeparateDefinitionBlocks = FormatStyle::SDS_Always;
+  EXPECT_EQ("enum Foo {\n"
+            "  FOO, BAR\n"
+            "};\n"
+            "\n"
+            "int foo() {\n"
+            "  int i, j;\n"
+            "}\n"
+            "\n"
+            "int i, j, k;\n"
+            "\n"
+            "int bar() {\n"
+            "  int j, k;\n"
+            "}\n"
+            "\n"
+            "enum Bar {\n"
+            "  FOOBAR, BARFOO\n"
+            "};",
+            separateDefinitionBlocks("enum Foo {\n"
+                                     "  FOO, BAR\n"
+                                     "};\n"
+                                     "int foo() {\n"
+                                     "  int i, j;\n"
+                                     "}\n"
+                                     "int i, j, k;\n"
+                                     "int bar() {\n"
+                                     "  int j, k;\n"
+                                     "}\n"
+                                     "enum Bar {\n"
+                                     "  FOOBAR, BARFOO\n"
+                                     "};", Style));
+}
+
+TEST_F(DefinitionBlockSeparatorTest, Never) {
+  FormatStyle Style = getLLVMStyle();
+  Style.SeparateDefinitionBlocks = FormatStyle::SDS_Never;
+  EXPECT_EQ("enum Foo {\n"
+            "  FOO, BAR\n"
+            "};\n"
+            "int foo() {\n"
+            "  int i, j;\n"
+            "}\n"
+            "int i, j, k;\n"
+            "int bar() {\n"
+            "  int j, k;\n"
+            "}\n"
+            "enum Bar {\n"
+            "  FOOBAR, BARFOO\n"
+            "};",
+            separateDefinitionBlocks("enum Foo {\n"
+                                     "  FOO, BAR\n"
+                                     "};\n"
+                                     "\n"
+                                     "int foo() {\n"
+                                     "  int i, j;\n"
+                                     "}\n"
+                                     "\n"
+                                     "int i, j, k;\n"
+                                     "\n"
+                                     "int bar() {\n"
+                                     "  int j, k;\n"
+                                     "}\n"
+                                     "\n"
+                                     "enum Bar {\n"
+                                     "  FOOBAR, BARFOO\n"
+                                     "};",
+                                     Style));
+}
+
+TEST_F(DefinitionBlockSeparatorTest, OpeningBracketOwnsLine) {
+  FormatStyle Style = getLLVMStyle();
+  Style.SeparateDefinitionBlocks = FormatStyle::SDS_Always;
+  EXPECT_EQ("enum Foo\n"
+            "{\n"
+            "  FOO, BAR\n"
+            "};\n"
+            "\n"
+            "int foo()\n"
+            "{\n"
+            "  int i, j;\n"
+            "}\n"
+            "\n"
+            "int i, j, k;\n"
+            "\n"
+            "int bar()\n"
+            "{\n"
+            "  int j, k;\n"
+            "}\n"
+            "\n"
+            "enum Bar\n"
+            "{\n"
+            "  FOOBAR, BARFOO\n"
+            "};",
+            separateDefinitionBlocks("enum Foo\n"
+                                     "{\n"
+                                     "  FOO, BAR\n"
+                                     "};\n"
+                                     "int foo()\n"
+                                     "{\n"
+                                     "  int i, j;\n"
+                                     "}\n"
+                                     "int i, j, k;\n"
+                                     "int bar()\n"
+                                     "{\n"
+                                     "  int j, k;\n"
+                                     "}\n"
+                                     "enum Bar\n"
+                                     "{\n"
+                                     "  FOOBAR, BARFOO\n"
+                                     "};", Style));
+}
+
+TEST_F(DefinitionBlockSeparatorTest, Leave) {
+  FormatStyle Style = getLLVMStyle();
+  Style.SeparateDefinitionBlocks = FormatStyle::SDS_Leave;
+  EXPECT_EQ("enum Foo\n"
+            "{\n"
+            "  FOO, BAR\n"
+            "};\n\n\n\n"
+            "int foo()\n"
+            "{\n"
+            "  int i, j;\n"
+            "}\n"
+            "int i, j, k;\n"
+            "int bar()\n"
+            "{\n"
+            "  int j, k;\n"
+            "}\n"
+            "enum Bar\n"
+            "{\n"
+            "  FOOBAR, BARFOO\n"
+            "};",
+            separateDefinitionBlocks("enum Foo\n"
+                                     "{\n"
+                                     "  FOO, BAR\n"
+                                     "};\n\n\n\n"
+                                     "int foo()\n"
+                                     "{\n"
+                                     "  int i, j;\n"
+                                     "}\n"
+                                     "int i, j, k;\n"
+                                     "int bar()\n"
+                                     "{\n"
+                                     "  int j, k;\n"
+                                     "}\n"
+                                     "enum Bar\n"
+                                     "{\n"
+                                     "  FOOBAR, BARFOO\n"
+                                     "};", Style));
+}
+} // end namespace
+} // end namespace format
+} // end namespace clang