jrmolin updated this revision to Diff 505215.
jrmolin marked 3 inline comments as done.
jrmolin added a comment.

respond to code review requests/comments


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/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -25346,6 +25346,27 @@
   verifyFormat("auto x = 5s .count() == 5;");
 }
 
+TEST_F(FormatTest, BreakBeforeParameterList) {
+  FormatStyle Style = getLLVMStyle();
+  EXPECT_EQ(Style.AlwaysBreakBeforeFunctionParameters, false);
+
+  const StringRef Code("int function1(int param1, int param2, int param3);\n"
+                       "int function2();\n");
+
+  // verify that there is no break by default
+  verifyFormat("int function1(int param1, int param2, int param3);\n"
+               "int function2();\n",
+               Code, Style);
+
+  // verify that there is a break when told to break
+  Style.AlwaysBreakBeforeFunctionParameters = true;
+  verifyFormat("int function1(\n"
+               "    int param1,\n"
+               "    int param2,\n"
+               "    int param3);\n"
+               "int function2();\n",
+               Code, Style);
+}
 } // namespace
 } // namespace test
 } // namespace format
Index: clang/lib/Format/TokenAnnotator.cpp
===================================================================
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -4737,6 +4737,16 @@
     return true;
   }
 
+  // If AlwaysBreakBeforeFunctionParameters is true, we want to break before
+  // the next parameter, if there is one.
+  if (Left.is(tok::l_paren) && Style.AlwaysBreakBeforeFunctionParameters &&
+      !Right.is(tok::r_paren) && Left.Previous) {
+      const FormatToken &TwoPrevious = *Left.Previous;
+      if (TwoPrevious.is(TT_FunctionDeclarationName)) {
+          return true;
+      }
+  }
+
   // 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
@@ -870,6 +870,8 @@
                    Style.AlwaysBreakAfterReturnType);
     IO.mapOptional("AlwaysBreakBeforeMultilineStrings",
                    Style.AlwaysBreakBeforeMultilineStrings);
+    IO.mapOptional("AlwaysBreakBeforeFunctionParameters",
+                   Style.AlwaysBreakBeforeFunctionParameters);
     IO.mapOptional("AlwaysBreakTemplateDeclarations",
                    Style.AlwaysBreakTemplateDeclarations);
     IO.mapOptional("AttributeMacros", Style.AttributeMacros);
@@ -1325,6 +1327,7 @@
   LLVMStyle.AllowShortLoopsOnASingleLine = false;
   LLVMStyle.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None;
   LLVMStyle.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None;
+  LLVMStyle.AlwaysBreakBeforeFunctionParameters = false;
   LLVMStyle.AlwaysBreakBeforeMultilineStrings = false;
   LLVMStyle.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_MultiLine;
   LLVMStyle.AttributeMacros.push_back("__capability");
Index: clang/lib/Format/ContinuationIndenter.cpp
===================================================================
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -354,6 +354,11 @@
     auto LambdaBodyLength = getLengthToMatchingParen(Current, State.Stack);
     return LambdaBodyLength > getColumnLimit(State);
   }
+  // Check if we want to break before function parameters in declarations
+  if (startsNextParameter(Current, Style) &&
+      Style.AlwaysBreakBeforeFunctionParameters &&
+      State.Line->MustBeDeclaration)
+    return true;
   if (Current.MustBreakBefore ||
       (Current.is(TT_InlineASMColon) &&
        (Style.BreakBeforeInlineASMColon == FormatStyle::BBIAS_Always ||
@@ -1055,7 +1060,9 @@
     // 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
+    // AlwaysBreakBeforeFunctionParameters is true and it's a function
+    // declaration.
     bool PreviousIsBreakingCtorInitializerColon =
         PreviousNonComment && PreviousNonComment->is(TT_CtorInitializerColon) &&
         Style.BreakConstructorInitializers == FormatStyle::BCIS_AfterColon;
@@ -1070,7 +1077,9 @@
          !State.Line->MustBeDeclaration) ||
         (!AllowAllConstructorInitializersOnNextLine &&
          PreviousIsBreakingCtorInitializerColon) ||
-        Previous.is(TT_DictLiteral)) {
+        Previous.is(TT_DictLiteral) ||
+        (Style.AlwaysBreakBeforeFunctionParameters &&
+         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,16 @@
   /// \version 3.8
   ReturnTypeBreakingStyle AlwaysBreakAfterReturnType;
 
+  /// \brief Always break before function parameters in a declaration, e.g.:
+  /// \code
+  ///   int
+  ///   someFunction(
+  ///       int argument1,
+  ///       int argument2);
+  /// \endcode
+  /// \version 17
+  bool AlwaysBreakBeforeFunctionParameters;
+
   /// 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
@@ -1368,6 +1368,24 @@
 
 
 
+.. _AlwaysBreakBeforeFunctionParameters:
+
+**AlwaysBreakBeforeFunctionParameters** (``Boolean``) :versionbadge:`clang-format 16.0`
+  If ``true``, always break before function parameters.
+
+  This flag is meant to align function parameters starting on the line following
+  a function declaration or definition. Thus, it will only take effect if a function
+  declares a parameter (or multiple parameters). Example uses ``AlwaysBreakAfterReturnType``
+  set to ``All``.
+
+  .. code-block:: c++
+
+     true:                                  false:
+     int                            vs.     int
+     add_two_numbers(                       add_two_numbers(int a,
+         int a,                                 int b);
+         int b);
+
 .. _AlwaysBreakBeforeMultilineStrings:
 
 **AlwaysBreakBeforeMultilineStrings** (``Boolean``) :versionbadge:`clang-format 3.4` :ref:`¶ <AlwaysBreakBeforeMultilineStrings>`
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to