From a28f0747c2f3728bd8a6f64f7c8ba80b4e0cda9f Mon Sep 17 00:00:00 2001 From: Owen Pan Date: Tue, 3 Jan 2023 23:07:15 -0800 Subject: [PATCH] [clang-format] Add an option for breaking after C++11 attributes Fixes #45968. Fixes #54265. Fixes #58102. Differential Revision: https://reviews.llvm.org/D140956 --- clang/docs/ClangFormatStyleOptions.rst | 35 +++++++++++++++++++++ clang/docs/ReleaseNotes.rst | 2 ++ clang/include/clang/Format/Format.h | 31 ++++++++++++++++++ clang/lib/Format/ContinuationIndenter.cpp | 1 + clang/lib/Format/Format.cpp | 11 +++++++ clang/lib/Format/FormatToken.h | 6 +++- clang/lib/Format/TokenAnnotator.cpp | 41 +++++++++++++++++++++--- clang/lib/Format/TokenAnnotator.h | 5 ++- clang/unittests/Format/ConfigParseTest.cpp | 7 +++++ clang/unittests/Format/FormatTest.cpp | 50 ++++++++++++++++++++++++++++++ 10 files changed, 182 insertions(+), 7 deletions(-) diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst index b69a115..2fa0eef 100644 --- a/clang/docs/ClangFormatStyleOptions.rst +++ b/clang/docs/ClangFormatStyleOptions.rst @@ -1762,6 +1762,41 @@ the configuration (without a prefix: ``Auto``). } +**BreakAfterAttributes** (``AttributeBreakingStyle``) :versionbadge:`clang-format 16` + Break after a group of C++11 attributes before a function + declaration/definition name. + + Possible values: + + * ``ABS_Always`` (in configuration: ``Always``) + Always break after attributes. + + .. code-block:: c++ + + [[nodiscard]] + inline int f(); + [[gnu::const]] [[nodiscard]] + int g(); + + * ``ABS_Leave`` (in configuration: ``Leave``) + Leave the line breaking after attributes as is. + + .. code-block:: c++ + + [[nodiscard]] inline int f(); + [[gnu::const]] [[nodiscard]] + int g(); + + * ``ABS_Never`` (in configuration: ``Never``) + Never break after attributes. + + .. code-block:: c++ + + [[nodiscard]] inline int f(); + [[gnu::const]] [[nodiscard]] int g(); + + + **BreakAfterJavaFieldAnnotations** (``Boolean``) :versionbadge:`clang-format 3.8` Break after each annotation on a field in Java files. diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index a3abdf4..cf7c730 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -867,6 +867,8 @@ clang-format To match the default behavior of clang-format 15, use the ``Keyword`` value. - Add ``IntegerLiteralSeparator`` option for fixing integer literal separators in C++, C#, Java, and JavaScript. +- Add ``BreakAfterAttributes`` option for breaking after a group of C++11 + attributes before a function declaration/definition name. clang-extdef-mapping -------------------- diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h index b6fd661..1762ff9 100644 --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -1221,6 +1221,36 @@ struct FormatStyle { /// \version 3.8 BraceWrappingFlags BraceWrapping; + /// Different ways to break after attributes. + enum AttributeBreakingStyle : int8_t { + /// Always break after attributes. + /// \code + /// [[nodiscard]] + /// inline int f(); + /// [[gnu::const]] [[nodiscard]] + /// int g(); + /// \endcode + ABS_Always, + /// Leave the line breaking after attributes as is. + /// \code + /// [[nodiscard]] inline int f(); + /// [[gnu::const]] [[nodiscard]] + /// int g(); + /// \endcode + ABS_Leave, + /// Never break after attributes. + /// \code + /// [[nodiscard]] inline int f(); + /// [[gnu::const]] [[nodiscard]] int g(); + /// \endcode + ABS_Never, + }; + + /// Break after a group of C++11 attributes before a function + /// declaration/definition name. + /// \version 16 + AttributeBreakingStyle BreakAfterAttributes; + /// If ``true``, clang-format will always break after a Json array `[` /// otherwise it will scan until the closing `]` to determine if it should add /// newlines between elements (prettier compatible). @@ -4079,6 +4109,7 @@ struct FormatStyle { BinPackArguments == R.BinPackArguments && BinPackParameters == R.BinPackParameters && BitFieldColonSpacing == R.BitFieldColonSpacing && + BreakAfterAttributes == R.BreakAfterAttributes && BreakAfterJavaFieldAnnotations == R.BreakAfterJavaFieldAnnotations && BreakArrays == R.BreakArrays && BreakBeforeBinaryOperators == R.BreakBeforeBinaryOperators && diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp index 04244c1..afc1860 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -566,6 +566,7 @@ bool ContinuationIndenter::mustBreak(const LineState &State) { // If the return type spans multiple lines, wrap before the function name. if (((Current.is(TT_FunctionDeclarationName) && + !State.Line->ReturnTypeWrapped && // Don't break before a C# function when no break after return type. (!Style.isCSharp() || Style.AlwaysBreakAfterReturnType != FormatStyle::RTBS_None) && diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index 3304aac..223a86b 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -112,6 +112,15 @@ template <> struct MappingTraits { }; template <> +struct ScalarEnumerationTraits { + static void enumeration(IO &IO, FormatStyle::AttributeBreakingStyle &Value) { + IO.enumCase(Value, "Always", FormatStyle::ABS_Always); + IO.enumCase(Value, "Leave", FormatStyle::ABS_Leave); + IO.enumCase(Value, "Never", FormatStyle::ABS_Never); + } +}; + +template <> struct ScalarEnumerationTraits { static void enumeration(IO &IO, FormatStyle::ArrayInitializerAlignmentStyle &Value) { @@ -838,6 +847,7 @@ template <> struct MappingTraits { IO.mapOptional("BinPackParameters", Style.BinPackParameters); IO.mapOptional("BitFieldColonSpacing", Style.BitFieldColonSpacing); IO.mapOptional("BraceWrapping", Style.BraceWrapping); + IO.mapOptional("BreakAfterAttributes", Style.BreakAfterAttributes); IO.mapOptional("BreakAfterJavaFieldAnnotations", Style.BreakAfterJavaFieldAnnotations); IO.mapOptional("BreakArrays", Style.BreakArrays); @@ -1301,6 +1311,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) { /*SplitEmptyFunction=*/true, /*SplitEmptyRecord=*/true, /*SplitEmptyNamespace=*/true}; + LLVMStyle.BreakAfterAttributes = FormatStyle::ABS_Never; LLVMStyle.BreakAfterJavaFieldAnnotations = false; LLVMStyle.BreakArrays = true; LLVMStyle.BreakBeforeBinaryOperators = FormatStyle::BOS_None; diff --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h index e8c6cd9..517beb6 100644 --- a/clang/lib/Format/FormatToken.h +++ b/clang/lib/Format/FormatToken.h @@ -247,7 +247,8 @@ struct FormatToken { CanBreakBefore(false), ClosesTemplateDeclaration(false), StartsBinaryExpression(false), EndsBinaryExpression(false), PartOfMultiVariableDeclStmt(false), ContinuesLineCommentSection(false), - Finalized(false), ClosesRequiresClause(false), BlockKind(BK_Unknown), + Finalized(false), ClosesRequiresClause(false), + EndsCppAttributeGroup(false), BlockKind(BK_Unknown), Decision(FD_Unformatted), PackingKind(PPK_Inconclusive), TypeIsFinalized(false), Type(TT_Unknown) {} @@ -318,6 +319,9 @@ struct FormatToken { /// \c true if this is the last token within requires clause. unsigned ClosesRequiresClause : 1; + /// \c true if this token ends a group of C++ attributes. + unsigned EndsCppAttributeGroup : 1; + private: /// Contains the kind of block if this token is a brace. unsigned BlockKind : 2; diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp index 52314a1..05fd60e 100644 --- a/clang/lib/Format/TokenAnnotator.cpp +++ b/clang/lib/Format/TokenAnnotator.cpp @@ -595,8 +595,9 @@ private: (Contexts.back().CanBeExpression || Contexts.back().IsExpression || Contexts.back().ContextType == Context::TemplateArgument); - bool IsCpp11AttributeSpecifier = isCpp11AttributeSpecifier(*Left) || - Contexts.back().InCpp11AttributeSpecifier; + const bool IsInnerSquare = Contexts.back().InCpp11AttributeSpecifier; + const bool IsCpp11AttributeSpecifier = + isCpp11AttributeSpecifier(*Left) || IsInnerSquare; // Treat C# Attributes [STAThread] much like C++ attributes [[...]]. bool IsCSharpAttributeSpecifier = @@ -631,6 +632,8 @@ private: Left->setType(TT_InlineASMSymbolicNameLSquare); } else if (IsCpp11AttributeSpecifier) { Left->setType(TT_AttributeSquare); + if (!IsInnerSquare && Left->Previous) + Left->Previous->EndsCppAttributeGroup = false; } else if (Style.isJavaScript() && Parent && Contexts.back().ContextKind == tok::l_brace && Parent->isOneOf(tok::l_brace, tok::comma)) { @@ -704,8 +707,11 @@ private: while (CurrentToken) { if (CurrentToken->is(tok::r_square)) { - if (IsCpp11AttributeSpecifier) + if (IsCpp11AttributeSpecifier) { CurrentToken->setType(TT_AttributeSquare); + if (!IsInnerSquare) + CurrentToken->EndsCppAttributeGroup = true; + } if (IsCSharpAttributeSpecifier) { CurrentToken->setType(TT_AttributeSquare); } else if (((CurrentToken->Next && @@ -2923,6 +2929,18 @@ bool TokenAnnotator::mustBreakForReturnType(const AnnotatedLine &Line) const { return false; } +static bool mustBreakAfterAttributes(const FormatToken &Tok, + const FormatStyle &Style) { + switch (Style.BreakAfterAttributes) { + case FormatStyle::ABS_Always: + return true; + case FormatStyle::ABS_Leave: + return Tok.NewlinesBefore > 0; + default: + return false; + } +} + void TokenAnnotator::calculateFormattingInformation(AnnotatedLine &Line) const { for (AnnotatedLine *ChildLine : Line.Children) calculateFormattingInformation(*ChildLine); @@ -2938,9 +2956,22 @@ void TokenAnnotator::calculateFormattingInformation(AnnotatedLine &Line) const { if (AlignArrayOfStructures) calculateArrayInitializerColumnList(Line); + for (FormatToken *Tok = Current, *AfterLastAttribute = nullptr; Tok; + Tok = Tok->Next) { + if (isFunctionDeclarationName(Style.isCpp(), *Tok, Line)) { + Tok->setType(TT_FunctionDeclarationName); + if (AfterLastAttribute && + mustBreakAfterAttributes(*AfterLastAttribute, Style)) { + AfterLastAttribute->MustBreakBefore = true; + Line.ReturnTypeWrapped = true; + } + break; + } + if (Tok->Previous->EndsCppAttributeGroup) + AfterLastAttribute = Tok; + } + while (Current) { - if (isFunctionDeclarationName(Style.isCpp(), *Current, Line)) - Current->setType(TT_FunctionDeclarationName); const FormatToken *Prev = Current->Previous; if (Current->is(TT_LineComment)) { if (Prev->is(BK_BracedInit) && Prev->opensScope()) { diff --git a/clang/lib/Format/TokenAnnotator.h b/clang/lib/Format/TokenAnnotator.h index 3cf2e38..354511b 100644 --- a/clang/lib/Format/TokenAnnotator.h +++ b/clang/lib/Format/TokenAnnotator.h @@ -47,7 +47,7 @@ public: MustBeDeclaration(Line.MustBeDeclaration), MightBeFunctionDecl(false), IsMultiVariableDeclStmt(false), Affected(false), LeadingEmptyLinesAffected(false), ChildrenAffected(false), - IsContinuation(Line.IsContinuation), + ReturnTypeWrapped(false), IsContinuation(Line.IsContinuation), FirstStartColumn(Line.FirstStartColumn) { assert(!Line.Tokens.empty()); @@ -151,6 +151,9 @@ public: /// \c True if one of this line's children intersects with an input range. bool ChildrenAffected; + /// \c True if breaking after last attribute group in function return type. + bool ReturnTypeWrapped; + /// \c True if this line should be indented by ContinuationIndent in addition /// to the normal indention level. bool IsContinuation; diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp index d1c9093..75be2e2 100644 --- a/clang/unittests/Format/ConfigParseTest.cpp +++ b/clang/unittests/Format/ConfigParseTest.cpp @@ -878,6 +878,13 @@ TEST(ConfigParseTest, ParsesConfiguration) { BreakBeforeConceptDeclarations, FormatStyle::BBCDS_Always); CHECK_PARSE("BreakBeforeConceptDeclarations: false", BreakBeforeConceptDeclarations, FormatStyle::BBCDS_Allowed); + + CHECK_PARSE("BreakAfterAttributes: Always", BreakAfterAttributes, + FormatStyle::ABS_Always); + CHECK_PARSE("BreakAfterAttributes: Leave", BreakAfterAttributes, + FormatStyle::ABS_Leave); + CHECK_PARSE("BreakAfterAttributes: Never", BreakAfterAttributes, + FormatStyle::ABS_Never); } TEST(ConfigParseTest, ParsesConfigurationWithLanguages) { diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 79e2979..4e4d239 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -25121,6 +25121,56 @@ TEST_F(FormatTest, RemoveSemicolon) { #endif } +TEST_F(FormatTest, BreakAfterAttributes) { + FormatStyle Style = getLLVMStyle(); + EXPECT_EQ(Style.BreakAfterAttributes, FormatStyle::ABS_Never); + + const StringRef Code("[[nodiscard]] inline int f(int &i);\n" + "[[foo([[]])]] [[nodiscard]]\n" + "int g(int &i);\n" + "[[nodiscard]]\n" + "inline int f(int &i) {\n" + " i = 1;\n" + " return 0;\n" + "}\n" + "[[foo([[]])]] [[nodiscard]] int g(int &i) {\n" + " i = 0;\n" + " return 1;\n" + "}"); + + verifyFormat("[[nodiscard]] inline int f(int &i);\n" + "[[foo([[]])]] [[nodiscard]] int g(int &i);\n" + "[[nodiscard]] inline int f(int &i) {\n" + " i = 1;\n" + " return 0;\n" + "}\n" + "[[foo([[]])]] [[nodiscard]] int g(int &i) {\n" + " i = 0;\n" + " return 1;\n" + "}", + Code, Style); + + Style.BreakAfterAttributes = FormatStyle::ABS_Always; + verifyFormat("[[nodiscard]]\n" + "inline int f(int &i);\n" + "[[foo([[]])]] [[nodiscard]]\n" + "int g(int &i);\n" + "[[nodiscard]]\n" + "inline int f(int &i) {\n" + " i = 1;\n" + " return 0;\n" + "}\n" + "[[foo([[]])]] [[nodiscard]]\n" + "int g(int &i) {\n" + " i = 0;\n" + " return 1;\n" + "}", + Code, Style); + + Style.BreakAfterAttributes = FormatStyle::ABS_Leave; + EXPECT_EQ(Code, format(Code, Style)); +} + } // namespace } // namespace format } // namespace clang -- 2.7.4