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
