[PATCH] D90533: [clang-format] Always consider option PenaltyBreakBeforeFirstCallParameter

[Mark Nauwelaerts via Phabricator via cfe-commits]

mnauw created this revision.
mnauw added a reviewer: sammccall.
mnauw added a project: clang-format.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
mnauw requested review of this revision.

If AlignAfterOpenBracket is set to BAS_DontAlign, then it turns out that 
PenaltyBreakBeforeFirstCallParameter is not considered (due to hard-coded 
shortcut, so to speak).

This patch attempts to preserve the current (specially crafted) behavior, while 
considering PenaltyBreakBeforeFirstCallParameter when really specified.  There 
may also be other ways to go about this to allow the user to actually set and 
use PenaltyBreakBeforeFirstCallParameter.  Failing all such options, it might 
otherwise be documented that PenaltyBreakBeforeFirstCallParameter has no effect 
in BAS_DontAlign.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90533

Files:
  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
@@ -4478,6 +4478,7 @@
   Style.TabWidth = 4;
   Style.UseTab = FormatStyle::UT_Always;
   Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+  Style.PenaltyBreakBeforeFirstCallParameter = 0;
   Style.AlignOperands = FormatStyle::OAS_DontAlign;
   EXPECT_EQ("return someVeryVeryLongConditionThatBarelyFitsOnALine\n"
             "\t&& (someOtherLongishConditionPart1\n"
@@ -4628,6 +4629,7 @@
                Style);
 
   Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+  Style.PenaltyBreakBeforeFirstCallParameter = 0;
   verifyFormat("return (a > b\n"
                "    // comment1\n"
                "    // comment2\n"
@@ -6028,6 +6030,7 @@
       "                                             aaaaaaaaaaaaaaaaaaaaa));");
   FormatStyle Style = getLLVMStyle();
   Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+  Style.PenaltyBreakBeforeFirstCallParameter = 0;
   verifyFormat("void aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n"
                "    aaaaaaaaaaa aaaaaaaa, aaaaaaaaa aaaaaaa) {}",
                Style);
@@ -6106,11 +6109,13 @@
                "          bbbbbbbbbbbbbbbbbbbbbb);",
                Style);
   Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+  Style.PenaltyBreakBeforeFirstCallParameter = 0;
   Style.AlignOperands = FormatStyle::OAS_Align;
   verifyFormat("int a = f(aaaaaaaaaaaaaaaaaaaaaa &&\n"
                "          bbbbbbbbbbbbbbbbbbbbbb);",
                Style);
   Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+  Style.PenaltyBreakBeforeFirstCallParameter = 0;
   Style.AlignOperands = FormatStyle::OAS_DontAlign;
   verifyFormat("int a = f(aaaaaaaaaaaaaaaaaaaaaa &&\n"
                "    bbbbbbbbbbbbbbbbbbbbbb);",
@@ -7460,6 +7465,7 @@
 TEST_F(FormatTest, WrapsTemplateParameters) {
   FormatStyle Style = getLLVMStyle();
   Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+  Style.PenaltyBreakBeforeFirstCallParameter = 0;
   Style.BreakBeforeBinaryOperators = FormatStyle::BOS_None;
   verifyFormat(
       "template <typename... a> struct q {};\n"
@@ -7468,6 +7474,7 @@
       "    y;",
       Style);
   Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+  Style.PenaltyBreakBeforeFirstCallParameter = 0;
   Style.BreakBeforeBinaryOperators = FormatStyle::BOS_All;
   verifyFormat(
       "template <typename... a> struct r {};\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===================================================================
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2740,7 +2740,7 @@
     return 100;
   if (Left.opensScope()) {
     if (Style.AlignAfterOpenBracket == FormatStyle::BAS_DontAlign)
-      return 0;
+      return Style.PenaltyBreakBeforeFirstCallParameter;
     if (Left.is(tok::l_brace) && !Style.Cpp11BracedListStyle)
       return 19;
     return Left.ParameterCount > 1 ? Style.PenaltyBreakBeforeFirstCallParameter
Index: clang/lib/Format/Format.cpp
===================================================================
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -579,6 +579,13 @@
     IO.mapOptional("ObjCSpaceBeforeProtocolList",
                    Style.ObjCSpaceBeforeProtocolList);
     IO.mapOptional("PenaltyBreakAssignment", Style.PenaltyBreakAssignment);
+    // If AlignAfterBracket is DontAlign,
+    // adjust default PenaltyBreakBeforeFirstCallParameter to favor Newline.
+    // Always accept any explicitly specified value though.
+    if (!IO.outputting() &&
+        Style.AlignAfterOpenBracket == FormatStyle::BAS_DontAlign) {
+      Style.PenaltyBreakBeforeFirstCallParameter = 0;
+    }
     IO.mapOptional("PenaltyBreakBeforeFirstCallParameter",
                    Style.PenaltyBreakBeforeFirstCallParameter);
     IO.mapOptional("PenaltyBreakComment", Style.PenaltyBreakComment);
@@ -1071,6 +1078,7 @@
 
   if (Language == FormatStyle::LK_Java) {
     GoogleStyle.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+    GoogleStyle.PenaltyBreakBeforeFirstCallParameter = 0;
     GoogleStyle.AlignOperands = FormatStyle::OAS_DontAlign;
     GoogleStyle.AlignTrailingComments = false;
     GoogleStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
@@ -1219,6 +1227,7 @@
   FormatStyle Style = getLLVMStyle();
   Style.AccessModifierOffset = -4;
   Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+  Style.PenaltyBreakBeforeFirstCallParameter = 0;
   Style.AlignOperands = FormatStyle::OAS_DontAlign;
   Style.AlignTrailingComments = false;
   Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Empty;

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits