r332436 - clang-format: Allow optimizer to break template declaration.
Eric Liu via cfe-commits
cfe-commits at lists.llvm.org
Wed May 16 05:50:51 PDT 2018
OK, I'll send a fix for the index test. Please make sure you run all clang
tests (`ninja check-clang` and `ninja check-clang-tools`) before landing a
patch! Thanks!
On Wed, May 16, 2018 at 2:32 PM Eric Liu <ioeric at google.com> wrote:
> 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/92b703fd/attachment-0001.html>
More information about the cfe-commits
mailing list