r332436 - clang-format: Allow optimizer to break template declaration.
David Zarzycki via cfe-commits
cfe-commits at lists.llvm.org
Wed May 16 04:01:43 PDT 2018
This change broke `test/Index/overriding-ftemplate-comments.cpp` on Red Hat Fedora 28. Was this expected?
> On May 16, 2018, at 4:25 AM, Francois Ferrand via cfe-commits <cfe-commits at lists.llvm.org> wrote:
>
> Author: typz
> Date: Wed May 16 01:25:03 2018
> New Revision: 332436
>
> URL: http://llvm.org/viewvc/llvm-project?rev=332436&view=rev
> Log:
> clang-format: Allow optimizer to break template declaration.
>
> Summary:
> Introduce `PenaltyBreakTemplateDeclaration` to control the penalty,
> and change `AlwaysBreakTemplateDeclarations` to an enum with 3 modes:
> * `No` for regular, penalty based, wrapping of template declaration
> * `MultiLine` for always wrapping before multi-line declarations (e.g.
> same as legacy behavior when `AlwaysBreakTemplateDeclarations=false`)
> * `Yes` for always wrapping (e.g. same as legacy behavior when
> `AlwaysBreakTemplateDeclarations=true`)
>
> Reviewers: krasimir, djasper, klimek
>
> Subscribers: cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D42684
>
> Modified:
> cfe/trunk/docs/ClangFormatStyleOptions.rst
> cfe/trunk/include/clang/Format/Format.h
> cfe/trunk/lib/Format/ContinuationIndenter.cpp
> cfe/trunk/lib/Format/Format.cpp
> cfe/trunk/lib/Format/TokenAnnotator.cpp
> cfe/trunk/unittests/Format/FormatTest.cpp
>
> Modified: cfe/trunk/docs/ClangFormatStyleOptions.rst
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangFormatStyleOptions.rst?rev=332436&r1=332435&r2=332436&view=diff
> ==============================================================================
> --- cfe/trunk/docs/ClangFormatStyleOptions.rst (original)
> +++ cfe/trunk/docs/ClangFormatStyleOptions.rst Wed May 16 01:25:03 2018
> @@ -490,15 +490,50 @@ the configuration (without a prefix: ``A
> "bbbb" "cccc";
> "cccc";
>
> -**AlwaysBreakTemplateDeclarations** (``bool``)
> - If ``true``, always break after the ``template<...>`` of a template
> - declaration.
> +**AlwaysBreakTemplateDeclarations** (``BreakTemplateDeclarationsStyle``)
> + The template declaration breaking style to use.
> +
> + Possible values:
> +
> + * ``BTDS_No`` (in configuration: ``No``)
> + Do not force break before declaration.
> + ``PenaltyBreakTemplateDeclaration`` is taken into account.
> +
> + .. code-block:: c++
> +
> + template <typename T> T foo() {
> + }
> + template <typename T> T foo(int aaaaaaaaaaaaaaaaaaaaa,
> + int bbbbbbbbbbbbbbbbbbbbb) {
> + }
> +
> + * ``BTDS_MultiLine`` (in configuration: ``MultiLine``)
> + Force break after template declaration only when the following
> + declaration spans multiple lines.
> +
> + .. code-block:: c++
> +
> + template <typename T> T foo() {
> + }
> + template <typename T>
> + T foo(int aaaaaaaaaaaaaaaaaaaaa,
> + int bbbbbbbbbbbbbbbbbbbbb) {
> + }
> +
> + * ``BTDS_Yes`` (in configuration: ``Yes``)
> + Always break after template declaration.
> +
> + .. code-block:: c++
> +
> + template <typename T>
> + T foo() {
> + }
> + template <typename T>
> + T foo(int aaaaaaaaaaaaaaaaaaaaa,
> + int bbbbbbbbbbbbbbbbbbbbb) {
> + }
>
> - .. code-block:: c++
>
> - true: false:
> - template <typename T> vs. template <typename T> class C {};
> - class C {};
>
> **BinPackArguments** (``bool``)
> If ``false``, a function call's arguments will either be all on the
> @@ -1590,6 +1625,9 @@ the configuration (without a prefix: ``A
> **PenaltyBreakString** (``unsigned``)
> The penalty for each line break introduced inside a string literal.
>
> +**PenaltyBreakTemplateDeclaration** (``unsigned``)
> + The penalty for breaking after template declaration.
> +
> **PenaltyExcessCharacter** (``unsigned``)
> The penalty for each character outside of the column limit.
>
>
> Modified: cfe/trunk/include/clang/Format/Format.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=332436&r1=332435&r2=332436&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Format/Format.h (original)
> +++ cfe/trunk/include/clang/Format/Format.h Wed May 16 01:25:03 2018
> @@ -351,14 +351,44 @@ struct FormatStyle {
> /// \endcode
> bool AlwaysBreakBeforeMultilineStrings;
>
> - /// If ``true``, always break after the ``template<...>`` of a template
> - /// declaration.
> - /// \code
> - /// true: false:
> - /// template <typename T> vs. template <typename T> class C {};
> - /// class C {};
> - /// \endcode
> - bool AlwaysBreakTemplateDeclarations;
> + /// Different ways to break after the template declaration.
> + enum BreakTemplateDeclarationsStyle {
> + /// Do not force break before declaration.
> + /// ``PenaltyBreakTemplateDeclaration`` is taken into account.
> + /// \code
> + /// template <typename T> T foo() {
> + /// }
> + /// template <typename T> T foo(int aaaaaaaaaaaaaaaaaaaaa,
> + /// int bbbbbbbbbbbbbbbbbbbbb) {
> + /// }
> + /// \endcode
> + BTDS_No,
> + /// Force break after template declaration only when the following
> + /// declaration spans multiple lines.
> + /// \code
> + /// template <typename T> T foo() {
> + /// }
> + /// template <typename T>
> + /// T foo(int aaaaaaaaaaaaaaaaaaaaa,
> + /// int bbbbbbbbbbbbbbbbbbbbb) {
> + /// }
> + /// \endcode
> + BTDS_MultiLine,
> + /// Always break after template declaration.
> + /// \code
> + /// template <typename T>
> + /// T foo() {
> + /// }
> + /// template <typename T>
> + /// T foo(int aaaaaaaaaaaaaaaaaaaaa,
> + /// int bbbbbbbbbbbbbbbbbbbbb) {
> + /// }
> + /// \endcode
> + BTDS_Yes
> + };
> +
> + /// The template declaration breaking style to use.
> + BreakTemplateDeclarationsStyle AlwaysBreakTemplateDeclarations;
>
> /// If ``false``, a function call's arguments will either be all on the
> /// same line or will have one line each.
> @@ -1295,6 +1325,9 @@ struct FormatStyle {
> /// The penalty for each line break introduced inside a string literal.
> unsigned PenaltyBreakString;
>
> + /// The penalty for breaking after template declaration.
> + unsigned PenaltyBreakTemplateDeclaration;
> +
> /// The penalty for each character outside of the column limit.
> unsigned PenaltyExcessCharacter;
>
> @@ -1679,6 +1712,8 @@ struct FormatStyle {
> PenaltyBreakString == R.PenaltyBreakString &&
> PenaltyExcessCharacter == R.PenaltyExcessCharacter &&
> PenaltyReturnTypeOnItsOwnLine == R.PenaltyReturnTypeOnItsOwnLine &&
> + PenaltyBreakTemplateDeclaration ==
> + R.PenaltyBreakTemplateDeclaration &&
> PointerAlignment == R.PointerAlignment &&
> RawStringFormats == R.RawStringFormats &&
> SpaceAfterCStyleCast == R.SpaceAfterCStyleCast &&
>
> Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=332436&r1=332435&r2=332436&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
> +++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Wed May 16 01:25:03 2018
> @@ -402,6 +402,12 @@ bool ContinuationIndenter::mustBreak(con
> Style.Language == FormatStyle::LK_JavaScript))
> return true;
>
> + // If the template declaration spans multiple lines, force wrap before the
> + // function/class declaration
> + if (Previous.ClosesTemplateDeclaration &&
> + State.Stack.back().BreakBeforeParameter)
> + return true;
> +
> if (State.Column <= NewLineColumn)
> return false;
>
> @@ -453,7 +459,7 @@ bool ContinuationIndenter::mustBreak(con
> // for cases where the entire line does not fit on a single line as a
> // different LineFormatter would be used otherwise.
> if (Previous.ClosesTemplateDeclaration)
> - return true;
> + return Style.AlwaysBreakTemplateDeclarations != FormatStyle::BTDS_No;
> if (Previous.is(TT_FunctionAnnotationRParen))
> return true;
> if (Previous.is(TT_LeadingJavaAnnotation) && Current.isNot(tok::l_paren) &&
>
> Modified: cfe/trunk/lib/Format/Format.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=332436&r1=332435&r2=332436&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Format/Format.cpp (original)
> +++ cfe/trunk/lib/Format/Format.cpp Wed May 16 01:25:03 2018
> @@ -169,6 +169,19 @@ struct ScalarEnumerationTraits<FormatSty
> };
>
> template <>
> +struct ScalarEnumerationTraits<FormatStyle::BreakTemplateDeclarationsStyle> {
> + static void enumeration(IO &IO, FormatStyle::BreakTemplateDeclarationsStyle &Value) {
> + IO.enumCase(Value, "No", FormatStyle::BTDS_No);
> + IO.enumCase(Value, "MultiLine", FormatStyle::BTDS_MultiLine);
> + IO.enumCase(Value, "Yes", FormatStyle::BTDS_Yes);
> +
> + // For backward compatibility.
> + IO.enumCase(Value, "false", FormatStyle::BTDS_MultiLine);
> + IO.enumCase(Value, "true", FormatStyle::BTDS_Yes);
> + }
> +};
> +
> +template <>
> struct ScalarEnumerationTraits<FormatStyle::DefinitionReturnTypeBreakingStyle> {
> static void
> enumeration(IO &IO, FormatStyle::DefinitionReturnTypeBreakingStyle &Value) {
> @@ -400,6 +413,8 @@ template <> struct MappingTraits<FormatS
> IO.mapOptional("PenaltyBreakFirstLessLess",
> Style.PenaltyBreakFirstLessLess);
> IO.mapOptional("PenaltyBreakString", Style.PenaltyBreakString);
> + IO.mapOptional("PenaltyBreakTemplateDeclaration",
> + Style.PenaltyBreakTemplateDeclaration);
> IO.mapOptional("PenaltyExcessCharacter", Style.PenaltyExcessCharacter);
> IO.mapOptional("PenaltyReturnTypeOnItsOwnLine",
> Style.PenaltyReturnTypeOnItsOwnLine);
> @@ -598,7 +613,7 @@ FormatStyle getLLVMStyle() {
> LLVMStyle.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None;
> LLVMStyle.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None;
> LLVMStyle.AlwaysBreakBeforeMultilineStrings = false;
> - LLVMStyle.AlwaysBreakTemplateDeclarations = false;
> + LLVMStyle.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_MultiLine;
> LLVMStyle.BinPackArguments = true;
> LLVMStyle.BinPackParameters = true;
> LLVMStyle.BreakBeforeBinaryOperators = FormatStyle::BOS_None;
> @@ -670,6 +685,7 @@ FormatStyle getLLVMStyle() {
> LLVMStyle.PenaltyExcessCharacter = 1000000;
> LLVMStyle.PenaltyReturnTypeOnItsOwnLine = 60;
> LLVMStyle.PenaltyBreakBeforeFirstCallParameter = 19;
> + LLVMStyle.PenaltyBreakTemplateDeclaration = prec::Relational;
>
> LLVMStyle.DisableFormat = false;
> LLVMStyle.SortIncludes = true;
> @@ -694,7 +710,7 @@ FormatStyle getGoogleStyle(FormatStyle::
> GoogleStyle.AllowShortIfStatementsOnASingleLine = true;
> GoogleStyle.AllowShortLoopsOnASingleLine = true;
> GoogleStyle.AlwaysBreakBeforeMultilineStrings = true;
> - GoogleStyle.AlwaysBreakTemplateDeclarations = true;
> + GoogleStyle.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_Yes;
> GoogleStyle.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
> GoogleStyle.DerivePointerAlignment = true;
> GoogleStyle.IncludeStyle.IncludeCategories = {
> @@ -819,7 +835,7 @@ FormatStyle getMozillaStyle() {
> MozillaStyle.AlwaysBreakAfterReturnType = FormatStyle::RTBS_TopLevel;
> MozillaStyle.AlwaysBreakAfterDefinitionReturnType =
> FormatStyle::DRTBS_TopLevel;
> - MozillaStyle.AlwaysBreakTemplateDeclarations = true;
> + MozillaStyle.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_Yes;
> MozillaStyle.BinPackParameters = false;
> MozillaStyle.BinPackArguments = false;
> MozillaStyle.BreakBeforeBraces = FormatStyle::BS_Mozilla;
>
> Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=332436&r1=332435&r2=332436&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
> +++ cfe/trunk/lib/Format/TokenAnnotator.cpp Wed May 16 01:25:03 2018
> @@ -2338,6 +2338,8 @@ unsigned TokenAnnotator::splitPenalty(co
> return 2;
> return 1;
> }
> + if (Left.ClosesTemplateDeclaration)
> + return Style.PenaltyBreakTemplateDeclaration;
> if (Left.is(TT_ConditionalExpr))
> return prec::Conditional;
> prec::Level Level = Left.getPrecedence();
> @@ -2869,7 +2871,7 @@ bool TokenAnnotator::mustBreakBefore(con
> if (Right.Previous->ClosesTemplateDeclaration &&
> Right.Previous->MatchingParen &&
> Right.Previous->MatchingParen->NestingLevel == 0 &&
> - Style.AlwaysBreakTemplateDeclarations)
> + Style.AlwaysBreakTemplateDeclarations == FormatStyle::BTDS_Yes)
> return true;
> if (Right.is(TT_CtorInitializerComma) &&
> Style.BreakConstructorInitializers == FormatStyle::BCIS_BeforeComma &&
>
> Modified: cfe/trunk/unittests/Format/FormatTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=332436&r1=332435&r2=332436&view=diff
> ==============================================================================
> --- cfe/trunk/unittests/Format/FormatTest.cpp (original)
> +++ cfe/trunk/unittests/Format/FormatTest.cpp Wed May 16 01:25:03 2018
> @@ -5475,7 +5475,7 @@ TEST_F(FormatTest, WrapsTemplateDeclarat
> " const typename aaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaaaaa);");
>
> FormatStyle AlwaysBreak = getLLVMStyle();
> - AlwaysBreak.AlwaysBreakTemplateDeclarations = true;
> + AlwaysBreak.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_Yes;
> verifyFormat("template <typename T>\nclass C {};", AlwaysBreak);
> verifyFormat("template <typename T>\nvoid f();", AlwaysBreak);
> verifyFormat("template <typename T>\nvoid f() {}", AlwaysBreak);
> @@ -5493,6 +5493,32 @@ TEST_F(FormatTest, WrapsTemplateDeclarat
> "public:\n"
> " E *f();\n"
> "};");
> +
> + FormatStyle NeverBreak = getLLVMStyle();
> + NeverBreak.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_No;
> + verifyFormat("template <typename T> class C {};", NeverBreak);
> + verifyFormat("template <typename T> void f();", NeverBreak);
> + verifyFormat("template <typename T> void f() {}", NeverBreak);
> + verifyFormat("template <typename T>\nvoid foo(aaaaaaaaaaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbbbb) {}",
> + NeverBreak);
> + verifyFormat("void aaaaaaaaaaaaaaaaaaa<aaaaaaaaaaaaaaaaaaaaaaaaaaaaa,\n"
> + " bbbbbbbbbbbbbbbbbbbbbbbbbbbb>(\n"
> + " ccccccccccccccccccccccccccccccccccccccccccccccc);",
> + NeverBreak);
> + verifyFormat("template <template <typename> class Fooooooo,\n"
> + " template <typename> class Baaaaaaar>\n"
> + "struct C {};",
> + NeverBreak);
> + verifyFormat("template <typename T> // T can be A, B or C.\n"
> + "struct C {};",
> + NeverBreak);
> + verifyFormat("template <enum E> class A {\n"
> + "public:\n"
> + " E *f();\n"
> + "};", NeverBreak);
> + NeverBreak.PenaltyBreakTemplateDeclaration = 100;
> + verifyFormat("template <typename T> void\nfoo(aaaaaaaaaaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbbbb) {}",
> + NeverBreak);
> }
>
> TEST_F(FormatTest, WrapsTemplateParameters) {
> @@ -10440,7 +10466,6 @@ TEST_F(FormatTest, ParsesConfigurationBo
> CHECK_PARSE_BOOL(AllowShortCaseLabelsOnASingleLine);
> CHECK_PARSE_BOOL(AllowShortIfStatementsOnASingleLine);
> CHECK_PARSE_BOOL(AllowShortLoopsOnASingleLine);
> - CHECK_PARSE_BOOL(AlwaysBreakTemplateDeclarations);
> CHECK_PARSE_BOOL(BinPackArguments);
> CHECK_PARSE_BOOL(BinPackParameters);
> CHECK_PARSE_BOOL(BreakAfterJavaFieldAnnotations);
> @@ -10506,6 +10531,8 @@ TEST_F(FormatTest, ParsesConfiguration)
> PenaltyBreakAssignment, 1234u);
> CHECK_PARSE("PenaltyBreakBeforeFirstCallParameter: 1234",
> PenaltyBreakBeforeFirstCallParameter, 1234u);
> + CHECK_PARSE("PenaltyBreakTemplateDeclaration: 1234",
> + PenaltyBreakTemplateDeclaration, 1234u);
> CHECK_PARSE("PenaltyExcessCharacter: 1234", PenaltyExcessCharacter, 1234u);
> CHECK_PARSE("PenaltyReturnTypeOnItsOwnLine: 1234",
> PenaltyReturnTypeOnItsOwnLine, 1234u);
> @@ -10660,6 +10687,18 @@ TEST_F(FormatTest, ParsesConfiguration)
> AlwaysBreakAfterReturnType,
> FormatStyle::RTBS_TopLevelDefinitions);
>
> + Style.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_Yes;
> + CHECK_PARSE("AlwaysBreakTemplateDeclarations: No", AlwaysBreakTemplateDeclarations,
> + FormatStyle::BTDS_No);
> + CHECK_PARSE("AlwaysBreakTemplateDeclarations: MultiLine", AlwaysBreakTemplateDeclarations,
> + FormatStyle::BTDS_MultiLine);
> + CHECK_PARSE("AlwaysBreakTemplateDeclarations: Yes", AlwaysBreakTemplateDeclarations,
> + FormatStyle::BTDS_Yes);
> + CHECK_PARSE("AlwaysBreakTemplateDeclarations: false", AlwaysBreakTemplateDeclarations,
> + FormatStyle::BTDS_MultiLine);
> + CHECK_PARSE("AlwaysBreakTemplateDeclarations: true", AlwaysBreakTemplateDeclarations,
> + FormatStyle::BTDS_Yes);
> +
> Style.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_All;
> CHECK_PARSE("AlwaysBreakAfterDefinitionReturnType: None",
> AlwaysBreakAfterDefinitionReturnType, FormatStyle::DRTBS_None);
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list