jrmolin updated this revision to Diff 519293.
jrmolin added a comment.

more code review updates. pulled main.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125171/new/

https://reviews.llvm.org/D125171

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/ConfigParseTest.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -25655,6 +25655,68 @@
   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(\n"
+               "    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);
+}
+
 TEST_F(FormatTest, InterfaceAsClassMemberName) {
   verifyFormat("class Foo {\n"
                "  int interface;\n"
Index: clang/unittests/Format/ConfigParseTest.cpp
===================================================================
--- clang/unittests/Format/ConfigParseTest.cpp
+++ clang/unittests/Format/ConfigParseTest.cpp
@@ -609,6 +609,13 @@
   CHECK_PARSE("BreakBeforeBraces: Custom", BreakBeforeBraces,
               FormatStyle::BS_Custom);
 
+  CHECK_PARSE("BreakBeforeFunctionParameters: Always",
+              BreakBeforeFunctionParameters, FormatStyle::FPBS_Always);
+  CHECK_PARSE("BreakBeforeFunctionParameters: Leave",
+              BreakBeforeFunctionParameters, FormatStyle::FPBS_Leave);
+  CHECK_PARSE("BreakBeforeFunctionParameters: Never",
+              BreakBeforeFunctionParameters, FormatStyle::FPBS_Never);
+
   Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Never;
   CHECK_PARSE("BraceWrapping:\n"
               "  AfterControlStatement: MultiLine",
Index: clang/lib/Format/TokenAnnotator.cpp
===================================================================
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -4874,6 +4874,21 @@
     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
Index: clang/lib/Format/Format.cpp
===================================================================
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -334,6 +334,16 @@
   }
 };
 
+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);
+  }
+};
+
 template <>
 struct ScalarEnumerationTraits<FormatStyle::IndentExternBlockStyle> {
   static void enumeration(IO &IO, FormatStyle::IndentExternBlockStyle &Value) {
@@ -891,6 +901,8 @@
     IO.mapOptional("BreakBeforeConceptDeclarations",
                    Style.BreakBeforeConceptDeclarations);
     IO.mapOptional("BreakBeforeBraces", Style.BreakBeforeBraces);
+    IO.mapOptional("BreakBeforeFunctionParameters",
+                   Style.BreakBeforeFunctionParameters);
     IO.mapOptional("BreakBeforeInlineASMColon",
                    Style.BreakBeforeInlineASMColon);
     IO.mapOptional("BreakBeforeTernaryOperators",
@@ -1364,6 +1376,7 @@
   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;
Index: clang/lib/Format/ContinuationIndenter.cpp
===================================================================
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -354,6 +354,7 @@
     auto LambdaBodyLength = getLengthToMatchingParen(Current, State.Stack);
     return LambdaBodyLength > getColumnLimit(State);
   }
+
   if (Current.MustBreakBefore ||
       (Current.is(TT_InlineASMColon) &&
        (Style.BreakBeforeInlineASMColon == FormatStyle::BBIAS_Always ||
@@ -579,6 +580,20 @@
     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.
@@ -1055,13 +1070,17 @@
     // 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 &&
@@ -1070,7 +1089,8 @@
          !State.Line->MustBeDeclaration) ||
         (!AllowAllConstructorInitializersOnNextLine &&
          PreviousIsBreakingCtorInitializerColon) ||
-        Previous.is(TT_DictLiteral)) {
+        Previous.is(TT_DictLiteral) ||
+        (BreakBeforeFunctionParameter && State.Line->MustBeDeclaration)) {
       CurrentState.BreakBeforeParameter = true;
     }
 
Index: clang/include/clang/Format/Format.h
===================================================================
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -807,6 +807,36 @@
   /// \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
+    ///    int someFunction(int arg1, int arg2);
+    /// \endcode
+    FPBS_Leave,
+
+    /// Never break before the first parameter. This removes breaks that are
+    /// there.
+    /// \code
+    ///    int someFunction(int arg1, int arg2);
+    /// \endcode
+    FPBS_Never,
+
+    /// Always break before the first parameter.
+    /// \code
+    ///    int someFunction(
+    ///        int arg1,
+    ///        int arg2);
+    /// \endcode
+    FPBS_Always,
+  };
+
+  /// The function parameter breaking style to use.
+  /// \version 17.0
+  FunctionParameterBreakingStyle BreakBeforeFunctionParameters;
+
   /// If ``true``, always break before multiline string literals.
   ///
   /// This flag is mean to make cases where there are multiple multiline strings
Index: clang/docs/ClangFormatStyleOptions.rst
===================================================================
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -2466,6 +2466,41 @@
 
 
 
+.. _BreakBeforeFunctionParameters:
+
+**BreakBeforeFunctionParameters** (``FunctionParameterBreakingStyle``) :versionbadge:`clang-format 17.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++
+
+       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++
+
+       int someFunction(int arg1, int arg2);
+
+  * ``FPBS_Always`` (in configuration: ``Always``)
+    Always break before the first parameter.
+
+    .. code-block:: c++
+
+       int someFunction(
+           int arg1,
+           int arg2);
+
+
+
 .. _BreakBeforeInlineASMColon:
 
 **BreakBeforeInlineASMColon** (``BreakBeforeInlineASMColonStyle``) :versionbadge:`clang-format 16` :ref:`¶ <BreakBeforeInlineASMColon>`
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D125171: [clang... MyDeveloperDay via Phabricator via cfe-commits
    • [PATCH] D125171: [... jonathan molinatto via Phabricator via cfe-commits

Reply via email to