r332436 - clang-format: Allow optimizer to break template declaration.
Eric Liu via cfe-commits
cfe-commits at lists.llvm.org
Wed May 16 05:32:05 PDT 2018
I can also reproduce on my local machine (Debian).
On Wed, May 16, 2018 at 1:01 PM David Zarzycki via cfe-commits <
cfe-commits at lists.llvm.org> wrote:
> 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
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180516/4e593ec3/attachment-0001.html>
More information about the cfe-commits
mailing list