[clang-format] Always consider option PenaltyBreakBeforeFirstCallParameter

[mnauw]

Summary

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.

[MyDeveloperDay]

You need to supply full context diffs

clang/unittests/Format/FormatTest.cpp

4481

Why do you need these? what is the default value of this as set in the LLVM style?

[mnauw]

clang/unittests/Format/FormatTest.cpp

4481

The default value in the LLVM style is 19. However, the fragment of code in TokenAnnotator.cpp basically hard-codes this to 0 (in case of BAS_DontAlign; see that part). Since that part is now changed to really use PenaltyBreakBeforeFirstCallParameter style option, the latter is set to 0 in these cases to retain the original behavior.

[mnauw]

Looks like PenaltyBreakBeforeFirstCallParameter not being considered so well has been addressed somewhat differently in D90246, and so that does the job as well.

clang/lib/Format/Format.cpp

     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);

 
   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;

   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;

clang/lib/Format/TokenAnnotator.cpp

     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

clang/unittests/Format/FormatTest.cpp

   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"

                Style);
 
   Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+  Style.PenaltyBreakBeforeFirstCallParameter = 0;
   verifyFormat("return (a > b\n"
                "    // comment1\n"
                "    // comment2\n"

       "                                             aaaaaaaaaaaaaaaaaaaaa));");
   FormatStyle Style = getLLVMStyle();
   Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+  Style.PenaltyBreakBeforeFirstCallParameter = 0;
   verifyFormat("void aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n"
                "    aaaaaaaaaaa aaaaaaaa, aaaaaaaaa aaaaaaa) {}",
                Style);

                "          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);",

 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"

       "    y;",
       Style);
   Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+  Style.PenaltyBreakBeforeFirstCallParameter = 0;
   Style.BreakBeforeBinaryOperators = FormatStyle::BOS_All;
   verifyFormat(
       "template <typename... a> struct r {};\n"