llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: None (VolatileAcorn) <details> <summary>Changes</summary> Deprecated the BinPackParameters boolean option in favor or PackParameters enum option in order to provide a BreakAlways setting. Related issues that have requested this feature #<!-- -->51833 #<!-- -->23796 #<!-- -->53190 Partially solves - this issue requests is for both arguments and parameters --- Patch is 32.33 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/101882.diff 10 Files Affected: - (modified) clang/docs/ClangFormatStyleOptions.rst (+39-13) - (modified) clang/include/clang/Format/Format.h (+34-14) - (modified) clang/lib/Format/ContinuationIndenter.cpp (+5-21) - (modified) clang/lib/Format/Format.cpp (+34-4) - (modified) clang/lib/Format/FormatToken.h (+18) - (modified) clang/lib/Format/TokenAnnotator.cpp (+8) - (modified) clang/unittests/Format/ConfigParseTest.cpp (+15-1) - (modified) clang/unittests/Format/FormatTest.cpp (+133-20) - (modified) clang/unittests/Format/FormatTestComments.cpp (+2-2) - (modified) clang/unittests/Format/FormatTestObjC.cpp (+1-1) ``````````diff diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst index 6c2e6da594847..510b956968dc6 100644 --- a/clang/docs/ClangFormatStyleOptions.rst +++ b/clang/docs/ClangFormatStyleOptions.rst @@ -2068,19 +2068,7 @@ the configuration (without a prefix: ``Auto``). .. _BinPackParameters: **BinPackParameters** (``Boolean``) :versionbadge:`clang-format 3.7` :ref:`¶ <BinPackParameters>` - If ``false``, a function declaration's or function definition's - parameters will either all be on the same line or will have one line each. - - .. code-block:: c++ - - true: - void f(int aaaaaaaaaaaaaaaaaaaa, int aaaaaaaaaaaaaaaaaaaa, - int aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) {} - - false: - void f(int aaaaaaaaaaaaaaaaaaaa, - int aaaaaaaaaaaaaaaaaaaa, - int aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) {} + This option is **deprecated**. See ``PackParameters``. .. _BitFieldColonSpacing: @@ -4984,6 +4972,44 @@ the configuration (without a prefix: ``Auto``). +.. _PackParameters: + +**PackParameters** (``PackParametersStyle``) :versionbadge:`clang-format 20` :ref:`¶ <PackParameters>` + The pack parameters style to use. + + Possible values: + + * ``PPS_CurrentLine`` (in configuration: ``CurrentLine``) + Put all parameters on the current line if they fit. + Otherwise, put each one on its own line. + + .. code-block:: c++ + + void f(int a, int b, int c); + + void f(int a, + int b, + int ccccccccccccccccccccccccccccccccccccc); + + * ``PPS_BinPack`` (in configuration: ``BinPack``) + Bin-pack parameters. + + .. code-block:: c++ + + void f(int a, int bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb, + int ccccccccccccccccccccccccccccccccccccccccccc); + + * ``PPS_BreakAlways`` (in configuration: ``BreakAlways``) + Always put each parameter on its own line. + + .. code-block:: c++ + + void f(int a, + int b, + int c); + + + .. _PenaltyBreakAssignment: **PenaltyBreakAssignment** (``Unsigned``) :versionbadge:`clang-format 5` :ref:`¶ <PenaltyBreakAssignment>` diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h index c454ab2bc0ce2..2991ab18f980d 100644 --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -1192,20 +1192,9 @@ struct FormatStyle { /// \version 3.7 bool BinPackArguments; - /// If ``false``, a function declaration's or function definition's - /// parameters will either all be on the same line or will have one line each. - /// \code - /// true: - /// void f(int aaaaaaaaaaaaaaaaaaaa, int aaaaaaaaaaaaaaaaaaaa, - /// int aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) {} - /// - /// false: - /// void f(int aaaaaaaaaaaaaaaaaaaa, - /// int aaaaaaaaaaaaaaaaaaaa, - /// int aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) {} - /// \endcode + /// This option is **deprecated**. See ``PackParameters``. /// \version 3.7 - bool BinPackParameters; + // bool BinPackParameters; /// Styles for adding spacing around ``:`` in bitfield definitions. enum BitFieldColonSpacingStyle : int8_t { @@ -3537,6 +3526,37 @@ struct FormatStyle { /// \version 14 PackConstructorInitializersStyle PackConstructorInitializers; + /// Different way to try to fit all parameters on a line. + enum PackParametersStyle : int8_t { + /// Put all parameters on the current line if they fit. + /// Otherwise, put each one on its own line. + /// \code + /// void f(int a, int b, int c); + /// + /// void f(int a, + /// int b, + /// int ccccccccccccccccccccccccccccccccccccc); + /// \endcode + PPS_CurrentLine, + /// Bin-pack parameters. + /// \code + /// void f(int a, int bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb, + /// int ccccccccccccccccccccccccccccccccccccccccccc); + /// \endcode + PPS_BinPack, + /// Always put each parameter on its own line. + /// \code + /// void f(int a, + /// int b, + /// int c); + /// \endcode + PPS_BreakAlways, + }; + + /// The pack parameters style to use. + /// \version 20 + PackParametersStyle PackParameters; + /// The penalty for breaking around an assignment operator. /// \version 5 unsigned PenaltyBreakAssignment; @@ -5024,7 +5044,6 @@ struct FormatStyle { R.AlwaysBreakBeforeMultilineStrings && AttributeMacros == R.AttributeMacros && BinPackArguments == R.BinPackArguments && - BinPackParameters == R.BinPackParameters && BitFieldColonSpacing == R.BitFieldColonSpacing && BracedInitializerIndentWidth == R.BracedInitializerIndentWidth && BreakAdjacentStringLiterals == R.BreakAdjacentStringLiterals && @@ -5094,6 +5113,7 @@ struct FormatStyle { ObjCSpaceAfterProperty == R.ObjCSpaceAfterProperty && ObjCSpaceBeforeProtocolList == R.ObjCSpaceBeforeProtocolList && PackConstructorInitializers == R.PackConstructorInitializers && + PackParameters == R.PackParameters && PenaltyBreakAssignment == R.PenaltyBreakAssignment && PenaltyBreakBeforeFirstCallParameter == R.PenaltyBreakBeforeFirstCallParameter && diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp index df86a774ba0f4..8495c7a7314e2 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -128,24 +128,6 @@ static bool startsSegmentOfBuilderTypeCall(const FormatToken &Tok) { return Tok.isMemberAccess() && Tok.Previous && Tok.Previous->closesScope(); } -// Returns \c true if \c Current starts a new parameter. -static bool startsNextParameter(const FormatToken &Current, - const FormatStyle &Style) { - const FormatToken &Previous = *Current.Previous; - if (Current.is(TT_CtorInitializerComma) && - Style.BreakConstructorInitializers == FormatStyle::BCIS_BeforeComma) { - return true; - } - if (Style.Language == FormatStyle::LK_Proto && Current.is(TT_SelectorName)) - return true; - return Previous.is(tok::comma) && !Current.isTrailingComment() && - ((Previous.isNot(TT_CtorInitializerComma) || - Style.BreakConstructorInitializers != - FormatStyle::BCIS_BeforeComma) && - (Previous.isNot(TT_InheritanceComma) || - Style.BreakInheritanceList != FormatStyle::BILS_BeforeComma)); -} - static bool opensProtoMessageField(const FormatToken &LessTok, const FormatStyle &Style) { if (LessTok.isNot(tok::less)) @@ -411,7 +393,8 @@ bool ContinuationIndenter::mustBreak(const LineState &State) { // sets BreakBeforeParameter to avoid bin packing and this creates a // completely unnecessary line break after a template type that isn't // line-wrapped. - (Previous.NestingLevel == 1 || Style.BinPackParameters)) || + (Previous.NestingLevel == 1 || + (Style.PackParameters == FormatStyle::PPS_BinPack))) || (Style.BreakBeforeTernaryOperators && Current.is(TT_ConditionalExpr) && Previous.isNot(tok::question)) || (!Style.BreakBeforeTernaryOperators && @@ -1918,11 +1901,12 @@ void ContinuationIndenter::moveStatePastScopeOpener(LineState &State, // for backwards compatibility. bool ObjCBinPackProtocolList = (Style.ObjCBinPackProtocolList == FormatStyle::BPS_Auto && - Style.BinPackParameters) || + (Style.PackParameters == FormatStyle::PPS_BinPack)) || Style.ObjCBinPackProtocolList == FormatStyle::BPS_Always; bool BinPackDeclaration = - (State.Line->Type != LT_ObjCDecl && Style.BinPackParameters) || + (State.Line->Type != LT_ObjCDecl && + (Style.PackParameters == FormatStyle::PPS_BinPack)) || (State.Line->Type == LT_ObjCDecl && ObjCBinPackProtocolList); bool GenericSelection = diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index 7fd42e46e0ccb..96b9c50f3068e 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -464,6 +464,14 @@ struct ScalarEnumerationTraits<FormatStyle::PackConstructorInitializersStyle> { } }; +template <> struct ScalarEnumerationTraits<FormatStyle::PackParametersStyle> { + static void enumeration(IO &IO, FormatStyle::PackParametersStyle &Value) { + IO.enumCase(Value, "CurrentLine", FormatStyle::PPS_CurrentLine); + IO.enumCase(Value, "BinPack", FormatStyle::PPS_BinPack); + IO.enumCase(Value, "BreakAlways", FormatStyle::PPS_BreakAlways); + } +}; + template <> struct ScalarEnumerationTraits<FormatStyle::PointerAlignmentStyle> { static void enumeration(IO &IO, FormatStyle::PointerAlignmentStyle &Value) { IO.enumCase(Value, "Middle", FormatStyle::PAS_Middle); @@ -852,6 +860,15 @@ template <> struct MappingTraits<FormatStyle> { bool OnCurrentLine = IsGoogleOrChromium; bool OnNextLine = true; + // For backward compatibility: + // The default value of BinPackParameters was true unless BasedOnStyle was + // Mozilla or Chromium. If BinPackParameters was true then the equilvalent + // value for PackParameters is PPS_BinPack and PPS_CurrentLine otherwise. + const bool IsChromiumOrMozilla = + BasedOnStyle.equals_insensitive("chromium") || + BasedOnStyle.equals_insensitive("mozilla"); + bool BinPackParameters = !IsChromiumOrMozilla; + bool BreakBeforeInheritanceComma = false; bool BreakConstructorInitializersBeforeComma = false; @@ -870,6 +887,7 @@ template <> struct MappingTraits<FormatStyle> { IO.mapOptional("AlwaysBreakAfterReturnType", Style.BreakAfterReturnType); IO.mapOptional("AlwaysBreakTemplateDeclarations", Style.BreakTemplateDeclarations); + IO.mapOptional("BinPackParameters", BinPackParameters); IO.mapOptional("BreakBeforeInheritanceComma", BreakBeforeInheritanceComma); IO.mapOptional("BreakConstructorInitializersBeforeComma", @@ -947,7 +965,6 @@ template <> struct MappingTraits<FormatStyle> { Style.AlwaysBreakBeforeMultilineStrings); IO.mapOptional("AttributeMacros", Style.AttributeMacros); IO.mapOptional("BinPackArguments", Style.BinPackArguments); - IO.mapOptional("BinPackParameters", Style.BinPackParameters); IO.mapOptional("BitFieldColonSpacing", Style.BitFieldColonSpacing); IO.mapOptional("BracedInitializerIndentWidth", Style.BracedInitializerIndentWidth); @@ -1037,6 +1054,7 @@ template <> struct MappingTraits<FormatStyle> { Style.ObjCSpaceBeforeProtocolList); IO.mapOptional("PackConstructorInitializers", Style.PackConstructorInitializers); + IO.mapOptional("PackParameters", Style.PackParameters); IO.mapOptional("PenaltyBreakAssignment", Style.PenaltyBreakAssignment); IO.mapOptional("PenaltyBreakBeforeFirstCallParameter", Style.PenaltyBreakBeforeFirstCallParameter); @@ -1174,6 +1192,18 @@ template <> struct MappingTraits<FormatStyle> { Style.PackConstructorInitializers = FormatStyle::PCIS_CurrentLine; } + // If BinPackParameters was specified but PackParameters was not, initialize + // the latter from the former for backwards compatibility. + if (IsChromiumOrMozilla) { + if (BinPackParameters && + (Style.PackParameters == FormatStyle::PPS_CurrentLine)) { + Style.PackParameters = FormatStyle::PPS_BinPack; + } + } else if (!BinPackParameters && + (Style.PackParameters == FormatStyle::PPS_BinPack)) { + Style.PackParameters = FormatStyle::PPS_CurrentLine; + } + if (Style.LineEnding == FormatStyle::LE_DeriveLF) { if (!DeriveLineEnding) Style.LineEnding = UseCRLF ? FormatStyle::LE_CRLF : FormatStyle::LE_LF; @@ -1449,7 +1479,6 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) { LLVMStyle.AlwaysBreakBeforeMultilineStrings = false; LLVMStyle.AttributeMacros.push_back("__capability"); LLVMStyle.BinPackArguments = true; - LLVMStyle.BinPackParameters = true; LLVMStyle.BitFieldColonSpacing = FormatStyle::BFCS_Both; LLVMStyle.BracedInitializerIndentWidth = std::nullopt; LLVMStyle.BraceWrapping = {/*AfterCaseLabel=*/false, @@ -1543,6 +1572,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) { LLVMStyle.ObjCSpaceAfterProperty = false; LLVMStyle.ObjCSpaceBeforeProtocolList = true; LLVMStyle.PackConstructorInitializers = FormatStyle::PCIS_BinPack; + LLVMStyle.PackParameters = FormatStyle::PPS_BinPack; LLVMStyle.PointerAlignment = FormatStyle::PAS_Right; LLVMStyle.PPIndentWidth = -1; LLVMStyle.QualifierAlignment = FormatStyle::QAS_Leave; @@ -1823,8 +1853,8 @@ FormatStyle getChromiumStyle(FormatStyle::LanguageKind Language) { ChromiumStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline; ChromiumStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never; ChromiumStyle.AllowShortLoopsOnASingleLine = false; - ChromiumStyle.BinPackParameters = false; ChromiumStyle.DerivePointerAlignment = false; + ChromiumStyle.PackParameters = FormatStyle::PPS_CurrentLine; if (Language == FormatStyle::LK_ObjC) ChromiumStyle.ColumnLimit = 80; } @@ -1838,7 +1868,6 @@ FormatStyle getMozillaStyle() { MozillaStyle.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_TopLevel; MozillaStyle.BinPackArguments = false; - MozillaStyle.BinPackParameters = false; MozillaStyle.BreakAfterReturnType = FormatStyle::RTBS_TopLevel; MozillaStyle.BreakBeforeBraces = FormatStyle::BS_Mozilla; MozillaStyle.BreakConstructorInitializers = FormatStyle::BCIS_BeforeComma; @@ -1851,6 +1880,7 @@ FormatStyle getMozillaStyle() { MozillaStyle.IndentCaseLabels = true; MozillaStyle.ObjCSpaceAfterProperty = true; MozillaStyle.ObjCSpaceBeforeProtocolList = false; + MozillaStyle.PackParameters = FormatStyle::PPS_CurrentLine; MozillaStyle.PenaltyReturnTypeOnItsOwnLine = 200; MozillaStyle.PointerAlignment = FormatStyle::PAS_Left; MozillaStyle.SpaceAfterTemplateKeyword = false; diff --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h index abcedb66b57cc..fb97f4630ab24 100644 --- a/clang/lib/Format/FormatToken.h +++ b/clang/lib/Format/FormatToken.h @@ -1978,6 +1978,24 @@ inline bool continuesLineComment(const FormatToken &FormatTok, FormatTok.OriginalColumn >= MinContinueColumn; } +// Returns \c true if \c Current starts a new parameter. +inline bool startsNextParameter(const FormatToken &Current, + const FormatStyle &Style) { + const FormatToken &Previous = *Current.Previous; + if (Current.is(TT_CtorInitializerComma) && + Style.BreakConstructorInitializers == FormatStyle::BCIS_BeforeComma) { + return true; + } + if (Style.Language == FormatStyle::LK_Proto && Current.is(TT_SelectorName)) + return true; + return Previous.is(tok::comma) && !Current.isTrailingComment() && + ((Previous.isNot(TT_CtorInitializerComma) || + Style.BreakConstructorInitializers != + FormatStyle::BCIS_BeforeComma) && + (Previous.isNot(TT_InheritanceComma) || + Style.BreakInheritanceList != FormatStyle::BILS_BeforeComma)); +} + } // namespace format } // namespace clang diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp index 4ed3e9d0e8e85..99a137cd6477d 100644 --- a/clang/lib/Format/TokenAnnotator.cpp +++ b/clang/lib/Format/TokenAnnotator.cpp @@ -5458,6 +5458,14 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line, return true; } + // Ignores the first parameter as this will be handled separately by + // BreakFunctionDefinitionParameters or AlignAfterOpenBracket. + if ((FormatStyle::PPS_BreakAlways == Style.PackParameters) && + Line.MightBeFunctionDecl && !Left.opensScope() && + startsNextParameter(Right, Style)) { + return true; + } + const auto *BeforeLeft = Left.Previous; const auto *AfterRight = Right.Next; diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp index cc044153b7c9b..20d5d9c24b32f 100644 --- a/clang/unittests/Format/ConfigParseTest.cpp +++ b/clang/unittests/Format/ConfigParseTest.cpp @@ -160,7 +160,6 @@ TEST(ConfigParseTest, ParsesConfigurationBools) { CHECK_PARSE_BOOL(AllowShortEnumsOnASingleLine); CHECK_PARSE_BOOL(AllowShortLoopsOnASingleLine); CHECK_PARSE_BOOL(BinPackArguments); - CHECK_PARSE_BOOL(BinPackParameters); CHECK_PARSE_BOOL(BreakAdjacentStringLiterals); CHECK_PARSE_BOOL(BreakAfterJavaFieldAnnotations); CHECK_PARSE_BOOL(BreakBeforeTernaryOperators); @@ -456,6 +455,21 @@ TEST(ConfigParseTest, ParsesConfiguration) { "AllowAllConstructorInitializersOnNextLine: false", PackConstructorInitializers, FormatStyle::PCIS_CurrentLine); + Style.PackParameters = FormatStyle::PPS_BinPack; + CHECK_PARSE("PackParameters: CurrentLine", PackParameters, + FormatStyle::PPS_CurrentLine); + CHECK_PARSE("PackParameters: BinPack", PackParameters, + FormatStyle::PPS_BinPack); + CHECK_PARSE("PackParameters: BreakAlways", PackParameters, + FormatStyle::PPS_BreakAlways); + // For backward compatibility + CHECK_PARSE("BasedOnStyle: Mozilla\n" + "BinPackParameters: true", + PackParameters, FormatStyle::PPS_BinPack); + CHECK_PARSE("BasedOnStyle: Llvm\n" + "BinPackParameters: false", + PackParameters, FormatStyle::PPS_CurrentLine); + Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_LogicalBlock; CHECK_PARSE("EmptyLineBeforeAccessModifier: Never", EmptyLineBeforeAccessModifier, FormatStyle::ELBAMS_Never); diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 2a754a29e81e7..6de132ee4af17 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -2338,7 +2338,7 @@ TEST_F(FormatTest, FormatsForLoop) { "for (const Foo<Bar> &baz = in.value(); !baz.at_end(); ++baz) {\n}"); FormatStyle NoBinPacking = getLLVMStyle(); - NoBinPacking.BinPackParameters = false; + NoBinPacking.PackParameters = FormatStyle::PPS_CurrentLine; verifyFormat("for (int aaaaaaaaaaa = 1;\n" " aaaaaaaaaaa <= aaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaa,\n" " aaaaaaaaaaaaaaaa,\n" @@ -7165,7 +7165,7 @@ TEST_F(FormatTest, LineBreakingInBinaryExpressions) { "}"); FormatStyle OnePerLine = getLLVMStyle(); - OnePerLine.BinPackParameters = false; + OnePerLine.PackParameters = FormatStyle::PPS_CurrentLine; verifyFormat( "if (aaaaaaaaaaaaaaaaaaaaaaaaaaaa || aaaaaaaaaaaaaaaaaaaaaaaaaaaa ||\n" " aaaaaaaaaaaaaaaaaaaaaaaaaaaa || aaaaaaaaaaaaaaaaaaaaaaaaaaaa ||\n" @@ -7319,7 +7319,7 @@ TEST_F(FormatTest, ExpressionIndentationBreakingBeforeOperators) { Style = getLLVMStyleWithColumns(20); Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak; - Style.BinPackParameters = false; + Style.PackParameters = FormatStyle::PPS_CurrentLine; Style.BreakBeforeBinaryOperators = FormatStyle::BOS_NonAssignment; Style.ContinuationIndentWidth = 2; verifyFormat("struct Foo {\n" @@ -7694,7 +7694,7 @@ TEST_F(FormatTest, ConstructorInitializers) { " : aaaaa(aaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaa,\n" " aaaaaaaaaaaaaaaaaaaaaa) {}", OnePerLine); - OnePerLine.BinPackParameters = false; + OnePerLine.PackParameters = FormatStyle::PPS_CurrentLine; verifyFormat( "Constructor()\n" " : aaaaaaaaaaaaaaaaaaaaaaaa(\n" @@ -7718,7 +7718,7 @@ TEST_F(FormatTest, ConstructorInitializers) { TEST_F(FormatTest, AllowAllConstructorInitializersOnNextLine) { FormatStyle Style = getLLVMStyleWithColumns(60); Style.BreakConstructorInitializers = FormatStyle::BCIS_BeforeComma; - Style.BinPackParameters = false; + Style.PackParameters = FormatStyle::PPS_CurrentLine; for (int i = 0; i < 4; ++i) { ... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/101882 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
