From 879bfe6a979295f834b76df66b19a203b93eed0f Mon Sep 17 00:00:00 2001 From: Krasimir Georgiev Date: Tue, 10 Jan 2023 09:23:44 +0000 Subject: [PATCH] Revert "[clang-format] Add an option for breaking after C++11 attributes" This reverts commit a28f0747c2f3728bd8a6f64f7c8ba80b4e0cda9f. It appears that this regresses some function definitions, added an example as a comment over at 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, 7 insertions(+), 182 deletions(-) diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst index 4134d5a..23f5786 100644 --- a/clang/docs/ClangFormatStyleOptions.rst +++ b/clang/docs/ClangFormatStyleOptions.rst @@ -1762,41 +1762,6 @@ 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 8718dea..927c852 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -889,8 +889,6 @@ 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. - Add ``InsertNewlineAtEOF`` option for inserting a newline at EOF if missing. clang-extdef-mapping diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h index 6e5d3a1..3a7a183 100644 --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -1221,36 +1221,6 @@ 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). @@ -4113,7 +4083,6 @@ 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 afc1860..04244c1 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -566,7 +566,6 @@ 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 68f24fa..bee9fb5 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -112,15 +112,6 @@ 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) { @@ -847,7 +838,6 @@ 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); @@ -1312,7 +1302,6 @@ 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 517beb6..e8c6cd9 100644 --- a/clang/lib/Format/FormatToken.h +++ b/clang/lib/Format/FormatToken.h @@ -247,8 +247,7 @@ struct FormatToken { CanBreakBefore(false), ClosesTemplateDeclaration(false), StartsBinaryExpression(false), EndsBinaryExpression(false), PartOfMultiVariableDeclStmt(false), ContinuesLineCommentSection(false), - Finalized(false), ClosesRequiresClause(false), - EndsCppAttributeGroup(false), BlockKind(BK_Unknown), + Finalized(false), ClosesRequiresClause(false), BlockKind(BK_Unknown), Decision(FD_Unformatted), PackingKind(PPK_Inconclusive), TypeIsFinalized(false), Type(TT_Unknown) {} @@ -319,9 +318,6 @@ 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 e37ffbd..c081b24 100644 --- a/clang/lib/Format/TokenAnnotator.cpp +++ b/clang/lib/Format/TokenAnnotator.cpp @@ -594,9 +594,8 @@ private: (Contexts.back().CanBeExpression || Contexts.back().IsExpression || Contexts.back().ContextType == Context::TemplateArgument); - const bool IsInnerSquare = Contexts.back().InCpp11AttributeSpecifier; - const bool IsCpp11AttributeSpecifier = - isCpp11AttributeSpecifier(*Left) || IsInnerSquare; + bool IsCpp11AttributeSpecifier = isCpp11AttributeSpecifier(*Left) || + Contexts.back().InCpp11AttributeSpecifier; // Treat C# Attributes [STAThread] much like C++ attributes [[...]]. bool IsCSharpAttributeSpecifier = @@ -631,8 +630,6 @@ 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)) { @@ -706,11 +703,8 @@ 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 && @@ -2932,18 +2926,6 @@ 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); @@ -2959,22 +2941,9 @@ 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 354511b..3cf2e38 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), - ReturnTypeWrapped(false), IsContinuation(Line.IsContinuation), + IsContinuation(Line.IsContinuation), FirstStartColumn(Line.FirstStartColumn) { assert(!Line.Tokens.empty()); @@ -151,9 +151,6 @@ 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 ccdeaac..b702164 100644 --- a/clang/unittests/Format/ConfigParseTest.cpp +++ b/clang/unittests/Format/ConfigParseTest.cpp @@ -879,13 +879,6 @@ 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 242e3f8..550c5a2 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -25127,56 +25127,6 @@ 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)); -} - TEST_F(FormatTest, InsertNewlineAtEOF) { FormatStyle Style = getLLVMStyle(); Style.InsertNewlineAtEOF = true; -- 2.7.4