[clang-format] Add a new clang-format option AlwaysBreakBeforeFunctionParameters

[jrmolin]

Summary

Add the definition, documentation, and implementation for a new clang-format option.

This is for new issue https://github.com/llvm/llvm-project/issues/62092

[jrmolin]

Updated the version string in the docs and pulled in main.

[jrmolin]

I finally found the correct CodeOwners.rst file.

[jrmolin]

Finally figured out how to run the latest git-clang-format on the code

[owenpan]

Please see https://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options.

[owenpan]

See also https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface.

[owenpan]

Please add unit tests to relevant files in clang/unittests/Format/.

[HazardyKnusperkeks]

Please reupload with the complete context.

Please add tests.

clang/include/clang/Format/Format.h

827

That's not a valid declaration (missing return type), so not a good example.

831

Please sort alphabetically. (So above AlwaysBreakBeforeMultilineStrings.)

831

You are missing the \since 17.

clang/lib/Format/Format.cpp

1510

Only set it in getLLVMStyle, which you are missing right now. Every other style will inherit it.

clang/lib/Format/TokenAnnotator.cpp

4740–4742

Merge the ifs.

[HazardyKnusperkeks]

And please add an entry to the ReleaseNotes.rst.

[jrmolin]

In D125171#4167866, @owenpan wrote:

Please see https://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options.

I am doing this for my team, which writes the security endpoint for Elastic Defend. The code is currently private, though the binaries are free to download and run. The number of contributors is around 30, and the lines of code is in the 100Ks (around 500K). I have not found a way to accomplish what this does with the available options. I am adding tests and am happy to maintain this. It is a rather small addition, so it is quite simple to keep updated.

[jrmolin]

clang/include/clang/Format/Format.h

827

fixed. creating a new patch with all the requested context.

831

\version 17? I added this locally and am generating a new patch, with all the requested context.

831

sorted. creating a patch with requested context

clang/lib/Format/TokenAnnotator.cpp

4740–4742

I'm not sure I collapsed what you wanted. I am generating a new patch with all the context.

[jrmolin]

respond to code review requests/comments

[MyDeveloperDay]

clang/docs/ClangFormatStyleOptions.rst

1372

Regererate

1376

Ok you wrote this by hand it’s auto generated from format.h

clang/lib/Format/Format.cpp

889

Needs a parse test

clang/unittests/Format/FormatTest.cpp

25440

Please write it out long form

[owenpan]

In D125171#4193996, @jrmolin wrote:

In D125171#4167866, @owenpan wrote:

Please see https://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options.

I am doing this for my team, which writes the security endpoint for Elastic Defend. The code is currently private, though the binaries are free to download and run. The number of contributors is around 30, and the lines of code is in the 100Ks (around 500K). I have not found a way to accomplish what this does with the available options. I am adding tests and am happy to maintain this. It is a rather small addition, so it is quite simple to keep updated.

Do you "have a publicly accessible style guide"?

[jrmolin]

In D125171#4195298, @owenpan wrote:

In D125171#4193996, @jrmolin wrote:

In D125171#4167866, @owenpan wrote:

Please see https://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options.

I am doing this for my team, which writes the security endpoint for Elastic Defend. The code is currently private, though the binaries are free to download and run. The number of contributors is around 30, and the lines of code is in the 100Ks (around 500K). I have not found a way to accomplish what this does with the available options. I am adding tests and am happy to maintain this. It is a rather small addition, so it is quite simple to keep updated.

Do you "have a publicly accessible style guide"?

Unfortunately, we don't. I can create one in the likeness of the others, but I'm not sure where it will get published. I will talk to my managers about publishing a style doc.

clang/docs/ClangFormatStyleOptions.rst

1372

How do I re-generate? I'm happy to! I just don't see where to do that.

clang/lib/Format/Format.cpp

889

I will add one. Thanks for the reminder!

clang/unittests/Format/FormatTest.cpp

25440

What do you mean by "long form"?

[MyDeveloperDay]

clang/docs/ClangFormatStyleOptions.rst

1372

docs/tools/dump_format_style.py but ensure you move your example to Format.h first or its going to wipe out these changes

[jrmolin]

I updated the code comment in Format.h and ran the docs generator. I didn't modify the Macros section, but that got updated when I ran the docs generator.

We don't have a published style guide, unfortunately. I can work towards that, but I don't know how long that would take. My team doesn't like to agree on formatting changes. Hence this patch. We rolled this into the 3.9.0 release, and have been stuck with that ever since.

[jrmolin]

I still don't know what Please write it out long form means for the unit test.

clang/docs/ClangFormatStyleOptions.rst

3725

I didn't modify this section in the header at all. I assume someone modified the Format.h file and didn't update the docs.

[MyDeveloperDay]

clang/docs/ClangFormatStyleOptions.rst

3725

that needs to be looked at, as to what changed that meant it changed and done separately.

3761

This won't generate without a /version, we need to get this fixed outside of this commit

clang/include/clang/Format/Format.h

823

16.0

[MyDeveloperDay]

if you go ahead an rebase your should get rG7a5b95732ade: [clang-format] NFC Format.h and ClangFormatStyleOptions.rst are out of date that will remove the need for the Macro section in the rst

[MyDeveloperDay]

Please rebase

[jrmolin]

In D125171#4215910, @MyDeveloperDay wrote:

if you go ahead an rebase your should get rG7a5b95732ade: [clang-format] NFC Format.h and ClangFormatStyleOptions.rst are out of date that will remove the need for the Macro section in the rst

So pull in that git hash and re-diff? I couldn't get arcanist installed cleanly (php issues), so I've been using git diff -U999999... to generate the patches. I just keep pulling down main before creating a patch.

[HazardyKnusperkeks]

clang/lib/Format/TokenAnnotator.cpp

4845–4851

That I meant with merge.

[jrmolin]

pulled main, re-generated the docs, then re-generated the diff. Also added a parse test and collapsed some nested if conditions.

[jrmolin]

clang/lib/Format/TokenAnnotator.cpp

4845–4851

Ah, understood. I generally don't collapse ifs, so I didn't know how much you wanted. It seems like ... all of it.

[HazardyKnusperkeks]

Looks good to me, not giving my formal approval since the ongoing discussion about the style guide.

clang/include/clang/Format/Format.h

823

Since when do we add the .0?

clang/lib/Format/TokenAnnotator.cpp

4845–4851

I think now you need to format it.

[jrmolin]

ran the formatter, ran the documentation generator.

[jrmolin]

I think I have hit all the requests now. We're in the middle of building release candidates and testing, so management is taking longer and longer to get back to me about a style guide for my team.

We added it, because it forces a consistent look across all function declarations. I don't know what the next steps are now. I'm stuck; you're stuck. :shrug:

clang/lib/Format/TokenAnnotator.cpp

4845–4851

I ran the formatting

[jrmolin]

In D125171#4230397, @jrmolin wrote:

I think I have hit all the requests now. We're in the middle of building release candidates and testing, so management is taking longer and longer to get back to me about a style guide for my team.

We added it, because it forces a consistent look across all function declarations. I don't know what the next steps are now. I'm stuck; you're stuck. :shrug:

I started a style guide at https://github.com/elastic/endpoint/blob/add_style_guide/style_guide.md, but it hasn't been approved or merged to main. It's at least a start...

[MyDeveloperDay]

I don't like using additional variables in unit tests, if someone changes 25400 multiple tests could break, each test should be stand alone in my view

clang/unittests/Format/FormatTest.cpp

25446–25451

This comment was removed by MyDeveloperDay.

[MyDeveloperDay]

Can you show your example from a implementation case and not just a declaration i.e. what happens with

int function1(int param1) {}

clang/lib/Format/TokenAnnotator.cpp

4846

can you test the () case in your tests please with AlwaysBreakBeforeFunctionParameters set to true

[MyDeveloperDay]

clang/docs/ClangFormatStyleOptions.rst

1374

here you say declaration but don't we want the same in definition too? otherewise should it be AlwaysBreakBeforeFunctionDefinitionParameters if you are expecting something else?

1380

This isn't relevant is it? AlwaysBreakAfterReturnType

[jrmolin]

• updated the tests to use long-form
• added new tests to verify the no-parameter cases
• updated the documentation to minimize formatting variables
• ran the style guide generator

[jrmolin]

clang/docs/ClangFormatStyleOptions.rst

1380

No, it's not relevant, but I saw it in other examples, and I like the way it looks. I can change the example to be more minimal.

clang/lib/Format/TokenAnnotator.cpp

4846

updated the tests.

clang/unittests/Format/FormatTest.cpp

25446–25451

Oh, I see. Sure!

[jrmolin]

• updated the documentation to show the option works for function declaration and definition

[jrmolin]

fix a comment in the documentation

[MyDeveloperDay]

is it possible to point to a github issue that this related to in the review summary, if not please can you make one so we can track the issue its trying to solve

[jrmolin]

In D125171#4261301, @MyDeveloperDay wrote:

is it possible to point to a github issue that this related to in the review summary, if not please can you make one so we can track the issue its trying to solve

added an issue: https://github.com/llvm/llvm-project/issues/62092

[MyDeveloperDay]

clang/lib/Format/Format.cpp

890

This should be Alphabetic should this be before the MultlineStrings option?

1350

ok, we have this think that the lifetime of an option goes from bool -> enum -> struct, sometimes we pick up early that true/false aren't good enough

so here is the think... AlwaysBreakBeforeFunctionParameters should this be an enum and BreakBeforeFunctionParameters but with values

Leave, Never, Always

i.e. does AlwaysBreakBeforeFunctionParameters = false mean never break? or sometimes break.

We don't really want "false" to mean do something..we want it to mean don't do anything i.e. Leave

clang/unittests/Format/FormatTest.cpp

25437

I would say all these function could go to the single format of verifyFormat

25469

you can just use the single form of verifyFormat() it will call messUp to remove the whitespace

[jrmolin]

changing the added option from a boolean to an enum that takes Leave, Always, and Never.

[jrmolin]

clang/lib/Format/Format.cpp

1350

Please let me know if I did this wrong! This took more than I was expecting. I added parsing for "false" and "true" to be "Leave" and "Always". That is kind of confusing, and there is no need for "backwards compatibility" in this, but it looked easy. Should I remove that?

clang/unittests/Format/FormatTest.cpp

25437

I consolidated the tests and expanded.

[jrmolin]

there were a couple of bugs in the last patch that I missed. format tests all pass again with this one.

I am using "Leave" as a transparent value, so no line breaks are added and none are removed -- the penalty calculus drives. Just like it was before this patch. It's the best I can do for making users not have to care about this option.

[jrmolin]

a unit test was failing, so I fixed it.

clang/include/clang/Format/Format.h

851

so should this be 17.0 now? This has gone on quite long.

clang/lib/Format/Format.cpp

345

I should probably delete these, right?

[MyDeveloperDay]

clang/include/clang/Format/Format.h

816

Can you format this as we do the other documentation?

[jrmolin]

change formatting of enum options

[jrmolin]

pulled upstream/main

[MyDeveloperDay]

clang/lib/Format/Format.cpp

347

yes remove these... this is why you need to mark the comments done

[jrmolin]

more code review updates. pulled main.

[H-G-Hristov]

clang/unittests/Format/FormatTest.cpp

25490

Does this work with BAS_BlockIndent? I don't see any related tests.

[jrmolin]

clang/unittests/Format/FormatTest.cpp

25490

How do you want me to test it? I'm happy to, if that means this will get merged at some point.

[owenpan]

@jrmolin is this option really for breaking before the first parameter? Or are you trying to have one parameter per line as shown by the examples in your GitHub issue? Also, how would it interact with BAS_AlwaysBreak, BinPackParameters, and AllowAllParametersOfDeclarationOnNextLine? What if there is only one parameter? And do we really want something like the following for example?

template <typename T>
void baz(
    T t) requires C<T>

clang/include/clang/Format/Format.h

812–814

816

Can you add an example to show that the default penalty would break before the first parameter?

837

clang/lib/Format/ContinuationIndenter.cpp

357

Unrelated?

584

586

What would happen if there‘s a comment between the l_paren and the parameter?

1073–1076

Please reflow the comments.

clang/lib/Format/TokenAnnotator.cpp

4843–4845

Reflow comments.

4846

Again, we need to handle the case where a comment exists between the l_paren and the parameter.

4848–4856

Can you refactor the switch statement as it's also used in ContinuationIndenter.cpp above?

clang/docs/ClangFormatStyleOptions.rst

.. code-block:: c++
       int
       f() {
         return 1;
       }
 
 
 
 .. _AlwaysBreakBeforeMultilineStrings:
 
 **AlwaysBreakBeforeMultilineStrings** (``Boolean``) :versionbadge:`clang-format 3.4` :ref:`¶ <AlwaysBreakBeforeMultilineStrings>`
   If ``true``, always break before multiline string literals.
 
   This flag is mean to make cases where there are multiple multiline strings
   in a file look more consistent. Thus, it will only take effect if wrapping
   the string at that point leads to it being indented
   ``ContinuationIndentWidth`` spaces from the start of the line.
 
   .. code-block:: c++
 
      true:                                  false:
      aaaa =                         vs.     aaaa = "bbbb"
          "bbbb"                                    "cccc";
          "cccc";
 
 .. _AlwaysBreakTemplateDeclarations:
* ``BBCDS_Always`` (in configuration: ``Always``)
 
     .. code-block:: c++
 
       template <typename T>
       concept C = ...;
 
 
 
+.. _BreakBeforeFunctionParameters:
+
+**BreakBeforeFunctionParameters** (``FunctionParameterBreakingStyle``) :versionbadge:`clang-format 16.0` :ref:`¶ <BreakBeforeFunctionParameters>`
+  The function parameter breaking style to use.
+
+  Possible values:
+
+  * ``FPBS_Leave`` (in configuration: ``Leave``)
+    Be transparent with line breaks before function parameters. This allows the
+    penalty calculations to drive line breaks.
+
+    .. code-block:: c++
+
+       original:
+       int someFunction(int arg1, int arg2);
+
+       formatted:
+       int someFunction(int arg1, int arg2);
+
+  * ``FPBS_Never`` (in configuration: ``Never``)
+    Never break before the first parameter. This removes breaks that are there.
+
+    .. code-block:: c++
+
+       original:
+       int someFunction(
+           int arg1,
+           int arg2);
+
+       formatted:
+       int someFunction(int arg1, int arg2);
+
+  * ``FPBS_Always`` (in configuration: ``Always``)
+    Always break before the first parameter.
+
+    .. code-block:: c++
+
+       original:
+       int someFunction(int arg1, int arg2);
+
+       formatted:
+       int someFunction(
+           int arg1,
+           int arg2);
+
+
+
 .. _BreakBeforeInlineASMColon:
 
 **BreakBeforeInlineASMColon** (``BreakBeforeInlineASMColonStyle``) :versionbadge:`clang-format 16` :ref:`¶ <BreakBeforeInlineASMColon>`
   The inline ASM colon style to use.
 
   Possible values:
 
   * ``BBIAS_Never`` (in configuration: ``Never``)
.. code-block:: c++
      bar();
      NS_TABLE_FOO_END
 
 .. _MacroBlockEnd:
 
 **MacroBlockEnd** (``String``) :versionbadge:`clang-format 3.7` :ref:`¶ <MacroBlockEnd>`
   A regular expression matching macros that end a block.
 
 .. _Macros:
 
 **Macros** (``List of Strings``) :versionbadge:`clang-format 17.0` :ref:`¶ <Macros>`
   A list of macros of the form ``<definition>=<expansion>`` .
 
   Code will be parsed with macros expanded, in order to determine how to
   interpret and format the macro arguments.
 
   For example, the code:
**Macros** (``List of Strings``) :versionbadge:`clang-format 17.0` :ref:`¶ <Macros>`
    * Both function-like macros and object-like macros are supported.
    * Macro arguments must be used exactly once in the expansion.
    * No recursive expansion; macros referencing other macros will be
      ignored.
    * Overloading by arity is supported: for example, given the macro
      definitions A=x, A()=y, A(a)=a
 
 
   .. code-block:: c++
 
      A; -> x;
      A(); -> y;
      A(z); -> z;
      A(a, b); // will not be expanded.
 
 .. _MaxEmptyLinesToKeep:
 

clang/include/clang/Format/Format.h

struct FormatStyle {
   /// option is **deprecated** and is retained for backwards compatibility.
   /// \version 3.7
   DefinitionReturnTypeBreakingStyle AlwaysBreakAfterDefinitionReturnType;
 
   /// The function declaration return type breaking style to use.
   /// \version 3.8
   ReturnTypeBreakingStyle AlwaysBreakAfterReturnType;
 
+  /// Different ways to break before the function parameters.
+  enum FunctionParameterBreakingStyle : int8_t {
+    /// Be transparent with line breaks before function parameters. This allows
+    /// the
+    /// penalty calculations to drive line breaks.
+    /// \code
+    ///    original:
+    ///    int someFunction(int arg1, int arg2);
+    ///
+    ///    formatted:
+    ///    int someFunction(int arg1, int arg2);
+    /// \endcode
+    FPBS_Leave,
+
+    /// Never break before the first parameter. This removes breaks that are
+    /// there.
+    /// \code
+    ///    original:
+    ///    int someFunction(
+    ///        int arg1,
+    ///        int arg2);
+    ///
+    ///    formatted:
+    ///    int someFunction(int arg1, int arg2);
+    /// \endcode
+    FPBS_Never,
+
+    /// Always break before the first parameter.
+    /// \code
+    ///    original:
+    ///    int someFunction(int arg1, int arg2);
+    ///
+    ///    formatted:
+    ///    int someFunction(
+    ///        int arg1,
+    ///        int arg2);
+    /// \endcode
+    FPBS_Always,
+  };
+
+  /// The function parameter breaking style to use.
+  /// \version 16.0
+  FunctionParameterBreakingStyle BreakBeforeFunctionParameters;
+
   /// If ``true``, always break before multiline string literals.
   ///
   /// This flag is mean to make cases where there are multiple multiline strings
   /// in a file look more consistent. Thus, it will only take effect if wrapping
   /// the string at that point leads to it being indented
   /// ``ContinuationIndentWidth`` spaces from the start of the line.
   /// \code
   ///    true:                                  false:

clang/lib/Format/ContinuationIndenter.cpp

bool ContinuationIndenter::mustBreak(const LineState &State) {
   const FormatToken &Current = *State.NextToken;
   const FormatToken &Previous = *Current.Previous;
   const auto &CurrentState = State.Stack.back();
   if (Style.BraceWrapping.BeforeLambdaBody && Current.CanBreakBefore &&
       Current.is(TT_LambdaLBrace) && Previous.isNot(TT_LineComment)) {
     auto LambdaBodyLength = getLengthToMatchingParen(Current, State.Stack);
     return LambdaBodyLength > getColumnLimit(State);
   }
+
   if (Current.MustBreakBefore ||
       (Current.is(TT_InlineASMColon) &&
        (Style.BreakBeforeInlineASMColon == FormatStyle::BBIAS_Always ||
         Style.BreakBeforeInlineASMColon == FormatStyle::BBIAS_OnlyMultiline))) {
     return true;
   }
   if (CurrentState.BreakBeforeClosingBrace &&
       Current.closesBlockOrBlockTypeList(Style)) {
if (((Current.is(TT_FunctionDeclarationName) &&
         // Don't always break between a JavaScript `function` and the function
         // name.
         !Style.isJavaScript()) ||
        (Current.is(tok::kw_operator) && !Previous.is(tok::coloncolon))) &&
       !Previous.is(tok::kw_template) && CurrentState.BreakBeforeParameter) {
     return true;
   }
 
+  // Check if we want to break before the first function parameter in
+  // declarations and definitions
+  if (startsNextParameter(Current, Style) && State.Line->MustBeDeclaration &&
+      Current.Previous->is(tok::l_paren)) {
+    switch (Style.BreakBeforeFunctionParameters) {
+    case FormatStyle::FPBS_Always:
+      return true;
+    case FormatStyle::FPBS_Never:
+      return false;
+    case FormatStyle::FPBS_Leave:
+      break;
+    }
+  }
+
   // The following could be precomputed as they do not depend on the state.
   // However, as they should take effect only if the UnwrappedLine does not fit
   // into the ColumnLimit, they are checked here in the ContinuationIndenter.
   if (Style.ColumnLimit != 0 && Previous.is(BK_Block) &&
       Previous.is(tok::l_brace) &&
       !Current.isOneOf(tok::r_brace, tok::comment)) {
     return true;
   }
if (PreviousNonComment && PreviousNonComment->is(tok::l_paren)) {
     CurrentState.BreakBeforeClosingParen =
         Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent;
   }
 
   if (CurrentState.AvoidBinPacking) {
     // If we are breaking after '(', '{', '<', or this is the break after a ':'
     // to start a member initializater list in a constructor, this should not
     // be considered bin packing unless the relevant AllowAll option is false or
-    // this is a dict/object literal.
+    // this is a dict/object literal. Break if
+    // BreakBeforeFunctionParameters is Always and it's a function
+    // declaration or if it's Leave and a newline exists already.
     bool PreviousIsBreakingCtorInitializerColon =
         PreviousNonComment && PreviousNonComment->is(TT_CtorInitializerColon) &&
         Style.BreakConstructorInitializers == FormatStyle::BCIS_AfterColon;
     bool AllowAllConstructorInitializersOnNextLine =
         Style.PackConstructorInitializers == FormatStyle::PCIS_NextLine ||
         Style.PackConstructorInitializers == FormatStyle::PCIS_NextLineOnly;
+    bool BreakBeforeFunctionParameter =
+        Style.BreakBeforeFunctionParameters == FormatStyle::FPBS_Always;
     if (!(Previous.isOneOf(tok::l_paren, tok::l_brace, TT_BinaryOperator) ||
           PreviousIsBreakingCtorInitializerColon) ||
         (!Style.AllowAllParametersOfDeclarationOnNextLine &&
          State.Line->MustBeDeclaration) ||
         (!Style.AllowAllArgumentsOnNextLine &&
          !State.Line->MustBeDeclaration) ||
         (!AllowAllConstructorInitializersOnNextLine &&
          PreviousIsBreakingCtorInitializerColon) ||
-        Previous.is(TT_DictLiteral)) {
+        Previous.is(TT_DictLiteral) ||
+        (BreakBeforeFunctionParameter && State.Line->MustBeDeclaration)) {
       CurrentState.BreakBeforeParameter = true;
     }
 
     // If we are breaking after a ':' to start a member initializer list,
     // and we allow all arguments on the next line, we should not break
     // before the next parameter.
     if (PreviousIsBreakingCtorInitializerColon &&
         AllowAllConstructorInitializersOnNextLine) {

clang/lib/Format/Format.cpp

enumeration(IO &IO, FormatStyle::EmptyLineBeforeAccessModifierStyle &Value) {
     IO.enumCase(Value, "Never", FormatStyle::ELBAMS_Never);
     IO.enumCase(Value, "Leave", FormatStyle::ELBAMS_Leave);
     IO.enumCase(Value, "LogicalBlock", FormatStyle::ELBAMS_LogicalBlock);
     IO.enumCase(Value, "Always", FormatStyle::ELBAMS_Always);
   }
 };
 
 template <>
+struct ScalarEnumerationTraits<FormatStyle::FunctionParameterBreakingStyle> {
+  static void enumeration(IO &IO,
+                          FormatStyle::FunctionParameterBreakingStyle &Value) {
+    IO.enumCase(Value, "Leave", FormatStyle::FPBS_Leave);
+    IO.enumCase(Value, "Never", FormatStyle::FPBS_Never);
+    IO.enumCase(Value, "Always", FormatStyle::FPBS_Always);
+
+    // For backward compatibility.
+    IO.enumCase(Value, "false", FormatStyle::FPBS_Leave);
+    IO.enumCase(Value, "true", FormatStyle::FPBS_Always);
+  }
+};
+
+template <>
 struct ScalarEnumerationTraits<FormatStyle::IndentExternBlockStyle> {
   static void enumeration(IO &IO, FormatStyle::IndentExternBlockStyle &Value) {
     IO.enumCase(Value, "AfterExternBlock", FormatStyle::IEBS_AfterExternBlock);
     IO.enumCase(Value, "Indent", FormatStyle::IEBS_Indent);
     IO.enumCase(Value, "NoIndent", FormatStyle::IEBS_NoIndent);
     IO.enumCase(Value, "true", FormatStyle::IEBS_Indent);
     IO.enumCase(Value, "false", FormatStyle::IEBS_NoIndent);
   }
IO.mapOptional("AllowShortLambdasOnASingleLine",
                    Style.AllowShortLambdasOnASingleLine);
     IO.mapOptional("AllowShortLoopsOnASingleLine",
                    Style.AllowShortLoopsOnASingleLine);
     IO.mapOptional("AlwaysBreakAfterDefinitionReturnType",
                    Style.AlwaysBreakAfterDefinitionReturnType);
     IO.mapOptional("AlwaysBreakAfterReturnType",
                    Style.AlwaysBreakAfterReturnType);
     IO.mapOptional("AlwaysBreakBeforeMultilineStrings",
                    Style.AlwaysBreakBeforeMultilineStrings);
     IO.mapOptional("AlwaysBreakTemplateDeclarations",
                    Style.AlwaysBreakTemplateDeclarations);
     IO.mapOptional("AttributeMacros", Style.AttributeMacros);
     IO.mapOptional("BinPackArguments", Style.BinPackArguments);
     IO.mapOptional("BinPackParameters", Style.BinPackParameters);
     IO.mapOptional("BitFieldColonSpacing", Style.BitFieldColonSpacing);
     IO.mapOptional("BraceWrapping", Style.BraceWrapping);
     IO.mapOptional("BreakAfterAttributes", Style.BreakAfterAttributes);
     IO.mapOptional("BreakAfterJavaFieldAnnotations",
                    Style.BreakAfterJavaFieldAnnotations);
     IO.mapOptional("BreakArrays", Style.BreakArrays);
     IO.mapOptional("BreakBeforeBinaryOperators",
                    Style.BreakBeforeBinaryOperators);
     IO.mapOptional("BreakBeforeConceptDeclarations",
                    Style.BreakBeforeConceptDeclarations);
     IO.mapOptional("BreakBeforeBraces", Style.BreakBeforeBraces);
+    IO.mapOptional("BreakBeforeFunctionParameters",
+                   Style.BreakBeforeFunctionParameters);
     IO.mapOptional("BreakBeforeInlineASMColon",
                    Style.BreakBeforeInlineASMColon);
     IO.mapOptional("BreakBeforeTernaryOperators",
                    Style.BreakBeforeTernaryOperators);
     IO.mapOptional("BreakConstructorInitializers",
                    Style.BreakConstructorInitializers);
     IO.mapOptional("BreakInheritanceList", Style.BreakInheritanceList);
     IO.mapOptional("BreakStringLiterals", Style.BreakStringLiterals);
FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
   LLVMStyle.AllowShortCaseLabelsOnASingleLine = false;
   LLVMStyle.AllowShortEnumsOnASingleLine = true;
   LLVMStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
   LLVMStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
   LLVMStyle.AllowShortLambdasOnASingleLine = FormatStyle::SLS_All;
   LLVMStyle.AllowShortLoopsOnASingleLine = false;
   LLVMStyle.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None;
   LLVMStyle.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None;
   LLVMStyle.AlwaysBreakBeforeMultilineStrings = false;
   LLVMStyle.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_MultiLine;
   LLVMStyle.AttributeMacros.push_back("__capability");
   LLVMStyle.BitFieldColonSpacing = FormatStyle::BFCS_Both;
   LLVMStyle.BinPackArguments = true;
   LLVMStyle.BinPackParameters = true;
   LLVMStyle.BraceWrapping = {/*AfterCaseLabel=*/false,
                              /*AfterClass=*/false,
                              /*AfterControlStatement=*/FormatStyle::BWACS_Never,
LLVMStyle.BraceWrapping = {/*AfterCaseLabel=*/false,
                              /*SplitEmptyRecord=*/true,
                              /*SplitEmptyNamespace=*/true};
   LLVMStyle.BreakAfterAttributes = FormatStyle::ABS_Never;
   LLVMStyle.BreakAfterJavaFieldAnnotations = false;
   LLVMStyle.BreakArrays = true;
   LLVMStyle.BreakBeforeBinaryOperators = FormatStyle::BOS_None;
   LLVMStyle.BreakBeforeBraces = FormatStyle::BS_Attach;
   LLVMStyle.BreakBeforeConceptDeclarations = FormatStyle::BBCDS_Always;
+  LLVMStyle.BreakBeforeFunctionParameters = FormatStyle::FPBS_Leave;
   LLVMStyle.BreakBeforeInlineASMColon = FormatStyle::BBIAS_OnlyMultiline;
   LLVMStyle.BreakBeforeTernaryOperators = true;
   LLVMStyle.BreakConstructorInitializers = FormatStyle::BCIS_BeforeColon;
   LLVMStyle.BreakInheritanceList = FormatStyle::BILS_BeforeColon;
   LLVMStyle.BreakStringLiterals = true;
   LLVMStyle.ColumnLimit = 80;
   LLVMStyle.CommentPragmas = "^ IWYU pragma:";
   LLVMStyle.CompactNamespaces = false;
FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
   // Defaults that differ when not C++.
   switch (Language) {
   case FormatStyle::LK_TableGen:
     LLVMStyle.SpacesInContainerLiterals = false;
     break;
   case FormatStyle::LK_Json:
     LLVMStyle.ColumnLimit = 0;
     break;
   case FormatStyle::LK_Verilog:
     LLVMStyle.IndentCaseLabels = true;
     LLVMStyle.SpacesInContainerLiterals = false;
     break;
   default:
     break;
   }
 
   return LLVMStyle;

clang/lib/Format/TokenAnnotator.cpp

if (Left.is(TT_DictLiteral) && Left.is(tok::l_brace) && Line.Level == 0 &&
         Left.Previous && Left.Previous->is(tok::equal) &&
         Line.First->isOneOf(tok::identifier, Keywords.kw_import, tok::kw_export,
                             tok::kw_const) &&
         // kw_var/kw_let are pseudo-tokens that are tok::identifier, so match
         // above.
         !Line.First->isOneOf(Keywords.kw_var, Keywords.kw_let)) {
       // Object literals on the top level of a file are treated as "enum-style".
       // Each key/value pair is put on a separate line, instead of bin-packing.
       return true;
     }
     if (Left.is(tok::l_brace) && Line.Level == 0 &&
         (Line.startsWith(tok::kw_enum) ||
          Line.startsWith(tok::kw_const, tok::kw_enum) ||
          Line.startsWith(tok::kw_export, tok::kw_enum) ||
          Line.startsWith(tok::kw_export, tok::kw_const, tok::kw_enum))) {
       // JavaScript top-level enum key/value pairs are put on separate lines
       // instead of bin-packing.
       return true;
     }
if (Style.isJson()) {
     }
   }
 
   if (Line.startsWith(tok::kw_asm) && Right.is(TT_InlineASMColon) &&
       Style.BreakBeforeInlineASMColon == FormatStyle::BBIAS_Always) {
     return true;
   }
 
+  // If BreakBeforeFunctionParameters is Always, we want to break before
+  // the next parameter, if there is one. If it is Leave and a newline exists,
+  // make sure we insert one. Otherwise, no newline.
+  if (Left.is(tok::l_paren) && !Right.is(tok::r_paren) && Left.Previous &&
+      Left.Previous->is(TT_FunctionDeclarationName)) {
+    switch (Style.BreakBeforeFunctionParameters) {
+    case FormatStyle::FPBS_Always:
+      return true;
+    case FormatStyle::FPBS_Never:
+      return false;
+    case FormatStyle::FPBS_Leave:
+      break;
+    }
+  }
+
   // If the last token before a '}', ']', or ')' is a comma or a trailing
   // comment, the intention is to insert a line break after it in order to make
   // shuffling around entries easier. Import statements, especially in
   // JavaScript, can be an exception to this rule.
   if (Style.JavaScriptWrapImports || Line.Type != LT_ImportStatement) {
     const FormatToken *BeforeClosingBrace = nullptr;
     if ((Left.isOneOf(tok::l_brace, TT_ArrayInitializerLSquare) ||
          (Style.isJavaScript() && Left.is(tok::l_paren))) &&

clang/unittests/Format/ConfigParseTest.cpp

#undef CHECK_ALIGN_CONSECUTIVE
   CHECK_PARSE("BreakBeforeBraces: Whitesmiths", BreakBeforeBraces,
               FormatStyle::BS_Whitesmiths);
   CHECK_PARSE("BreakBeforeBraces: GNU", BreakBeforeBraces, FormatStyle::BS_GNU);
   CHECK_PARSE("BreakBeforeBraces: WebKit", BreakBeforeBraces,
               FormatStyle::BS_WebKit);
   CHECK_PARSE("BreakBeforeBraces: Custom", BreakBeforeBraces,
               FormatStyle::BS_Custom);
 
+  Style.BreakBeforeFunctionParameters = FormatStyle::FPBS_Never;
+  CHECK_PARSE("BreakBeforeFunctionParameters: Leave",
+              BreakBeforeFunctionParameters, FormatStyle::FPBS_Leave);
+  CHECK_PARSE("BreakBeforeFunctionParameters: Always",
+              BreakBeforeFunctionParameters, FormatStyle::FPBS_Always);
+  CHECK_PARSE("BreakBeforeFunctionParameters: Never",
+              BreakBeforeFunctionParameters, FormatStyle::FPBS_Never);
+
   Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Never;
   CHECK_PARSE("BraceWrapping:\n"
               "  AfterControlStatement: MultiLine",
               BraceWrapping.AfterControlStatement,
               FormatStyle::BWACS_MultiLine);
   CHECK_PARSE("BraceWrapping:\n"
               "  AfterControlStatement: Always",
               BraceWrapping.AfterControlStatement, FormatStyle::BWACS_Always);

clang/unittests/Format/FormatTest.cpp

TEST_F(FormatTest, InsertNewlineAtEOF) {
   verifyFormat("int i;\n", "int i;", Style);
 }
 
 TEST_F(FormatTest, SpaceAfterUDL) {
   verifyFormat("auto c = (4s).count();");
   verifyFormat("auto x = 5s .count() == 5;");
 }
 
+TEST_F(FormatTest, BreakBeforeParameterList) {
+  FormatStyle Style = getLLVMStyle();
+  EXPECT_EQ(Style.BreakBeforeFunctionParameters, FormatStyle::FPBS_Leave);
+  Style.BinPackParameters = false;
+
+  // test Leave (basically transparent mode)
+
+  // verify that there is no break by default
+  verifyFormat("int function1();\n" // formatted
+               "int function2(int param1, int param2, int param3);\n"
+               "void function3(int param1, int param2, int param3) {}\n"
+               "int function4(int param1, int param2, int param3);\n",
+               "int function1();\n" // original
+               "int function2(int param1, int param2, int param3);\n"
+               "void function3(int param1, int param2, int param3) {}\n"
+               "int function4(int param1, int param2, int param3);\n",
+               Style);
+
+  // test Always
+  // verify that there is a break when told to break
+  Style.BreakBeforeFunctionParameters = FormatStyle::FPBS_Always;
+  verifyFormat("int function1(\n"
+               "    int param1,\n"
+               "    int param2,\n"
+               "    int param3);\n"
+               "int function2();\n"
+               "void function3(\n"
+               "    int param1,\n"
+               "    int param2,\n"
+               "    int param3) {}\n"
+               "int function4(\n"
+               "    int param1,\n"
+               "    int param2,\n"
+               "    int param3);\n"
+               "int function5(\n"
+               "    int param1,\n"
+               "    int param2,\n"
+               "    int param3);\n",
+               Style);
+
+  // verify that having no parameters doesn't affect the parentheses
+  verifyFormat("void function1() {}\n", // the formatted part
+               "void function1() {}\n", // the original
+               Style);
+
+  verifyFormat("void function1();\n", // the formatted part
+               "void function1();\n", // the original
+               Style);
+
+  // test Never
+  Style.BreakBeforeFunctionParameters = FormatStyle::FPBS_Never;
+  verifyFormat("int function1();\n" // the formatted part
+               "int function2(int param1, int param2, int param3);\n",
+               "int function1();\n" // the original
+               "int function2(\n"
+               "    int param1,\n"
+               "    int param2,\n"
+               "    int param3);\n",
+               Style);
+}
 } // namespace
 } // namespace test
 } // namespace format
 } // namespace clang