This should be fixed by r332465 On Wed, May 16, 2018 at 2:50 PM Eric Liu <ioe...@google.com> wrote:
> 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 <ioe...@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@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@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@lists.llvm.org >>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>> >>> _______________________________________________ >>> cfe-commits mailing list >>> cfe-commits@lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>> >>
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
