https://github.com/rmarker updated https://github.com/llvm/llvm-project/pull/78011
>From a1312a0a463bb946f336977b5b01ef7afbede678 Mon Sep 17 00:00:00 2001 From: rmarker <rmar...@outlook.com> Date: Thu, 11 Jan 2024 15:01:18 +1030 Subject: [PATCH 01/17] [clang-format] Add ShortReturnTypeColumn option. --- clang/docs/ClangFormatStyleOptions.rst | 13 +++++++ clang/docs/ReleaseNotes.rst | 1 + clang/include/clang/Format/Format.h | 12 ++++++ clang/lib/Format/ContinuationIndenter.cpp | 3 +- clang/lib/Format/Format.cpp | 2 + clang/unittests/Format/ConfigParseTest.cpp | 1 + clang/unittests/Format/FormatTest.cpp | 44 ++++++++++++++++++++++ 7 files changed, 75 insertions(+), 1 deletion(-) diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst index 4dc0de3a90f26..7836cc8f1c9bb 100644 --- a/clang/docs/ClangFormatStyleOptions.rst +++ b/clang/docs/ClangFormatStyleOptions.rst @@ -4999,6 +4999,19 @@ the configuration (without a prefix: ``Auto``). int bar; int bar; } // namespace b } // namespace b +.. _ShortReturnTypeColumn: + +**ShortReturnTypeColumn** (``Unsigned``) :versionbadge:`clang-format 19` :ref:`¶ <ShortReturnTypeColumn>` + When ``AlwaysBreakAfterReturnType`` is ``None``, line breaks are prevented + after short return types. This configures the column limit for a type + to be regarded as short. + + + .. note:: + + This isn't the length of the type itself, but the column where it + finishes. I.e. it includes indentation, etc. + .. _SkipMacroDefinitionBody: **SkipMacroDefinitionBody** (``Boolean``) :versionbadge:`clang-format 18` :ref:`¶ <SkipMacroDefinitionBody>` diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 5330cd9caad80..669b420fe21ec 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -184,6 +184,7 @@ AST Matchers clang-format ------------ +- Add ``ShortReturnTypeColumn`` option. libclang -------- diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h index bc9eecd42f9eb..7fd574c98a394 100644 --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -3932,6 +3932,17 @@ struct FormatStyle { /// \version 13 unsigned ShortNamespaceLines; + /// When ``AlwaysBreakAfterReturnType`` is ``None``, line breaks are prevented + /// after short return types. This configures the column limit for a type + /// to be regarded as short. + /// + /// \note + /// This isn't the length of the type itself, but the column where it + /// finishes. I.e. it includes indentation, etc. + /// \endnote + /// \version 19 + unsigned ShortReturnTypeColumn; + /// Do not format macro definition body. /// \version 18 bool SkipMacroDefinitionBody; @@ -4899,6 +4910,7 @@ struct FormatStyle { RequiresExpressionIndentation == R.RequiresExpressionIndentation && SeparateDefinitionBlocks == R.SeparateDefinitionBlocks && ShortNamespaceLines == R.ShortNamespaceLines && + ShortReturnTypeColumn == R.ShortReturnTypeColumn && SkipMacroDefinitionBody == R.SkipMacroDefinitionBody && SortIncludes == R.SortIncludes && SortJavaStaticImport == R.SortJavaStaticImport && diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp index a3eb9138b2183..3f9c0cc815745 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -328,7 +328,8 @@ bool ContinuationIndenter::canBreak(const LineState &State) { // Don't break after very short return types (e.g. "void") as that is often // unexpected. - if (Current.is(TT_FunctionDeclarationName) && State.Column < 6) { + if (Current.is(TT_FunctionDeclarationName) && + State.Column <= Style.ShortReturnTypeColumn) { if (Style.AlwaysBreakAfterReturnType == FormatStyle::RTBS_None) return false; } diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index ff326dc784783..35478fac7b496 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -1085,6 +1085,7 @@ template <> struct MappingTraits<FormatStyle> { Style.RequiresExpressionIndentation); IO.mapOptional("SeparateDefinitionBlocks", Style.SeparateDefinitionBlocks); IO.mapOptional("ShortNamespaceLines", Style.ShortNamespaceLines); + IO.mapOptional("ShortReturnTypeColumn", Style.ShortReturnTypeColumn); IO.mapOptional("SkipMacroDefinitionBody", Style.SkipMacroDefinitionBody); IO.mapOptional("SortIncludes", Style.SortIncludes); IO.mapOptional("SortJavaStaticImport", Style.SortJavaStaticImport); @@ -1557,6 +1558,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) { LLVMStyle.RequiresExpressionIndentation = FormatStyle::REI_OuterScope; LLVMStyle.SeparateDefinitionBlocks = FormatStyle::SDS_Leave; LLVMStyle.ShortNamespaceLines = 1; + LLVMStyle.ShortReturnTypeColumn = 5; LLVMStyle.SkipMacroDefinitionBody = false; LLVMStyle.SortIncludes = FormatStyle::SI_CaseSensitive; LLVMStyle.SortJavaStaticImport = FormatStyle::SJSIO_Before; diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp index 2a8d79359a49b..baf892b43320d 100644 --- a/clang/unittests/Format/ConfigParseTest.cpp +++ b/clang/unittests/Format/ConfigParseTest.cpp @@ -261,6 +261,7 @@ TEST(ConfigParseTest, ParsesConfiguration) { CHECK_PARSE("PenaltyExcessCharacter: 1234", PenaltyExcessCharacter, 1234u); CHECK_PARSE("PenaltyReturnTypeOnItsOwnLine: 1234", PenaltyReturnTypeOnItsOwnLine, 1234u); + CHECK_PARSE("ShortReturnTypeColumn: 1234", ShortReturnTypeColumn, 1234u); CHECK_PARSE("SpacesBeforeTrailingComments: 1234", SpacesBeforeTrailingComments, 1234u); CHECK_PARSE("IndentWidth: 32", IndentWidth, 32u); diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index e5e763edf5b5b..5325af89b90ee 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -12406,6 +12406,50 @@ TEST_F(FormatTest, BreaksLongDeclarations) { verifyFormat("template <typename T> // Templates on own line.\n" "static int // Some comment.\n" "MyFunction(int a);"); + + FormatStyle ShortReturnType = getLLVMStyle(); + verifyFormat("Type " + "Loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo" + "ooooooooong::\n" + " FunctionDeclaration();", + ShortReturnType); + verifyFormat("struct S {\n" + " Type\n" + " " + "Loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo" + "oooooooooooong::\n" + " FunctionDeclaration();\n" + "}", + ShortReturnType); + + ShortReturnType.ShortReturnTypeColumn = 0; + verifyFormat("Type\n" + "Loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo" + "ooooooooong::\n" + " FunctionDeclaration();", + ShortReturnType); + verifyFormat("struct S {\n" + " Type\n" + " " + "Loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo" + "oooooooooooong::\n" + " FunctionDeclaration();\n" + "}", + ShortReturnType); + + ShortReturnType.ShortReturnTypeColumn = 7; + verifyFormat("Type " + "Loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo" + "ooooooooong::\n" + " FunctionDeclaration();", + ShortReturnType); + verifyFormat("struct S {\n" + " Type " + "Loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo" + "oooooooooooong::\n" + " FunctionDeclaration();\n" + "}", + ShortReturnType); } TEST_F(FormatTest, FormatsAccessModifiers) { >From 66f4c6af253575a258f26bb1f7afeab0e44ffb28 Mon Sep 17 00:00:00 2001 From: rmarker <rmar...@outlook.com> Date: Fri, 26 Jan 2024 17:34:36 +1030 Subject: [PATCH 02/17] Take indentation into account for short return types. --- clang/lib/Format/ContinuationIndenter.cpp | 2 +- clang/unittests/Format/FormatTest.cpp | 9 ++++----- clang/unittests/Format/FormatTestSelective.cpp | 3 +-- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp index 3f9c0cc815745..3201bc0e7dc2c 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -329,7 +329,7 @@ bool ContinuationIndenter::canBreak(const LineState &State) { // Don't break after very short return types (e.g. "void") as that is often // unexpected. if (Current.is(TT_FunctionDeclarationName) && - State.Column <= Style.ShortReturnTypeColumn) { + State.Column - State.FirstIndent <= Style.ShortReturnTypeColumn) { if (Style.AlwaysBreakAfterReturnType == FormatStyle::RTBS_None) return false; } diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 5325af89b90ee..778d98a4493b6 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -12414,8 +12414,7 @@ TEST_F(FormatTest, BreaksLongDeclarations) { " FunctionDeclaration();", ShortReturnType); verifyFormat("struct S {\n" - " Type\n" - " " + " Type " "Loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo" "oooooooooooong::\n" " FunctionDeclaration();\n" @@ -14392,9 +14391,9 @@ TEST_F(FormatTest, SplitEmptyFunctionButNotRecord) { " bbbbbbbbbbbbbbbbbbb()\n" " {\n" " }\n" - " void\n" - " m(int aaaaaaaaaaaaaaaaaaaaaaaaaaaa,\n" - " int bbbbbbbbbbbbbbbbbbbbbbbb)\n" + " void m(\n" + " int aaaaaaaaaaaaaaaaaaaaaaaaaaaa,\n" + " int bbbbbbbbbbbbbbbbbbbbbbbb)\n" " {\n" " }\n" "};", diff --git a/clang/unittests/Format/FormatTestSelective.cpp b/clang/unittests/Format/FormatTestSelective.cpp index c21c9bfe60790..43de93d219b14 100644 --- a/clang/unittests/Format/FormatTestSelective.cpp +++ b/clang/unittests/Format/FormatTestSelective.cpp @@ -522,8 +522,7 @@ TEST_F(FormatTestSelective, ReformatRegionAdjustsIndent) { Style.ColumnLimit = 11; EXPECT_EQ(" int a;\n" - " void\n" - " ffffff() {\n" + " void ffffff() {\n" " }", format(" int a;\n" "void ffffff() {}", >From 4024406a85d4509b4a0bdd41eb8cfe0441f2a3e7 Mon Sep 17 00:00:00 2001 From: rmarker <rmar...@outlook.com> Date: Fri, 26 Jan 2024 20:52:15 +1030 Subject: [PATCH 03/17] Add AllowShortType option for AlwaysBreakAfterReturnType. Replaced the new ShortReturnTypeColumn option with this alternative. --- clang/docs/ClangFormatStyleOptions.rst | 38 +++++--- clang/docs/ReleaseNotes.rst | 2 +- clang/include/clang/Format/Format.h | 33 ++++--- clang/lib/Format/ContinuationIndenter.cpp | 15 +-- clang/lib/Format/Format.cpp | 3 +- clang/lib/Format/TokenAnnotator.cpp | 1 + clang/unittests/Format/ConfigParseTest.cpp | 3 +- clang/unittests/Format/FormatTest.cpp | 102 +++++++++++---------- 8 files changed, 113 insertions(+), 84 deletions(-) diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst index 7836cc8f1c9bb..9201a152b1162 100644 --- a/clang/docs/ClangFormatStyleOptions.rst +++ b/clang/docs/ClangFormatStyleOptions.rst @@ -146,7 +146,7 @@ the configuration (without a prefix: ``Auto``). .. _BasedOnStyle: -**BasedOnStyle** (``String``) :ref:`¶ <BasedOnStyle>` +**BasedOnStyle** (``String``) :ref:`¶ <BasedOnStyle>` The style used for all options not specifically set in the configuration. This option is supported only in the :program:`clang-format` configuration @@ -1547,6 +1547,23 @@ the configuration (without a prefix: ``Auto``). }; int f(); int f() { return 1; } + int foooooooooooooooooooooooooooooooooooooooooooooooo:: + baaaaaaaaaaaaaaaaaaaaar(); + + * ``RTBS_AllowShortType`` (in configuration: ``AllowShortType``) + Break after return type automatically, while allowing a break after + short return types. + ``PenaltyReturnTypeOnItsOwnLine`` is taken into account. + + .. code-block:: c++ + + class A { + int f() { return 0; }; + }; + int f(); + int f() { return 1; } + int + foooooooooooooooooooooooooooooooooooooooooooooooo::baaaaaaaaaaaaaaaaaaaaar(); * ``RTBS_All`` (in configuration: ``All``) Always break after the return type. @@ -1580,6 +1597,8 @@ the configuration (without a prefix: ``Auto``). f() { return 1; } + int + foooooooooooooooooooooooooooooooooooooooooooooooo::baaaaaaaaaaaaaaaaaaaaar(); * ``RTBS_AllDefinitions`` (in configuration: ``AllDefinitions``) Always break after the return type of function definitions. @@ -1597,6 +1616,8 @@ the configuration (without a prefix: ``Auto``). f() { return 1; } + int + foooooooooooooooooooooooooooooooooooooooooooooooo::baaaaaaaaaaaaaaaaaaaaar(); * ``RTBS_TopLevelDefinitions`` (in configuration: ``TopLevelDefinitions``) Always break after the return type of top-level definitions. @@ -1611,6 +1632,8 @@ the configuration (without a prefix: ``Auto``). f() { return 1; } + int + foooooooooooooooooooooooooooooooooooooooooooooooo::baaaaaaaaaaaaaaaaaaaaar(); @@ -4999,19 +5022,6 @@ the configuration (without a prefix: ``Auto``). int bar; int bar; } // namespace b } // namespace b -.. _ShortReturnTypeColumn: - -**ShortReturnTypeColumn** (``Unsigned``) :versionbadge:`clang-format 19` :ref:`¶ <ShortReturnTypeColumn>` - When ``AlwaysBreakAfterReturnType`` is ``None``, line breaks are prevented - after short return types. This configures the column limit for a type - to be regarded as short. - - - .. note:: - - This isn't the length of the type itself, but the column where it - finishes. I.e. it includes indentation, etc. - .. _SkipMacroDefinitionBody: **SkipMacroDefinitionBody** (``Boolean``) :versionbadge:`clang-format 18` :ref:`¶ <SkipMacroDefinitionBody>` diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 669b420fe21ec..56befd7eefb2f 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -184,7 +184,7 @@ AST Matchers clang-format ------------ -- Add ``ShortReturnTypeColumn`` option. +- Add ``AllowShortType`` option for ``AlwaysBreakAfterReturnType``. libclang -------- diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h index 7fd574c98a394..02f1c6c8b30e9 100644 --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -922,8 +922,23 @@ struct FormatStyle { /// }; /// int f(); /// int f() { return 1; } + /// int foooooooooooooooooooooooooooooooooooooooooooooooo:: + /// baaaaaaaaaaaaaaaaaaaaar(); /// \endcode RTBS_None, + /// Break after return type automatically, while allowing a break after + /// short return types. + /// ``PenaltyReturnTypeOnItsOwnLine`` is taken into account. + /// \code + /// class A { + /// int f() { return 0; }; + /// }; + /// int f(); + /// int f() { return 1; } + /// int + /// foooooooooooooooooooooooooooooooooooooooooooooooo::baaaaaaaaaaaaaaaaaaaaar(); + /// \endcode + RTBS_AllowShortType, /// Always break after the return type. /// \code /// class A { @@ -951,6 +966,8 @@ struct FormatStyle { /// f() { /// return 1; /// } + /// int + /// foooooooooooooooooooooooooooooooooooooooooooooooo::baaaaaaaaaaaaaaaaaaaaar(); /// \endcode RTBS_TopLevel, /// Always break after the return type of function definitions. @@ -966,6 +983,8 @@ struct FormatStyle { /// f() { /// return 1; /// } + /// int + /// foooooooooooooooooooooooooooooooooooooooooooooooo::baaaaaaaaaaaaaaaaaaaaar(); /// \endcode RTBS_AllDefinitions, /// Always break after the return type of top-level definitions. @@ -978,6 +997,8 @@ struct FormatStyle { /// f() { /// return 1; /// } + /// int + /// foooooooooooooooooooooooooooooooooooooooooooooooo::baaaaaaaaaaaaaaaaaaaaar(); /// \endcode RTBS_TopLevelDefinitions, }; @@ -3932,17 +3953,6 @@ struct FormatStyle { /// \version 13 unsigned ShortNamespaceLines; - /// When ``AlwaysBreakAfterReturnType`` is ``None``, line breaks are prevented - /// after short return types. This configures the column limit for a type - /// to be regarded as short. - /// - /// \note - /// This isn't the length of the type itself, but the column where it - /// finishes. I.e. it includes indentation, etc. - /// \endnote - /// \version 19 - unsigned ShortReturnTypeColumn; - /// Do not format macro definition body. /// \version 18 bool SkipMacroDefinitionBody; @@ -4910,7 +4920,6 @@ struct FormatStyle { RequiresExpressionIndentation == R.RequiresExpressionIndentation && SeparateDefinitionBlocks == R.SeparateDefinitionBlocks && ShortNamespaceLines == R.ShortNamespaceLines && - ShortReturnTypeColumn == R.ShortReturnTypeColumn && SkipMacroDefinitionBody == R.SkipMacroDefinitionBody && SortIncludes == R.SortIncludes && SortJavaStaticImport == R.SortJavaStaticImport && diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp index 3201bc0e7dc2c..ec0fc7fb9243b 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -326,12 +326,13 @@ bool ContinuationIndenter::canBreak(const LineState &State) { return false; } - // Don't break after very short return types (e.g. "void") as that is often - // unexpected. - if (Current.is(TT_FunctionDeclarationName) && - State.Column - State.FirstIndent <= Style.ShortReturnTypeColumn) { - if (Style.AlwaysBreakAfterReturnType == FormatStyle::RTBS_None) + if (Style.AlwaysBreakAfterReturnType == FormatStyle::RTBS_None) { + // Don't break after very short return types (e.g. "void") as that is often + // unexpected. + if (Current.is(TT_FunctionDeclarationName) && + State.Column - State.FirstIndent < 6) { return false; + } } // If binary operators are moved to the next line (including commas for some @@ -588,7 +589,9 @@ bool ContinuationIndenter::mustBreak(const LineState &State) { !State.Line->ReturnTypeWrapped && // Don't break before a C# function when no break after return type. (!Style.isCSharp() || - Style.AlwaysBreakAfterReturnType != FormatStyle::RTBS_None) && + (Style.AlwaysBreakAfterReturnType != FormatStyle::RTBS_None && + Style.AlwaysBreakAfterReturnType != + FormatStyle::RTBS_AllowShortType)) && // Don't always break between a JavaScript `function` and the function // name. !Style.isJavaScript() && Previous.isNot(tok::kw_template) && diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index 35478fac7b496..a1034ce7c3266 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -574,6 +574,7 @@ template <> struct ScalarEnumerationTraits<FormatStyle::ReturnTypeBreakingStyle> { static void enumeration(IO &IO, FormatStyle::ReturnTypeBreakingStyle &Value) { IO.enumCase(Value, "None", FormatStyle::RTBS_None); + IO.enumCase(Value, "AllowShortType", FormatStyle::RTBS_AllowShortType); IO.enumCase(Value, "All", FormatStyle::RTBS_All); IO.enumCase(Value, "TopLevel", FormatStyle::RTBS_TopLevel); IO.enumCase(Value, "TopLevelDefinitions", @@ -1085,7 +1086,6 @@ template <> struct MappingTraits<FormatStyle> { Style.RequiresExpressionIndentation); IO.mapOptional("SeparateDefinitionBlocks", Style.SeparateDefinitionBlocks); IO.mapOptional("ShortNamespaceLines", Style.ShortNamespaceLines); - IO.mapOptional("ShortReturnTypeColumn", Style.ShortReturnTypeColumn); IO.mapOptional("SkipMacroDefinitionBody", Style.SkipMacroDefinitionBody); IO.mapOptional("SortIncludes", Style.SortIncludes); IO.mapOptional("SortJavaStaticImport", Style.SortJavaStaticImport); @@ -1558,7 +1558,6 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) { LLVMStyle.RequiresExpressionIndentation = FormatStyle::REI_OuterScope; LLVMStyle.SeparateDefinitionBlocks = FormatStyle::SDS_Leave; LLVMStyle.ShortNamespaceLines = 1; - LLVMStyle.ShortReturnTypeColumn = 5; LLVMStyle.SkipMacroDefinitionBody = false; LLVMStyle.SortIncludes = FormatStyle::SI_CaseSensitive; LLVMStyle.SortJavaStaticImport = FormatStyle::SJSIO_Before; diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp index 25fcceb878643..81d866e8a2f47 100644 --- a/clang/lib/Format/TokenAnnotator.cpp +++ b/clang/lib/Format/TokenAnnotator.cpp @@ -3432,6 +3432,7 @@ bool TokenAnnotator::mustBreakForReturnType(const AnnotatedLine &Line) const { switch (Style.AlwaysBreakAfterReturnType) { case FormatStyle::RTBS_None: + case FormatStyle::RTBS_AllowShortType: return false; case FormatStyle::RTBS_All: case FormatStyle::RTBS_TopLevel: diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp index baf892b43320d..8b1b289508a58 100644 --- a/clang/unittests/Format/ConfigParseTest.cpp +++ b/clang/unittests/Format/ConfigParseTest.cpp @@ -261,7 +261,6 @@ TEST(ConfigParseTest, ParsesConfiguration) { CHECK_PARSE("PenaltyExcessCharacter: 1234", PenaltyExcessCharacter, 1234u); CHECK_PARSE("PenaltyReturnTypeOnItsOwnLine: 1234", PenaltyReturnTypeOnItsOwnLine, 1234u); - CHECK_PARSE("ShortReturnTypeColumn: 1234", ShortReturnTypeColumn, 1234u); CHECK_PARSE("SpacesBeforeTrailingComments: 1234", SpacesBeforeTrailingComments, 1234u); CHECK_PARSE("IndentWidth: 32", IndentWidth, 32u); @@ -698,6 +697,8 @@ TEST(ConfigParseTest, ParsesConfiguration) { Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_All; CHECK_PARSE("AlwaysBreakAfterReturnType: None", AlwaysBreakAfterReturnType, FormatStyle::RTBS_None); + CHECK_PARSE("AlwaysBreakAfterReturnType: AllowShortType", + AlwaysBreakAfterReturnType, FormatStyle::RTBS_AllowShortType); CHECK_PARSE("AlwaysBreakAfterReturnType: All", AlwaysBreakAfterReturnType, FormatStyle::RTBS_All); CHECK_PARSE("AlwaysBreakAfterReturnType: TopLevel", diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 778d98a4493b6..c2f4576751934 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -9872,9 +9872,30 @@ TEST_F(FormatTest, ReturnTypeBreakingStyle) { verifyFormat("class A {\n" " int f() { return 1; }\n" " int g();\n" + " long fooooooooooooooooooooooooooooooooooooooooooooooo::\n" + " baaaaaaaaaaaaaaaaaaaar();\n" "};\n" "int f() { return 1; }\n" - "int g();", + "int g();\n" + "int foooooooooooooooooooooooooooooooooooooooooooooooo::\n" + " baaaaaaaaaaaaaaaaaaaaar();", + Style); + + // It is now allowed to break after a short return type if necessary. + Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_AllowShortType; + verifyFormat("class A {\n" + " int f() { return 1; }\n" + " int g();\n" + " long\n" + " " + "fooooooooooooooooooooooooooooooooooooooooooooooo::" + "baaaaaaaaaaaaaaaaaaaar();\n" + "};\n" + "int f() { return 1; }\n" + "int g();\n" + "int\n" + "foooooooooooooooooooooooooooooooooooooooooooooooo::" + "baaaaaaaaaaaaaaaaaaaaar();", Style); // All declarations and definitions should have the return type moved to its @@ -9891,13 +9912,20 @@ TEST_F(FormatTest, ReturnTypeBreakingStyle) { " }\n" " int\n" " g();\n" + " long\n" + " " + "fooooooooooooooooooooooooooooooooooooooooooooooo::" + "baaaaaaaaaaaaaaaaaaaar();\n" "};\n" "int\n" "f() {\n" " return 1;\n" "}\n" "int\n" - "g();", + "g();\n" + "int\n" + "foooooooooooooooooooooooooooooooooooooooooooooooo::" + "baaaaaaaaaaaaaaaaaaaaar();", Style); // Top-level definitions, and no kinds of declarations should have the @@ -9906,12 +9934,19 @@ TEST_F(FormatTest, ReturnTypeBreakingStyle) { verifyFormat("class B {\n" " int f() { return 1; }\n" " int g();\n" + " long\n" + " " + "fooooooooooooooooooooooooooooooooooooooooooooooo::" + "baaaaaaaaaaaaaaaaaaaar();\n" "};\n" "int\n" "f() {\n" " return 1;\n" "}\n" - "int g();", + "int g();\n" + "int\n" + "foooooooooooooooooooooooooooooooooooooooooooooooo::" + "baaaaaaaaaaaaaaaaaaaaar();", Style); // Top-level definitions and declarations should have the return type moved @@ -9920,13 +9955,20 @@ TEST_F(FormatTest, ReturnTypeBreakingStyle) { verifyFormat("class C {\n" " int f() { return 1; }\n" " int g();\n" + " long\n" + " " + "fooooooooooooooooooooooooooooooooooooooooooooooo::" + "baaaaaaaaaaaaaaaaaaaar();\n" "};\n" "int\n" "f() {\n" " return 1;\n" "}\n" "int\n" - "g();", + "g();\n" + "int\n" + "foooooooooooooooooooooooooooooooooooooooooooooooo::" + "baaaaaaaaaaaaaaaaaaaaar();", Style); // All definitions should have the return type moved to its own line, but no @@ -9938,12 +9980,19 @@ TEST_F(FormatTest, ReturnTypeBreakingStyle) { " return 1;\n" " }\n" " int g();\n" + " long\n" + " " + "fooooooooooooooooooooooooooooooooooooooooooooooo::" + "baaaaaaaaaaaaaaaaaaaar();\n" "};\n" "int\n" "f() {\n" " return 1;\n" "}\n" - "int g();", + "int g();\n" + "int\n" + "foooooooooooooooooooooooooooooooooooooooooooooooo::" + "baaaaaaaaaaaaaaaaaaaaar();", Style); verifyFormat("const char *\n" "f(void) {\n" // Break here. @@ -12406,49 +12455,6 @@ TEST_F(FormatTest, BreaksLongDeclarations) { verifyFormat("template <typename T> // Templates on own line.\n" "static int // Some comment.\n" "MyFunction(int a);"); - - FormatStyle ShortReturnType = getLLVMStyle(); - verifyFormat("Type " - "Loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo" - "ooooooooong::\n" - " FunctionDeclaration();", - ShortReturnType); - verifyFormat("struct S {\n" - " Type " - "Loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo" - "oooooooooooong::\n" - " FunctionDeclaration();\n" - "}", - ShortReturnType); - - ShortReturnType.ShortReturnTypeColumn = 0; - verifyFormat("Type\n" - "Loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo" - "ooooooooong::\n" - " FunctionDeclaration();", - ShortReturnType); - verifyFormat("struct S {\n" - " Type\n" - " " - "Loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo" - "oooooooooooong::\n" - " FunctionDeclaration();\n" - "}", - ShortReturnType); - - ShortReturnType.ShortReturnTypeColumn = 7; - verifyFormat("Type " - "Loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo" - "ooooooooong::\n" - " FunctionDeclaration();", - ShortReturnType); - verifyFormat("struct S {\n" - " Type " - "Loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo" - "oooooooooooong::\n" - " FunctionDeclaration();\n" - "}", - ShortReturnType); } TEST_F(FormatTest, FormatsAccessModifiers) { >From 189cd7ab7b0a49bf678880304fe8bf583d6f6197 Mon Sep 17 00:00:00 2001 From: rmarker <rmar...@outlook.com> Date: Sat, 27 Jan 2024 14:43:30 +1030 Subject: [PATCH 04/17] Address review feedback. --- clang/docs/ClangFormatStyleOptions.rst | 2 +- clang/lib/Format/ContinuationIndenter.cpp | 5 ++- clang/unittests/Format/FormatTest.cpp | 41 ++++++++--------------- 3 files changed, 17 insertions(+), 31 deletions(-) diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst index 9201a152b1162..de3a75df9a0c4 100644 --- a/clang/docs/ClangFormatStyleOptions.rst +++ b/clang/docs/ClangFormatStyleOptions.rst @@ -146,7 +146,7 @@ the configuration (without a prefix: ``Auto``). .. _BasedOnStyle: -**BasedOnStyle** (``String``) :ref:`¶ <BasedOnStyle>` +**BasedOnStyle** (``String``) :ref:`¶ <BasedOnStyle>` The style used for all options not specifically set in the configuration. This option is supported only in the :program:`clang-format` configuration diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp index ec0fc7fb9243b..47a2bb02aabb8 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -329,6 +329,7 @@ bool ContinuationIndenter::canBreak(const LineState &State) { if (Style.AlwaysBreakAfterReturnType == FormatStyle::RTBS_None) { // Don't break after very short return types (e.g. "void") as that is often // unexpected. + assert(State.Column >= State.FirstIndent); if (Current.is(TT_FunctionDeclarationName) && State.Column - State.FirstIndent < 6) { return false; @@ -589,9 +590,7 @@ bool ContinuationIndenter::mustBreak(const LineState &State) { !State.Line->ReturnTypeWrapped && // Don't break before a C# function when no break after return type. (!Style.isCSharp() || - (Style.AlwaysBreakAfterReturnType != FormatStyle::RTBS_None && - Style.AlwaysBreakAfterReturnType != - FormatStyle::RTBS_AllowShortType)) && + Style.AlwaysBreakAfterReturnType > FormatStyle::RTBS_AllowShortType) && // Don't always break between a JavaScript `function` and the function // name. !Style.isJavaScript() && Previous.isNot(tok::kw_template) && diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index c2f4576751934..bd0fc45cee9fd 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -9867,17 +9867,19 @@ TEST_F(FormatTest, AlignsStringLiterals) { TEST_F(FormatTest, ReturnTypeBreakingStyle) { FormatStyle Style = getLLVMStyle(); + Style.ColumnLimit = 60; + // No declarations or definitions should be moved to own line. Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None; verifyFormat("class A {\n" " int f() { return 1; }\n" " int g();\n" - " long fooooooooooooooooooooooooooooooooooooooooooooooo::\n" + " long foooooooooooooooooooooooooooo::\n" " baaaaaaaaaaaaaaaaaaaar();\n" "};\n" "int f() { return 1; }\n" "int g();\n" - "int foooooooooooooooooooooooooooooooooooooooooooooooo::\n" + "int foooooooooooooooooooooooooooo::\n" " baaaaaaaaaaaaaaaaaaaaar();", Style); @@ -9887,15 +9889,12 @@ TEST_F(FormatTest, ReturnTypeBreakingStyle) { " int f() { return 1; }\n" " int g();\n" " long\n" - " " - "fooooooooooooooooooooooooooooooooooooooooooooooo::" - "baaaaaaaaaaaaaaaaaaaar();\n" + " foooooooooooooooooooooooooooo::baaaaaaaaaaaaaaaaaaaar();\n" "};\n" "int f() { return 1; }\n" "int g();\n" "int\n" - "foooooooooooooooooooooooooooooooooooooooooooooooo::" - "baaaaaaaaaaaaaaaaaaaaar();", + "foooooooooooooooooooooooooooo::baaaaaaaaaaaaaaaaaaaaar();", Style); // All declarations and definitions should have the return type moved to its @@ -9913,9 +9912,7 @@ TEST_F(FormatTest, ReturnTypeBreakingStyle) { " int\n" " g();\n" " long\n" - " " - "fooooooooooooooooooooooooooooooooooooooooooooooo::" - "baaaaaaaaaaaaaaaaaaaar();\n" + " foooooooooooooooooooooooooooo::baaaaaaaaaaaaaaaaaaaar();\n" "};\n" "int\n" "f() {\n" @@ -9924,8 +9921,7 @@ TEST_F(FormatTest, ReturnTypeBreakingStyle) { "int\n" "g();\n" "int\n" - "foooooooooooooooooooooooooooooooooooooooooooooooo::" - "baaaaaaaaaaaaaaaaaaaaar();", + "foooooooooooooooooooooooooooo::baaaaaaaaaaaaaaaaaaaaar();", Style); // Top-level definitions, and no kinds of declarations should have the @@ -9935,9 +9931,7 @@ TEST_F(FormatTest, ReturnTypeBreakingStyle) { " int f() { return 1; }\n" " int g();\n" " long\n" - " " - "fooooooooooooooooooooooooooooooooooooooooooooooo::" - "baaaaaaaaaaaaaaaaaaaar();\n" + " foooooooooooooooooooooooooooo::baaaaaaaaaaaaaaaaaaaar();\n" "};\n" "int\n" "f() {\n" @@ -9945,8 +9939,7 @@ TEST_F(FormatTest, ReturnTypeBreakingStyle) { "}\n" "int g();\n" "int\n" - "foooooooooooooooooooooooooooooooooooooooooooooooo::" - "baaaaaaaaaaaaaaaaaaaaar();", + "foooooooooooooooooooooooooooo::baaaaaaaaaaaaaaaaaaaaar();", Style); // Top-level definitions and declarations should have the return type moved @@ -9956,9 +9949,7 @@ TEST_F(FormatTest, ReturnTypeBreakingStyle) { " int f() { return 1; }\n" " int g();\n" " long\n" - " " - "fooooooooooooooooooooooooooooooooooooooooooooooo::" - "baaaaaaaaaaaaaaaaaaaar();\n" + " foooooooooooooooooooooooooooo::baaaaaaaaaaaaaaaaaaaar();\n" "};\n" "int\n" "f() {\n" @@ -9967,8 +9958,7 @@ TEST_F(FormatTest, ReturnTypeBreakingStyle) { "int\n" "g();\n" "int\n" - "foooooooooooooooooooooooooooooooooooooooooooooooo::" - "baaaaaaaaaaaaaaaaaaaaar();", + "foooooooooooooooooooooooooooo::baaaaaaaaaaaaaaaaaaaaar();", Style); // All definitions should have the return type moved to its own line, but no @@ -9981,9 +9971,7 @@ TEST_F(FormatTest, ReturnTypeBreakingStyle) { " }\n" " int g();\n" " long\n" - " " - "fooooooooooooooooooooooooooooooooooooooooooooooo::" - "baaaaaaaaaaaaaaaaaaaar();\n" + " foooooooooooooooooooooooooooo::baaaaaaaaaaaaaaaaaaaar();\n" "};\n" "int\n" "f() {\n" @@ -9991,8 +9979,7 @@ TEST_F(FormatTest, ReturnTypeBreakingStyle) { "}\n" "int g();\n" "int\n" - "foooooooooooooooooooooooooooooooooooooooooooooooo::" - "baaaaaaaaaaaaaaaaaaaaar();", + "foooooooooooooooooooooooooooo::baaaaaaaaaaaaaaaaaaaaar();", Style); verifyFormat("const char *\n" "f(void) {\n" // Break here. >From 285be69c1ebc2fd92659d45e86330ab19a416cc7 Mon Sep 17 00:00:00 2001 From: rmarker <rmar...@outlook.com> Date: Sat, 27 Jan 2024 14:49:44 +1030 Subject: [PATCH 05/17] Read the contents with encoding utf-8 when generating the format style. On Windows it adding spurious characters to the output. --- clang/docs/tools/dump_format_style.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/docs/tools/dump_format_style.py b/clang/docs/tools/dump_format_style.py index 75d4a044ef19f..e41891f07de2e 100755 --- a/clang/docs/tools/dump_format_style.py +++ b/clang/docs/tools/dump_format_style.py @@ -474,7 +474,7 @@ class State: opts = sorted(opts, key=lambda x: x.name) options_text = "\n\n".join(map(str, opts)) -with open(DOC_FILE) as f: +with open(DOC_FILE, encoding="utf-8") as f: contents = f.read() contents = substitute(contents, "FORMAT_STYLE_OPTIONS", options_text) >From 0fb332e1d8019ccf184b473e57a9762fdccaf5a0 Mon Sep 17 00:00:00 2001 From: rmarker <rmar...@outlook.com> Date: Sat, 27 Jan 2024 23:15:07 +1030 Subject: [PATCH 06/17] Switch LLVMStyle.AlwaysBreakAfterReturnType to RTBS_AllowShortType. --- clang/lib/Format/Format.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index a1034ce7c3266..404307e70fe7f 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -1454,7 +1454,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) { LLVMStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never; LLVMStyle.AllowShortLambdasOnASingleLine = FormatStyle::SLS_All; LLVMStyle.AllowShortLoopsOnASingleLine = false; - LLVMStyle.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None; + LLVMStyle.AlwaysBreakAfterReturnType = FormatStyle::RTBS_AllowShortType; LLVMStyle.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None; LLVMStyle.AlwaysBreakBeforeMultilineStrings = false; LLVMStyle.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_MultiLine; >From 01fd15c864b01218d36f1acd86da8476cddf97c8 Mon Sep 17 00:00:00 2001 From: rmarker <rmar...@outlook.com> Date: Sat, 27 Jan 2024 23:31:21 +1030 Subject: [PATCH 07/17] Update formatting in polly to pass polly-check-format. --- polly/include/polly/DependenceInfo.h | 4 ++-- polly/include/polly/LinkAllPasses.h | 4 ++-- polly/lib/Analysis/DependenceInfo.cpp | 5 +++-- polly/lib/Analysis/ScopBuilder.cpp | 10 ++++++---- polly/lib/Analysis/ScopDetection.cpp | 11 ++++++----- polly/lib/CodeGen/IslNodeBuilder.cpp | 9 +++++---- polly/lib/Transform/MaximalStaticExpansion.cpp | 4 ++-- polly/lib/Transform/ScheduleOptimizer.cpp | 4 ++-- 8 files changed, 28 insertions(+), 23 deletions(-) diff --git a/polly/include/polly/DependenceInfo.h b/polly/include/polly/DependenceInfo.h index 7526a294c6baf..fb7b1740ea253 100644 --- a/polly/include/polly/DependenceInfo.h +++ b/polly/include/polly/DependenceInfo.h @@ -332,8 +332,8 @@ namespace llvm { void initializeDependenceInfoPass(llvm::PassRegistry &); void initializeDependenceInfoPrinterLegacyPassPass(llvm::PassRegistry &); void initializeDependenceInfoWrapperPassPass(llvm::PassRegistry &); -void initializeDependenceInfoPrinterLegacyFunctionPassPass( - llvm::PassRegistry &); +void +initializeDependenceInfoPrinterLegacyFunctionPassPass(llvm::PassRegistry &); } // namespace llvm #endif diff --git a/polly/include/polly/LinkAllPasses.h b/polly/include/polly/LinkAllPasses.h index a2f8f33299918..a293b7074f3bd 100644 --- a/polly/include/polly/LinkAllPasses.h +++ b/polly/include/polly/LinkAllPasses.h @@ -138,8 +138,8 @@ void initializeJSONImporterPrinterLegacyPassPass(llvm::PassRegistry &); void initializeDependenceInfoPass(llvm::PassRegistry &); void initializeDependenceInfoPrinterLegacyPassPass(llvm::PassRegistry &); void initializeDependenceInfoWrapperPassPass(llvm::PassRegistry &); -void initializeDependenceInfoPrinterLegacyFunctionPassPass( - llvm::PassRegistry &); +void +initializeDependenceInfoPrinterLegacyFunctionPassPass(llvm::PassRegistry &); void initializeIslAstInfoWrapperPassPass(llvm::PassRegistry &); void initializeIslAstInfoPrinterLegacyPassPass(llvm::PassRegistry &); void initializeCodeGenerationPass(llvm::PassRegistry &); diff --git a/polly/lib/Analysis/DependenceInfo.cpp b/polly/lib/Analysis/DependenceInfo.cpp index 69257c603877e..3da6738b64584 100644 --- a/polly/lib/Analysis/DependenceInfo.cpp +++ b/polly/lib/Analysis/DependenceInfo.cpp @@ -647,8 +647,9 @@ bool Dependences::isValidSchedule(Scop &S, isl::schedule NewSched) const { return isValidSchedule(S, NewSchedules); } -bool Dependences::isValidSchedule( - Scop &S, const StatementToIslMapTy &NewSchedule) const { +bool +Dependences::isValidSchedule(Scop &S, + const StatementToIslMapTy &NewSchedule) const { if (LegalityCheckDisabled) return true; diff --git a/polly/lib/Analysis/ScopBuilder.cpp b/polly/lib/Analysis/ScopBuilder.cpp index c62cb2a85c835..c14b01116ec40 100644 --- a/polly/lib/Analysis/ScopBuilder.cpp +++ b/polly/lib/Analysis/ScopBuilder.cpp @@ -827,8 +827,9 @@ void ScopBuilder::buildInvariantEquivalenceClasses() { } } -bool ScopBuilder::buildDomains( - Region *R, DenseMap<BasicBlock *, isl::set> &InvalidDomainMap) { +bool +ScopBuilder::buildDomains(Region *R, + DenseMap<BasicBlock *, isl::set> &InvalidDomainMap) { bool IsOnlyNonAffineRegion = scop->isNonAffineSubRegion(R); auto *EntryBB = R->getEntry(); auto *L = IsOnlyNonAffineRegion ? nullptr : LI.getLoopFor(EntryBB); @@ -3297,8 +3298,9 @@ bool ScopBuilder::buildAliasGroups() { return true; } -bool ScopBuilder::buildAliasGroup( - AliasGroupTy &AliasGroup, DenseSet<const ScopArrayInfo *> HasWriteAccess) { +bool +ScopBuilder::buildAliasGroup(AliasGroupTy &AliasGroup, + DenseSet<const ScopArrayInfo *> HasWriteAccess) { AliasGroupTy ReadOnlyAccesses; AliasGroupTy ReadWriteAccesses; SmallPtrSet<const ScopArrayInfo *, 4> ReadWriteArrays; diff --git a/polly/lib/Analysis/ScopDetection.cpp b/polly/lib/Analysis/ScopDetection.cpp index 938d3f149677b..8af4986dc1bbe 100644 --- a/polly/lib/Analysis/ScopDetection.cpp +++ b/polly/lib/Analysis/ScopDetection.cpp @@ -978,9 +978,10 @@ bool ScopDetection::hasValidArraySizes(DetectionContext &Context, // non-affine accesses are allowed, we drop the information. In case the // information is dropped the memory accesses need to be overapproximated // when translated to a polyhedral representation. -bool ScopDetection::computeAccessFunctions( - DetectionContext &Context, const SCEVUnknown *BasePointer, - std::shared_ptr<ArrayShape> Shape) const { +bool +ScopDetection::computeAccessFunctions(DetectionContext &Context, + const SCEVUnknown *BasePointer, + std::shared_ptr<ArrayShape> Shape) const { Value *BaseValue = BasePointer->getValue(); bool BasePtrHasNonAffine = false; MapInsnToMemAcc TempMemoryAccesses; @@ -1691,8 +1692,8 @@ bool ScopDetection::hasSufficientCompute(DetectionContext &Context, return InstCount >= ProfitabilityMinPerLoopInstructions; } -bool ScopDetection::hasPossiblyDistributableLoop( - DetectionContext &Context) const { +bool +ScopDetection::hasPossiblyDistributableLoop(DetectionContext &Context) const { for (auto *BB : Context.CurRegion.blocks()) { auto *L = LI.getLoopFor(BB); if (!Context.CurRegion.contains(L)) diff --git a/polly/lib/CodeGen/IslNodeBuilder.cpp b/polly/lib/CodeGen/IslNodeBuilder.cpp index a226cc2a1b250..885b5ee9a1fae 100644 --- a/polly/lib/CodeGen/IslNodeBuilder.cpp +++ b/polly/lib/CodeGen/IslNodeBuilder.cpp @@ -831,8 +831,9 @@ void IslNodeBuilder::createSubstitutionsVector( isl_ast_expr_free(Expr); } -void IslNodeBuilder::generateCopyStmt( - ScopStmt *Stmt, __isl_keep isl_id_to_ast_expr *NewAccesses) { +void +IslNodeBuilder::generateCopyStmt(ScopStmt *Stmt, + __isl_keep isl_id_to_ast_expr *NewAccesses) { assert(Stmt->size() == 2); auto ReadAccess = Stmt->begin(); auto WriteAccess = ReadAccess++; @@ -1129,8 +1130,8 @@ Value *IslNodeBuilder::preloadInvariantLoad(const MemoryAccess &MA, return PreloadVal; } -bool IslNodeBuilder::preloadInvariantEquivClass( - InvariantEquivClassTy &IAClass) { +bool +IslNodeBuilder::preloadInvariantEquivClass(InvariantEquivClassTy &IAClass) { // For an equivalence class of invariant loads we pre-load the representing // element with the unified execution context. However, we have to map all // elements of the class to the one preloaded load as they are referenced diff --git a/polly/lib/Transform/MaximalStaticExpansion.cpp b/polly/lib/Transform/MaximalStaticExpansion.cpp index e32a69d47f69c..de9b9bbc2cf5c 100644 --- a/polly/lib/Transform/MaximalStaticExpansion.cpp +++ b/polly/lib/Transform/MaximalStaticExpansion.cpp @@ -535,8 +535,8 @@ void MaximalStaticExpanderWrapperPass::printScop(raw_ostream &OS, S.print(OS, false); } -void MaximalStaticExpanderWrapperPass::getAnalysisUsage( - AnalysisUsage &AU) const { +void +MaximalStaticExpanderWrapperPass::getAnalysisUsage(AnalysisUsage &AU) const { ScopPass::getAnalysisUsage(AU); AU.addRequired<DependenceInfo>(); AU.addRequired<OptimizationRemarkEmitterWrapperPass>(); diff --git a/polly/lib/Transform/ScheduleOptimizer.cpp b/polly/lib/Transform/ScheduleOptimizer.cpp index 8ee2b66339adb..751b073b7f24f 100644 --- a/polly/lib/Transform/ScheduleOptimizer.cpp +++ b/polly/lib/Transform/ScheduleOptimizer.cpp @@ -990,8 +990,8 @@ void IslScheduleOptimizerWrapperPass::printScop(raw_ostream &OS, Scop &) const { runScheduleOptimizerPrinter(OS, LastSchedule); } -void IslScheduleOptimizerWrapperPass::getAnalysisUsage( - AnalysisUsage &AU) const { +void +IslScheduleOptimizerWrapperPass::getAnalysisUsage(AnalysisUsage &AU) const { ScopPass::getAnalysisUsage(AU); AU.addRequired<DependenceInfo>(); AU.addRequired<TargetTransformInfoWrapperPass>(); >From 64caa360077385479e7ad3791dc500674ab2f17a Mon Sep 17 00:00:00 2001 From: rmarker <rmar...@outlook.com> Date: Sun, 28 Jan 2024 09:33:21 +1030 Subject: [PATCH 08/17] Update tests for change to llvm style. --- clang/unittests/Format/FormatTest.cpp | 47 +++++++++++++------ clang/unittests/Format/FormatTestComments.cpp | 18 +++---- .../unittests/Format/FormatTestSelective.cpp | 3 +- 3 files changed, 45 insertions(+), 23 deletions(-) diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index bd0fc45cee9fd..b50ceae1965bd 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -7841,6 +7841,7 @@ TEST_F(FormatTest, AllowAllConstructorInitializersOnNextLine) { TEST_F(FormatTest, AllowAllArgumentsOnNextLine) { FormatStyle Style = getLLVMStyleWithColumns(60); Style.BinPackArguments = false; + Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None; for (int i = 0; i < 4; ++i) { // Test all combinations of parameters that should not have an effect. Style.AllowAllParametersOfDeclarationOnNextLine = i & 1; @@ -7898,6 +7899,7 @@ TEST_F(FormatTest, AllowAllArgumentsOnNextLineDontAlign) { "void functionDecl(int A, int B, int C);"; Style.AllowAllArgumentsOnNextLine = false; Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign; + Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None; verifyFormat(StringRef("functionCall(paramA, paramB,\n" " paramC);\n" "void functionDecl(int A, int B,\n" @@ -8378,10 +8380,11 @@ TEST_F(FormatTest, BreaksFunctionDeclarations) { " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,\n" " bbbb bbbb);"); - verifyFormat("void SomeLoooooooooooongFunction(\n" - " std::unique_ptr<aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa>\n" - " aaaaaaaaaaaaaaaaaaaaaaaaaa,\n" - " int bbbbbbbbbbbbb);"); + verifyFormat("void\n" + "SomeLoooooooooooongFunction(std::unique_ptr<" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa>\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaa,\n" + " int bbbbbbbbbbbbb);"); // Treat overloaded operators like other functions. verifyFormat("SomeLoooooooooooooooooooooooooogType\n" @@ -8446,11 +8449,15 @@ TEST_F(FormatTest, TrailingReturnType) { verifyFormat("auto SomeFunction(A aaaaaaaaaaaaaaaaaaaaa) const\n" " -> decltype(f(aaaaaaaaaaaaaaaaaaaaa)) {}"); verifyFormat("auto doSomething(Aaaaaa *aaaaaa) -> decltype(aaaaaa->f()) {}"); + + FormatStyle Style = getLLVMStyle(); + Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None; verifyFormat("template <typename T>\n" "auto aaaaaaaaaaaaaaaaaaaaaa(T t)\n" - " -> decltype(eaaaaaaaaaaaaaaa<T>(t.a).aaaaaaaa());"); + " -> decltype(eaaaaaaaaaaaaaaa<T>(t.a).aaaaaaaa());", + Style); - FormatStyle Style = getLLVMStyleWithColumns(60); + Style = getLLVMStyleWithColumns(60); verifyFormat("#define MAKE_DEF(NAME) \\\n" " auto NAME() -> int { return 42; }", Style); @@ -8717,6 +8724,7 @@ TEST_F(FormatTest, BreaksDesireably) { TEST_F(FormatTest, FormatsDeclarationsOnePerLine) { FormatStyle NoBinPacking = getGoogleStyle(); + NoBinPacking.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None; NoBinPacking.BinPackParameters = false; NoBinPacking.BinPackArguments = true; verifyFormat("void f() {\n" @@ -8741,10 +8749,13 @@ TEST_F(FormatTest, FormatsDeclarationsOnePerLine) { "void fffffffffff(aaaaaaaaaaaaaaaaaaaaaaaaaaa<aaaaaaaaaaaaaaaaaaaaaaa,\n" " aaaaaaaaaa> aaaaaaaaaa);", NoBinPacking); + FormatStyle BinPacking = getLLVMStyle(); + BinPacking.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None; verifyFormat( "void fffffffffff(\n" " aaaaaaaaaaaaaaaaaaaaaaaaaaa<aaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaa>\n" - " aaaaaaaaaa);"); + " aaaaaaaaaa);", + BinPacking); } TEST_F(FormatTest, FormatsOneParameterPerLineIfNecessary) { @@ -12054,6 +12065,7 @@ TEST_F(FormatTest, AttributesAfterMacro) { TEST_F(FormatTest, AttributePenaltyBreaking) { FormatStyle Style = getLLVMStyle(); + Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None; verifyFormat("void ABCDEFGH::ABCDEFGHIJKLMN(\n" " [[maybe_unused]] const shared_ptr<ALongTypeName> &C d) {}", Style); @@ -12431,13 +12443,17 @@ TEST_F(FormatTest, BreaksLongDeclarations) { verifyFormat("typedef size_t (*aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa)(\n" " const aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" " *aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);"); - verifyFormat("void aaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n" - " vector<aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa>\n" - " aaaaaaaaaaaaaaaaaaaaaaaa);"); - verifyFormat("void aaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n" - " vector<aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa<\n" - " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa>>\n" - " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);"); + verifyFormat("void\n" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaa(vector<" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa>\n" + " aaaaaaaaaaaaaaaaaaaaaaaa);"); + verifyFormat( + "void\n" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaa(vector<" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa<\n" + " " + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa>>\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);"); verifyFormat("template <typename T> // Templates on own line.\n" "static int // Some comment.\n" @@ -14366,6 +14382,7 @@ TEST_F(FormatTest, SplitEmptyFunction) { TEST_F(FormatTest, SplitEmptyFunctionButNotRecord) { FormatStyle Style = getLLVMStyleWithColumns(40); Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None; + Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None; Style.BreakBeforeBraces = FormatStyle::BS_Custom; Style.BraceWrapping.AfterFunction = true; Style.BraceWrapping.SplitEmptyFunction = true; @@ -21573,6 +21590,7 @@ TEST_F(FormatTest, OptimizeBreakPenaltyVsExcess) { TEST_F(FormatTest, BreakPenaltyAfterLParen) { FormatStyle Style = getLLVMStyle(); Style.ColumnLimit = 8; + Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None; Style.PenaltyExcessCharacter = 15; verifyFormat("int foo(\n" " int aaaaaaaaaaaaaaaaaaaaaaaa);", @@ -26994,6 +27012,7 @@ TEST_F(FormatTest, RemoveParentheses) { TEST_F(FormatTest, AllowBreakBeforeNoexceptSpecifier) { auto Style = getLLVMStyleWithColumns(35); + Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None; EXPECT_EQ(Style.AllowBreakBeforeNoexceptSpecifier, FormatStyle::BBNSS_Never); verifyFormat("void foo(int arg1,\n" diff --git a/clang/unittests/Format/FormatTestComments.cpp b/clang/unittests/Format/FormatTestComments.cpp index c249f4d9333fd..eb9c5f22439e7 100644 --- a/clang/unittests/Format/FormatTestComments.cpp +++ b/clang/unittests/Format/FormatTestComments.cpp @@ -519,10 +519,11 @@ TEST_F(FormatTestComments, CorrectlyHandlesLengthOfBlockComments) { } TEST_F(FormatTestComments, DontBreakNonTrailingBlockComments) { + FormatStyle Style = getLLVMStyleWithColumns(35); + Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None; EXPECT_EQ("void ffffffffff(\n" " int aaaaa /* test */);", - format("void ffffffffff(int aaaaa /* test */);", - getLLVMStyleWithColumns(35))); + format("void ffffffffff(int aaaaa /* test */);", Style)); } TEST_F(FormatTestComments, SplitsLongCxxComments) { @@ -604,11 +605,11 @@ TEST_F(FormatTestComments, SplitsLongCxxComments) { format("// A comment before a macro definition\n" "#define a b", getLLVMStyleWithColumns(20))); - EXPECT_EQ("void ffffff(\n" - " int aaaaaaaaa, // wwww\n" - " int bbbbbbbbbb, // xxxxxxx\n" - " // yyyyyyyyyy\n" - " int c, int d, int e) {}", + EXPECT_EQ("void\n" + "ffffff(int aaaaaaaaa, // wwww\n" + " int bbbbbbbbbb, // xxxxxxx\n" + " // yyyyyyyyyy\n" + " int c, int d, int e) {}", format("void ffffff(\n" " int aaaaaaaaa, // wwww\n" " int bbbbbbbbbb, // xxxxxxx yyyyyyyyyy\n" @@ -4100,6 +4101,7 @@ TEST_F(FormatTestComments, SpaceAtLineCommentBegin) { format(Code, Style)); Style = getLLVMStyleWithColumns(20); + Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None; StringRef WrapCode = "//Lorem ipsum dolor sit amet\n" "\n" "// Lorem ipsum dolor sit amet\n" @@ -4583,7 +4585,7 @@ TEST_F(FormatTestComments, SplitCommentIntroducers) { )", format(R"(// /\ -/ +/ )", getLLVMStyleWithColumns(10))); } diff --git a/clang/unittests/Format/FormatTestSelective.cpp b/clang/unittests/Format/FormatTestSelective.cpp index 43de93d219b14..c21c9bfe60790 100644 --- a/clang/unittests/Format/FormatTestSelective.cpp +++ b/clang/unittests/Format/FormatTestSelective.cpp @@ -522,7 +522,8 @@ TEST_F(FormatTestSelective, ReformatRegionAdjustsIndent) { Style.ColumnLimit = 11; EXPECT_EQ(" int a;\n" - " void ffffff() {\n" + " void\n" + " ffffff() {\n" " }", format(" int a;\n" "void ffffff() {}", >From 76dddac14f4b414edf393101f8907ea2efce8a80 Mon Sep 17 00:00:00 2001 From: rmarker <rmar...@outlook.com> Date: Mon, 29 Jan 2024 21:05:02 +1030 Subject: [PATCH 09/17] Revert "Read the contents with encoding utf-8 when generating the format style." This reverts commit 285be69c1ebc2fd92659d45e86330ab19a416cc7. --- clang/docs/tools/dump_format_style.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/docs/tools/dump_format_style.py b/clang/docs/tools/dump_format_style.py index e41891f07de2e..75d4a044ef19f 100755 --- a/clang/docs/tools/dump_format_style.py +++ b/clang/docs/tools/dump_format_style.py @@ -474,7 +474,7 @@ class State: opts = sorted(opts, key=lambda x: x.name) options_text = "\n\n".join(map(str, opts)) -with open(DOC_FILE, encoding="utf-8") as f: +with open(DOC_FILE) as f: contents = f.read() contents = substitute(contents, "FORMAT_STYLE_OPTIONS", options_text) >From a31a99f64027b896071c8afcca21b93ab4ac5add Mon Sep 17 00:00:00 2001 From: rmarker <rmar...@outlook.com> Date: Mon, 29 Jan 2024 21:51:59 +1030 Subject: [PATCH 10/17] Improve documentation. --- clang/docs/ClangFormatStyleOptions.rst | 21 ++++++++++++--------- clang/include/clang/Format/Format.h | 21 ++++++++++++--------- 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst index de3a75df9a0c4..b60d91c82cdf9 100644 --- a/clang/docs/ClangFormatStyleOptions.rst +++ b/clang/docs/ClangFormatStyleOptions.rst @@ -1547,13 +1547,14 @@ the configuration (without a prefix: ``Auto``). }; int f(); int f() { return 1; } - int foooooooooooooooooooooooooooooooooooooooooooooooo:: - baaaaaaaaaaaaaaaaaaaaar(); + int LongName:: + AnotherLongName(); * ``RTBS_AllowShortType`` (in configuration: ``AllowShortType``) - Break after return type automatically, while allowing a break after - short return types. - ``PenaltyReturnTypeOnItsOwnLine`` is taken into account. + Break after return type automatically. + This mode doesn't have the same inherent restriction on breaking after + short return types as ``RTBS_None`` and is solely based on + ``PenaltyReturnTypeOnItsOwnLine``. .. code-block:: c++ @@ -1563,7 +1564,7 @@ the configuration (without a prefix: ``Auto``). int f(); int f() { return 1; } int - foooooooooooooooooooooooooooooooooooooooooooooooo::baaaaaaaaaaaaaaaaaaaaar(); + LongName::AnotherLongName(); * ``RTBS_All`` (in configuration: ``All``) Always break after the return type. @@ -1582,6 +1583,8 @@ the configuration (without a prefix: ``Auto``). f() { return 1; } + int + LongName::AnotherLongName(); * ``RTBS_TopLevel`` (in configuration: ``TopLevel``) Always break after the return types of top-level functions. @@ -1598,7 +1601,7 @@ the configuration (without a prefix: ``Auto``). return 1; } int - foooooooooooooooooooooooooooooooooooooooooooooooo::baaaaaaaaaaaaaaaaaaaaar(); + LongName::AnotherLongName(); * ``RTBS_AllDefinitions`` (in configuration: ``AllDefinitions``) Always break after the return type of function definitions. @@ -1617,7 +1620,7 @@ the configuration (without a prefix: ``Auto``). return 1; } int - foooooooooooooooooooooooooooooooooooooooooooooooo::baaaaaaaaaaaaaaaaaaaaar(); + LongName::AnotherLongName(); * ``RTBS_TopLevelDefinitions`` (in configuration: ``TopLevelDefinitions``) Always break after the return type of top-level definitions. @@ -1633,7 +1636,7 @@ the configuration (without a prefix: ``Auto``). return 1; } int - foooooooooooooooooooooooooooooooooooooooooooooooo::baaaaaaaaaaaaaaaaaaaaar(); + LongName::AnotherLongName(); diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h index 02f1c6c8b30e9..4ede7c19c4471 100644 --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -922,13 +922,14 @@ struct FormatStyle { /// }; /// int f(); /// int f() { return 1; } - /// int foooooooooooooooooooooooooooooooooooooooooooooooo:: - /// baaaaaaaaaaaaaaaaaaaaar(); + /// int LongName:: + /// AnotherLongName(); /// \endcode RTBS_None, - /// Break after return type automatically, while allowing a break after - /// short return types. - /// ``PenaltyReturnTypeOnItsOwnLine`` is taken into account. + /// Break after return type automatically. + /// This mode doesn't have the same inherent restriction on breaking after + /// short return types as ``RTBS_None`` and is solely based on + /// ``PenaltyReturnTypeOnItsOwnLine``. /// \code /// class A { /// int f() { return 0; }; @@ -936,7 +937,7 @@ struct FormatStyle { /// int f(); /// int f() { return 1; } /// int - /// foooooooooooooooooooooooooooooooooooooooooooooooo::baaaaaaaaaaaaaaaaaaaaar(); + /// LongName::AnotherLongName(); /// \endcode RTBS_AllowShortType, /// Always break after the return type. @@ -953,6 +954,8 @@ struct FormatStyle { /// f() { /// return 1; /// } + /// int + /// LongName::AnotherLongName(); /// \endcode RTBS_All, /// Always break after the return types of top-level functions. @@ -967,7 +970,7 @@ struct FormatStyle { /// return 1; /// } /// int - /// foooooooooooooooooooooooooooooooooooooooooooooooo::baaaaaaaaaaaaaaaaaaaaar(); + /// LongName::AnotherLongName(); /// \endcode RTBS_TopLevel, /// Always break after the return type of function definitions. @@ -984,7 +987,7 @@ struct FormatStyle { /// return 1; /// } /// int - /// foooooooooooooooooooooooooooooooooooooooooooooooo::baaaaaaaaaaaaaaaaaaaaar(); + /// LongName::AnotherLongName(); /// \endcode RTBS_AllDefinitions, /// Always break after the return type of top-level definitions. @@ -998,7 +1001,7 @@ struct FormatStyle { /// return 1; /// } /// int - /// foooooooooooooooooooooooooooooooooooooooooooooooo::baaaaaaaaaaaaaaaaaaaaar(); + /// LongName::AnotherLongName(); /// \endcode RTBS_TopLevelDefinitions, }; >From 28dcfbd927a61b27825b22da47e937581f72c98f Mon Sep 17 00:00:00 2001 From: rmarker <rmar...@outlook.com> Date: Tue, 30 Jan 2024 21:16:54 +1030 Subject: [PATCH 11/17] Revert "Update tests for change to llvm style." This reverts commit 64caa360077385479e7ad3791dc500674ab2f17a. --- clang/unittests/Format/FormatTest.cpp | 47 ++++++------------- clang/unittests/Format/FormatTestComments.cpp | 18 ++++--- .../unittests/Format/FormatTestSelective.cpp | 3 +- 3 files changed, 23 insertions(+), 45 deletions(-) diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index b50ceae1965bd..bd0fc45cee9fd 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -7841,7 +7841,6 @@ TEST_F(FormatTest, AllowAllConstructorInitializersOnNextLine) { TEST_F(FormatTest, AllowAllArgumentsOnNextLine) { FormatStyle Style = getLLVMStyleWithColumns(60); Style.BinPackArguments = false; - Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None; for (int i = 0; i < 4; ++i) { // Test all combinations of parameters that should not have an effect. Style.AllowAllParametersOfDeclarationOnNextLine = i & 1; @@ -7899,7 +7898,6 @@ TEST_F(FormatTest, AllowAllArgumentsOnNextLineDontAlign) { "void functionDecl(int A, int B, int C);"; Style.AllowAllArgumentsOnNextLine = false; Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign; - Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None; verifyFormat(StringRef("functionCall(paramA, paramB,\n" " paramC);\n" "void functionDecl(int A, int B,\n" @@ -8380,11 +8378,10 @@ TEST_F(FormatTest, BreaksFunctionDeclarations) { " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,\n" " bbbb bbbb);"); - verifyFormat("void\n" - "SomeLoooooooooooongFunction(std::unique_ptr<" - "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa>\n" - " aaaaaaaaaaaaaaaaaaaaaaaaaa,\n" - " int bbbbbbbbbbbbb);"); + verifyFormat("void SomeLoooooooooooongFunction(\n" + " std::unique_ptr<aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa>\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaa,\n" + " int bbbbbbbbbbbbb);"); // Treat overloaded operators like other functions. verifyFormat("SomeLoooooooooooooooooooooooooogType\n" @@ -8449,15 +8446,11 @@ TEST_F(FormatTest, TrailingReturnType) { verifyFormat("auto SomeFunction(A aaaaaaaaaaaaaaaaaaaaa) const\n" " -> decltype(f(aaaaaaaaaaaaaaaaaaaaa)) {}"); verifyFormat("auto doSomething(Aaaaaa *aaaaaa) -> decltype(aaaaaa->f()) {}"); - - FormatStyle Style = getLLVMStyle(); - Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None; verifyFormat("template <typename T>\n" "auto aaaaaaaaaaaaaaaaaaaaaa(T t)\n" - " -> decltype(eaaaaaaaaaaaaaaa<T>(t.a).aaaaaaaa());", - Style); + " -> decltype(eaaaaaaaaaaaaaaa<T>(t.a).aaaaaaaa());"); - Style = getLLVMStyleWithColumns(60); + FormatStyle Style = getLLVMStyleWithColumns(60); verifyFormat("#define MAKE_DEF(NAME) \\\n" " auto NAME() -> int { return 42; }", Style); @@ -8724,7 +8717,6 @@ TEST_F(FormatTest, BreaksDesireably) { TEST_F(FormatTest, FormatsDeclarationsOnePerLine) { FormatStyle NoBinPacking = getGoogleStyle(); - NoBinPacking.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None; NoBinPacking.BinPackParameters = false; NoBinPacking.BinPackArguments = true; verifyFormat("void f() {\n" @@ -8749,13 +8741,10 @@ TEST_F(FormatTest, FormatsDeclarationsOnePerLine) { "void fffffffffff(aaaaaaaaaaaaaaaaaaaaaaaaaaa<aaaaaaaaaaaaaaaaaaaaaaa,\n" " aaaaaaaaaa> aaaaaaaaaa);", NoBinPacking); - FormatStyle BinPacking = getLLVMStyle(); - BinPacking.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None; verifyFormat( "void fffffffffff(\n" " aaaaaaaaaaaaaaaaaaaaaaaaaaa<aaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaa>\n" - " aaaaaaaaaa);", - BinPacking); + " aaaaaaaaaa);"); } TEST_F(FormatTest, FormatsOneParameterPerLineIfNecessary) { @@ -12065,7 +12054,6 @@ TEST_F(FormatTest, AttributesAfterMacro) { TEST_F(FormatTest, AttributePenaltyBreaking) { FormatStyle Style = getLLVMStyle(); - Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None; verifyFormat("void ABCDEFGH::ABCDEFGHIJKLMN(\n" " [[maybe_unused]] const shared_ptr<ALongTypeName> &C d) {}", Style); @@ -12443,17 +12431,13 @@ TEST_F(FormatTest, BreaksLongDeclarations) { verifyFormat("typedef size_t (*aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa)(\n" " const aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" " *aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);"); - verifyFormat("void\n" - "aaaaaaaaaaaaaaaaaaaaaaaaaaaaa(vector<" - "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa>\n" - " aaaaaaaaaaaaaaaaaaaaaaaa);"); - verifyFormat( - "void\n" - "aaaaaaaaaaaaaaaaaaaaaaaaaaaaa(vector<" - "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa<\n" - " " - "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa>>\n" - " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);"); + verifyFormat("void aaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n" + " vector<aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa>\n" + " aaaaaaaaaaaaaaaaaaaaaaaa);"); + verifyFormat("void aaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n" + " vector<aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa<\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa>>\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);"); verifyFormat("template <typename T> // Templates on own line.\n" "static int // Some comment.\n" @@ -14382,7 +14366,6 @@ TEST_F(FormatTest, SplitEmptyFunction) { TEST_F(FormatTest, SplitEmptyFunctionButNotRecord) { FormatStyle Style = getLLVMStyleWithColumns(40); Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None; - Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None; Style.BreakBeforeBraces = FormatStyle::BS_Custom; Style.BraceWrapping.AfterFunction = true; Style.BraceWrapping.SplitEmptyFunction = true; @@ -21590,7 +21573,6 @@ TEST_F(FormatTest, OptimizeBreakPenaltyVsExcess) { TEST_F(FormatTest, BreakPenaltyAfterLParen) { FormatStyle Style = getLLVMStyle(); Style.ColumnLimit = 8; - Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None; Style.PenaltyExcessCharacter = 15; verifyFormat("int foo(\n" " int aaaaaaaaaaaaaaaaaaaaaaaa);", @@ -27012,7 +26994,6 @@ TEST_F(FormatTest, RemoveParentheses) { TEST_F(FormatTest, AllowBreakBeforeNoexceptSpecifier) { auto Style = getLLVMStyleWithColumns(35); - Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None; EXPECT_EQ(Style.AllowBreakBeforeNoexceptSpecifier, FormatStyle::BBNSS_Never); verifyFormat("void foo(int arg1,\n" diff --git a/clang/unittests/Format/FormatTestComments.cpp b/clang/unittests/Format/FormatTestComments.cpp index eb9c5f22439e7..c249f4d9333fd 100644 --- a/clang/unittests/Format/FormatTestComments.cpp +++ b/clang/unittests/Format/FormatTestComments.cpp @@ -519,11 +519,10 @@ TEST_F(FormatTestComments, CorrectlyHandlesLengthOfBlockComments) { } TEST_F(FormatTestComments, DontBreakNonTrailingBlockComments) { - FormatStyle Style = getLLVMStyleWithColumns(35); - Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None; EXPECT_EQ("void ffffffffff(\n" " int aaaaa /* test */);", - format("void ffffffffff(int aaaaa /* test */);", Style)); + format("void ffffffffff(int aaaaa /* test */);", + getLLVMStyleWithColumns(35))); } TEST_F(FormatTestComments, SplitsLongCxxComments) { @@ -605,11 +604,11 @@ TEST_F(FormatTestComments, SplitsLongCxxComments) { format("// A comment before a macro definition\n" "#define a b", getLLVMStyleWithColumns(20))); - EXPECT_EQ("void\n" - "ffffff(int aaaaaaaaa, // wwww\n" - " int bbbbbbbbbb, // xxxxxxx\n" - " // yyyyyyyyyy\n" - " int c, int d, int e) {}", + EXPECT_EQ("void ffffff(\n" + " int aaaaaaaaa, // wwww\n" + " int bbbbbbbbbb, // xxxxxxx\n" + " // yyyyyyyyyy\n" + " int c, int d, int e) {}", format("void ffffff(\n" " int aaaaaaaaa, // wwww\n" " int bbbbbbbbbb, // xxxxxxx yyyyyyyyyy\n" @@ -4101,7 +4100,6 @@ TEST_F(FormatTestComments, SpaceAtLineCommentBegin) { format(Code, Style)); Style = getLLVMStyleWithColumns(20); - Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None; StringRef WrapCode = "//Lorem ipsum dolor sit amet\n" "\n" "// Lorem ipsum dolor sit amet\n" @@ -4585,7 +4583,7 @@ TEST_F(FormatTestComments, SplitCommentIntroducers) { )", format(R"(// /\ -/ +/ )", getLLVMStyleWithColumns(10))); } diff --git a/clang/unittests/Format/FormatTestSelective.cpp b/clang/unittests/Format/FormatTestSelective.cpp index c21c9bfe60790..43de93d219b14 100644 --- a/clang/unittests/Format/FormatTestSelective.cpp +++ b/clang/unittests/Format/FormatTestSelective.cpp @@ -522,8 +522,7 @@ TEST_F(FormatTestSelective, ReformatRegionAdjustsIndent) { Style.ColumnLimit = 11; EXPECT_EQ(" int a;\n" - " void\n" - " ffffff() {\n" + " void ffffff() {\n" " }", format(" int a;\n" "void ffffff() {}", >From 1f7657aa3a51494292fcdd72e5e5a40d789a4f48 Mon Sep 17 00:00:00 2001 From: rmarker <rmar...@outlook.com> Date: Tue, 30 Jan 2024 21:17:20 +1030 Subject: [PATCH 12/17] Revert "Update formatting in polly to pass polly-check-format." This reverts commit 01fd15c864b01218d36f1acd86da8476cddf97c8. --- polly/include/polly/DependenceInfo.h | 4 ++-- polly/include/polly/LinkAllPasses.h | 4 ++-- polly/lib/Analysis/DependenceInfo.cpp | 5 ++--- polly/lib/Analysis/ScopBuilder.cpp | 10 ++++------ polly/lib/Analysis/ScopDetection.cpp | 11 +++++------ polly/lib/CodeGen/IslNodeBuilder.cpp | 9 ++++----- polly/lib/Transform/MaximalStaticExpansion.cpp | 4 ++-- polly/lib/Transform/ScheduleOptimizer.cpp | 4 ++-- 8 files changed, 23 insertions(+), 28 deletions(-) diff --git a/polly/include/polly/DependenceInfo.h b/polly/include/polly/DependenceInfo.h index fb7b1740ea253..7526a294c6baf 100644 --- a/polly/include/polly/DependenceInfo.h +++ b/polly/include/polly/DependenceInfo.h @@ -332,8 +332,8 @@ namespace llvm { void initializeDependenceInfoPass(llvm::PassRegistry &); void initializeDependenceInfoPrinterLegacyPassPass(llvm::PassRegistry &); void initializeDependenceInfoWrapperPassPass(llvm::PassRegistry &); -void -initializeDependenceInfoPrinterLegacyFunctionPassPass(llvm::PassRegistry &); +void initializeDependenceInfoPrinterLegacyFunctionPassPass( + llvm::PassRegistry &); } // namespace llvm #endif diff --git a/polly/include/polly/LinkAllPasses.h b/polly/include/polly/LinkAllPasses.h index a293b7074f3bd..a2f8f33299918 100644 --- a/polly/include/polly/LinkAllPasses.h +++ b/polly/include/polly/LinkAllPasses.h @@ -138,8 +138,8 @@ void initializeJSONImporterPrinterLegacyPassPass(llvm::PassRegistry &); void initializeDependenceInfoPass(llvm::PassRegistry &); void initializeDependenceInfoPrinterLegacyPassPass(llvm::PassRegistry &); void initializeDependenceInfoWrapperPassPass(llvm::PassRegistry &); -void -initializeDependenceInfoPrinterLegacyFunctionPassPass(llvm::PassRegistry &); +void initializeDependenceInfoPrinterLegacyFunctionPassPass( + llvm::PassRegistry &); void initializeIslAstInfoWrapperPassPass(llvm::PassRegistry &); void initializeIslAstInfoPrinterLegacyPassPass(llvm::PassRegistry &); void initializeCodeGenerationPass(llvm::PassRegistry &); diff --git a/polly/lib/Analysis/DependenceInfo.cpp b/polly/lib/Analysis/DependenceInfo.cpp index 3da6738b64584..69257c603877e 100644 --- a/polly/lib/Analysis/DependenceInfo.cpp +++ b/polly/lib/Analysis/DependenceInfo.cpp @@ -647,9 +647,8 @@ bool Dependences::isValidSchedule(Scop &S, isl::schedule NewSched) const { return isValidSchedule(S, NewSchedules); } -bool -Dependences::isValidSchedule(Scop &S, - const StatementToIslMapTy &NewSchedule) const { +bool Dependences::isValidSchedule( + Scop &S, const StatementToIslMapTy &NewSchedule) const { if (LegalityCheckDisabled) return true; diff --git a/polly/lib/Analysis/ScopBuilder.cpp b/polly/lib/Analysis/ScopBuilder.cpp index c14b01116ec40..c62cb2a85c835 100644 --- a/polly/lib/Analysis/ScopBuilder.cpp +++ b/polly/lib/Analysis/ScopBuilder.cpp @@ -827,9 +827,8 @@ void ScopBuilder::buildInvariantEquivalenceClasses() { } } -bool -ScopBuilder::buildDomains(Region *R, - DenseMap<BasicBlock *, isl::set> &InvalidDomainMap) { +bool ScopBuilder::buildDomains( + Region *R, DenseMap<BasicBlock *, isl::set> &InvalidDomainMap) { bool IsOnlyNonAffineRegion = scop->isNonAffineSubRegion(R); auto *EntryBB = R->getEntry(); auto *L = IsOnlyNonAffineRegion ? nullptr : LI.getLoopFor(EntryBB); @@ -3298,9 +3297,8 @@ bool ScopBuilder::buildAliasGroups() { return true; } -bool -ScopBuilder::buildAliasGroup(AliasGroupTy &AliasGroup, - DenseSet<const ScopArrayInfo *> HasWriteAccess) { +bool ScopBuilder::buildAliasGroup( + AliasGroupTy &AliasGroup, DenseSet<const ScopArrayInfo *> HasWriteAccess) { AliasGroupTy ReadOnlyAccesses; AliasGroupTy ReadWriteAccesses; SmallPtrSet<const ScopArrayInfo *, 4> ReadWriteArrays; diff --git a/polly/lib/Analysis/ScopDetection.cpp b/polly/lib/Analysis/ScopDetection.cpp index 8af4986dc1bbe..938d3f149677b 100644 --- a/polly/lib/Analysis/ScopDetection.cpp +++ b/polly/lib/Analysis/ScopDetection.cpp @@ -978,10 +978,9 @@ bool ScopDetection::hasValidArraySizes(DetectionContext &Context, // non-affine accesses are allowed, we drop the information. In case the // information is dropped the memory accesses need to be overapproximated // when translated to a polyhedral representation. -bool -ScopDetection::computeAccessFunctions(DetectionContext &Context, - const SCEVUnknown *BasePointer, - std::shared_ptr<ArrayShape> Shape) const { +bool ScopDetection::computeAccessFunctions( + DetectionContext &Context, const SCEVUnknown *BasePointer, + std::shared_ptr<ArrayShape> Shape) const { Value *BaseValue = BasePointer->getValue(); bool BasePtrHasNonAffine = false; MapInsnToMemAcc TempMemoryAccesses; @@ -1692,8 +1691,8 @@ bool ScopDetection::hasSufficientCompute(DetectionContext &Context, return InstCount >= ProfitabilityMinPerLoopInstructions; } -bool -ScopDetection::hasPossiblyDistributableLoop(DetectionContext &Context) const { +bool ScopDetection::hasPossiblyDistributableLoop( + DetectionContext &Context) const { for (auto *BB : Context.CurRegion.blocks()) { auto *L = LI.getLoopFor(BB); if (!Context.CurRegion.contains(L)) diff --git a/polly/lib/CodeGen/IslNodeBuilder.cpp b/polly/lib/CodeGen/IslNodeBuilder.cpp index 885b5ee9a1fae..a226cc2a1b250 100644 --- a/polly/lib/CodeGen/IslNodeBuilder.cpp +++ b/polly/lib/CodeGen/IslNodeBuilder.cpp @@ -831,9 +831,8 @@ void IslNodeBuilder::createSubstitutionsVector( isl_ast_expr_free(Expr); } -void -IslNodeBuilder::generateCopyStmt(ScopStmt *Stmt, - __isl_keep isl_id_to_ast_expr *NewAccesses) { +void IslNodeBuilder::generateCopyStmt( + ScopStmt *Stmt, __isl_keep isl_id_to_ast_expr *NewAccesses) { assert(Stmt->size() == 2); auto ReadAccess = Stmt->begin(); auto WriteAccess = ReadAccess++; @@ -1130,8 +1129,8 @@ Value *IslNodeBuilder::preloadInvariantLoad(const MemoryAccess &MA, return PreloadVal; } -bool -IslNodeBuilder::preloadInvariantEquivClass(InvariantEquivClassTy &IAClass) { +bool IslNodeBuilder::preloadInvariantEquivClass( + InvariantEquivClassTy &IAClass) { // For an equivalence class of invariant loads we pre-load the representing // element with the unified execution context. However, we have to map all // elements of the class to the one preloaded load as they are referenced diff --git a/polly/lib/Transform/MaximalStaticExpansion.cpp b/polly/lib/Transform/MaximalStaticExpansion.cpp index de9b9bbc2cf5c..e32a69d47f69c 100644 --- a/polly/lib/Transform/MaximalStaticExpansion.cpp +++ b/polly/lib/Transform/MaximalStaticExpansion.cpp @@ -535,8 +535,8 @@ void MaximalStaticExpanderWrapperPass::printScop(raw_ostream &OS, S.print(OS, false); } -void -MaximalStaticExpanderWrapperPass::getAnalysisUsage(AnalysisUsage &AU) const { +void MaximalStaticExpanderWrapperPass::getAnalysisUsage( + AnalysisUsage &AU) const { ScopPass::getAnalysisUsage(AU); AU.addRequired<DependenceInfo>(); AU.addRequired<OptimizationRemarkEmitterWrapperPass>(); diff --git a/polly/lib/Transform/ScheduleOptimizer.cpp b/polly/lib/Transform/ScheduleOptimizer.cpp index 751b073b7f24f..8ee2b66339adb 100644 --- a/polly/lib/Transform/ScheduleOptimizer.cpp +++ b/polly/lib/Transform/ScheduleOptimizer.cpp @@ -990,8 +990,8 @@ void IslScheduleOptimizerWrapperPass::printScop(raw_ostream &OS, Scop &) const { runScheduleOptimizerPrinter(OS, LastSchedule); } -void -IslScheduleOptimizerWrapperPass::getAnalysisUsage(AnalysisUsage &AU) const { +void IslScheduleOptimizerWrapperPass::getAnalysisUsage( + AnalysisUsage &AU) const { ScopPass::getAnalysisUsage(AU); AU.addRequired<DependenceInfo>(); AU.addRequired<TargetTransformInfoWrapperPass>(); >From 4b7eeb1885323a94e85ceed9ef1eb45f60e368d5 Mon Sep 17 00:00:00 2001 From: rmarker <rmar...@outlook.com> Date: Tue, 30 Jan 2024 21:17:33 +1030 Subject: [PATCH 13/17] Revert "Switch LLVMStyle.AlwaysBreakAfterReturnType to RTBS_AllowShortType." This reverts commit 0fb332e1d8019ccf184b473e57a9762fdccaf5a0. --- clang/lib/Format/Format.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index 404307e70fe7f..a1034ce7c3266 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -1454,7 +1454,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) { LLVMStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never; LLVMStyle.AllowShortLambdasOnASingleLine = FormatStyle::SLS_All; LLVMStyle.AllowShortLoopsOnASingleLine = false; - LLVMStyle.AlwaysBreakAfterReturnType = FormatStyle::RTBS_AllowShortType; + LLVMStyle.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None; LLVMStyle.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None; LLVMStyle.AlwaysBreakBeforeMultilineStrings = false; LLVMStyle.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_MultiLine; >From d7238d4b6f500c2f654d7562858f2a673b8205c1 Mon Sep 17 00:00:00 2001 From: rmarker <rmar...@outlook.com> Date: Tue, 30 Jan 2024 23:18:12 +1030 Subject: [PATCH 14/17] Add ExceptShortType option for AlwaysBreakAfterReturnType. --- clang/docs/ClangFormatStyleOptions.rst | 17 +++++++++++++ clang/docs/ReleaseNotes.rst | 1 + clang/include/clang/Format/Format.h | 15 ++++++++++++ clang/lib/Format/ContinuationIndenter.cpp | 19 +++++++++------ clang/lib/Format/Format.cpp | 1 + clang/lib/Format/TokenAnnotator.cpp | 1 + clang/unittests/Format/ConfigParseTest.cpp | 2 ++ clang/unittests/Format/FormatTest.cpp | 24 +++++++++++++++---- .../unittests/Format/FormatTestSelective.cpp | 3 ++- 9 files changed, 70 insertions(+), 13 deletions(-) diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst index b60d91c82cdf9..bd49751ebdefe 100644 --- a/clang/docs/ClangFormatStyleOptions.rst +++ b/clang/docs/ClangFormatStyleOptions.rst @@ -1566,6 +1566,23 @@ the configuration (without a prefix: ``Auto``). int LongName::AnotherLongName(); + * ``RTBS_ExceptShortType`` (in configuration: ``ExceptShortType``) + Break after return type automatically. + ``PenaltyReturnTypeOnItsOwnLine`` is taken into account. + This mode will never break after short return types, unlike + ``RTBS_None`` which will only sometimes choose not to break after short + return types. + + .. code-block:: c++ + + class A { + int f() { return 0; }; + }; + int f(); + int f() { return 1; } + int LongName:: + AnotherLongName(); + * ``RTBS_All`` (in configuration: ``All``) Always break after the return type. diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 56befd7eefb2f..cfec986664b1f 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -185,6 +185,7 @@ AST Matchers clang-format ------------ - Add ``AllowShortType`` option for ``AlwaysBreakAfterReturnType``. +- Add ``ExceptShortType`` option for ``AlwaysBreakAfterReturnType``. libclang -------- diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h index 4ede7c19c4471..37881c76a4d07 100644 --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -940,6 +940,21 @@ struct FormatStyle { /// LongName::AnotherLongName(); /// \endcode RTBS_AllowShortType, + /// Break after return type automatically. + /// ``PenaltyReturnTypeOnItsOwnLine`` is taken into account. + /// This mode will never break after short return types, unlike + /// ``RTBS_None`` which will only sometimes choose not to break after short + /// return types. + /// \code + /// class A { + /// int f() { return 0; }; + /// }; + /// int f(); + /// int f() { return 1; } + /// int LongName:: + /// AnotherLongName(); + /// \endcode + RTBS_ExceptShortType, /// Always break after the return type. /// \code /// class A { diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp index 47a2bb02aabb8..a3aca4a725311 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -326,14 +326,19 @@ bool ContinuationIndenter::canBreak(const LineState &State) { return false; } - if (Style.AlwaysBreakAfterReturnType == FormatStyle::RTBS_None) { - // Don't break after very short return types (e.g. "void") as that is often - // unexpected. - assert(State.Column >= State.FirstIndent); - if (Current.is(TT_FunctionDeclarationName) && - State.Column - State.FirstIndent < 6) { + // Don't break after very short return types (e.g. "void") as that is often + // unexpected. + if (Current.is(TT_FunctionDeclarationName)) { + if (Style.AlwaysBreakAfterReturnType == FormatStyle::RTBS_None && + State.Column < 6) { return false; } + + if (Style.AlwaysBreakAfterReturnType == FormatStyle::RTBS_ExceptShortType) { + assert(State.Column >= State.FirstIndent); + if (State.Column - State.FirstIndent < 6) + return false; + } } // If binary operators are moved to the next line (including commas for some @@ -590,7 +595,7 @@ bool ContinuationIndenter::mustBreak(const LineState &State) { !State.Line->ReturnTypeWrapped && // Don't break before a C# function when no break after return type. (!Style.isCSharp() || - Style.AlwaysBreakAfterReturnType > FormatStyle::RTBS_AllowShortType) && + Style.AlwaysBreakAfterReturnType > FormatStyle::RTBS_ExceptShortType) && // Don't always break between a JavaScript `function` and the function // name. !Style.isJavaScript() && Previous.isNot(tok::kw_template) && diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index a1034ce7c3266..f980a9f23e962 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -575,6 +575,7 @@ struct ScalarEnumerationTraits<FormatStyle::ReturnTypeBreakingStyle> { static void enumeration(IO &IO, FormatStyle::ReturnTypeBreakingStyle &Value) { IO.enumCase(Value, "None", FormatStyle::RTBS_None); IO.enumCase(Value, "AllowShortType", FormatStyle::RTBS_AllowShortType); + IO.enumCase(Value, "ExceptShortType", FormatStyle::RTBS_ExceptShortType); IO.enumCase(Value, "All", FormatStyle::RTBS_All); IO.enumCase(Value, "TopLevel", FormatStyle::RTBS_TopLevel); IO.enumCase(Value, "TopLevelDefinitions", diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp index 81d866e8a2f47..3da75ffa936a5 100644 --- a/clang/lib/Format/TokenAnnotator.cpp +++ b/clang/lib/Format/TokenAnnotator.cpp @@ -3433,6 +3433,7 @@ bool TokenAnnotator::mustBreakForReturnType(const AnnotatedLine &Line) const { switch (Style.AlwaysBreakAfterReturnType) { case FormatStyle::RTBS_None: case FormatStyle::RTBS_AllowShortType: + case FormatStyle::RTBS_ExceptShortType: return false; case FormatStyle::RTBS_All: case FormatStyle::RTBS_TopLevel: diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp index 8b1b289508a58..13fd75718123b 100644 --- a/clang/unittests/Format/ConfigParseTest.cpp +++ b/clang/unittests/Format/ConfigParseTest.cpp @@ -699,6 +699,8 @@ TEST(ConfigParseTest, ParsesConfiguration) { FormatStyle::RTBS_None); CHECK_PARSE("AlwaysBreakAfterReturnType: AllowShortType", AlwaysBreakAfterReturnType, FormatStyle::RTBS_AllowShortType); + CHECK_PARSE("AlwaysBreakAfterReturnType: ExceptShortType", + AlwaysBreakAfterReturnType, FormatStyle::RTBS_ExceptShortType); CHECK_PARSE("AlwaysBreakAfterReturnType: All", AlwaysBreakAfterReturnType, FormatStyle::RTBS_All); CHECK_PARSE("AlwaysBreakAfterReturnType: TopLevel", diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index bd0fc45cee9fd..5a7209b5d6a55 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -9874,8 +9874,8 @@ TEST_F(FormatTest, ReturnTypeBreakingStyle) { verifyFormat("class A {\n" " int f() { return 1; }\n" " int g();\n" - " long foooooooooooooooooooooooooooo::\n" - " baaaaaaaaaaaaaaaaaaaar();\n" + " long\n" + " foooooooooooooooooooooooooooo::baaaaaaaaaaaaaaaaaaaar();\n" "};\n" "int f() { return 1; }\n" "int g();\n" @@ -9897,6 +9897,20 @@ TEST_F(FormatTest, ReturnTypeBreakingStyle) { "foooooooooooooooooooooooooooo::baaaaaaaaaaaaaaaaaaaaar();", Style); + // It now must never break after a short return type. + Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_ExceptShortType; + verifyFormat("class A {\n" + " int f() { return 1; }\n" + " int g();\n" + " long foooooooooooooooooooooooooooo::\n" + " baaaaaaaaaaaaaaaaaaaar();\n" + "};\n" + "int f() { return 1; }\n" + "int g();\n" + "int foooooooooooooooooooooooooooo::\n" + " baaaaaaaaaaaaaaaaaaaaar();", + Style); + // All declarations and definitions should have the return type moved to its // own line. Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_All; @@ -14384,9 +14398,9 @@ TEST_F(FormatTest, SplitEmptyFunctionButNotRecord) { " bbbbbbbbbbbbbbbbbbb()\n" " {\n" " }\n" - " void m(\n" - " int aaaaaaaaaaaaaaaaaaaaaaaaaaaa,\n" - " int bbbbbbbbbbbbbbbbbbbbbbbb)\n" + " void\n" + " m(int aaaaaaaaaaaaaaaaaaaaaaaaaaaa,\n" + " int bbbbbbbbbbbbbbbbbbbbbbbb)\n" " {\n" " }\n" "};", diff --git a/clang/unittests/Format/FormatTestSelective.cpp b/clang/unittests/Format/FormatTestSelective.cpp index 43de93d219b14..c21c9bfe60790 100644 --- a/clang/unittests/Format/FormatTestSelective.cpp +++ b/clang/unittests/Format/FormatTestSelective.cpp @@ -522,7 +522,8 @@ TEST_F(FormatTestSelective, ReformatRegionAdjustsIndent) { Style.ColumnLimit = 11; EXPECT_EQ(" int a;\n" - " void ffffff() {\n" + " void\n" + " ffffff() {\n" " }", format(" int a;\n" "void ffffff() {}", >From 8df12c23ea67db969f85dfd01a33d7145f5c94f6 Mon Sep 17 00:00:00 2001 From: rmarker <rmar...@outlook.com> Date: Thu, 1 Feb 2024 17:58:55 +1030 Subject: [PATCH 15/17] Address review feedback. Deprecate RTBS_None. Rename RTBS_AllowShortType -> RTBS_Automatic. Remove new option values from release notes. --- clang/docs/ClangFormatStyleOptions.rst | 25 ++++------------------ clang/docs/ReleaseNotes.rst | 2 -- clang/include/clang/Format/Format.h | 24 ++++----------------- clang/lib/Format/Format.cpp | 2 +- clang/lib/Format/TokenAnnotator.cpp | 2 +- clang/unittests/Format/ConfigParseTest.cpp | 4 ++-- clang/unittests/Format/FormatTest.cpp | 2 +- 7 files changed, 13 insertions(+), 48 deletions(-) diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst index bd49751ebdefe..9b7a5d4658a69 100644 --- a/clang/docs/ClangFormatStyleOptions.rst +++ b/clang/docs/ClangFormatStyleOptions.rst @@ -1537,24 +1537,10 @@ the configuration (without a prefix: ``Auto``). Possible values: * ``RTBS_None`` (in configuration: ``None``) - Break after return type automatically. - ``PenaltyReturnTypeOnItsOwnLine`` is taken into account. + This is **deprecated**. See ``Automatic`` below. - .. code-block:: c++ - - class A { - int f() { return 0; }; - }; - int f(); - int f() { return 1; } - int LongName:: - AnotherLongName(); - - * ``RTBS_AllowShortType`` (in configuration: ``AllowShortType``) - Break after return type automatically. - This mode doesn't have the same inherent restriction on breaking after - short return types as ``RTBS_None`` and is solely based on - ``PenaltyReturnTypeOnItsOwnLine``. + * ``RTBS_Automatic`` (in configuration: ``Automatic``) + Break after return type based on ``PenaltyReturnTypeOnItsOwnLine``. .. code-block:: c++ @@ -1567,10 +1553,7 @@ the configuration (without a prefix: ``Auto``). LongName::AnotherLongName(); * ``RTBS_ExceptShortType`` (in configuration: ``ExceptShortType``) - Break after return type automatically. - ``PenaltyReturnTypeOnItsOwnLine`` is taken into account. - This mode will never break after short return types, unlike - ``RTBS_None`` which will only sometimes choose not to break after short + Same as ``Automatic`` above, expect that there is no break after short return types. .. code-block:: c++ diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index cfec986664b1f..5330cd9caad80 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -184,8 +184,6 @@ AST Matchers clang-format ------------ -- Add ``AllowShortType`` option for ``AlwaysBreakAfterReturnType``. -- Add ``ExceptShortType`` option for ``AlwaysBreakAfterReturnType``. libclang -------- diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h index 37881c76a4d07..9ebef1abf7cff 100644 --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -914,22 +914,9 @@ struct FormatStyle { /// Different ways to break after the function definition or /// declaration return type. enum ReturnTypeBreakingStyle : int8_t { - /// Break after return type automatically. - /// ``PenaltyReturnTypeOnItsOwnLine`` is taken into account. - /// \code - /// class A { - /// int f() { return 0; }; - /// }; - /// int f(); - /// int f() { return 1; } - /// int LongName:: - /// AnotherLongName(); - /// \endcode + /// This is **deprecated**. See ``Automatic`` below. RTBS_None, - /// Break after return type automatically. - /// This mode doesn't have the same inherent restriction on breaking after - /// short return types as ``RTBS_None`` and is solely based on - /// ``PenaltyReturnTypeOnItsOwnLine``. + /// Break after return type based on ``PenaltyReturnTypeOnItsOwnLine``. /// \code /// class A { /// int f() { return 0; }; @@ -939,11 +926,8 @@ struct FormatStyle { /// int /// LongName::AnotherLongName(); /// \endcode - RTBS_AllowShortType, - /// Break after return type automatically. - /// ``PenaltyReturnTypeOnItsOwnLine`` is taken into account. - /// This mode will never break after short return types, unlike - /// ``RTBS_None`` which will only sometimes choose not to break after short + RTBS_Automatic, + /// Same as ``Automatic`` above, expect that there is no break after short /// return types. /// \code /// class A { diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index f980a9f23e962..b4718f9e96e33 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -574,7 +574,7 @@ template <> struct ScalarEnumerationTraits<FormatStyle::ReturnTypeBreakingStyle> { static void enumeration(IO &IO, FormatStyle::ReturnTypeBreakingStyle &Value) { IO.enumCase(Value, "None", FormatStyle::RTBS_None); - IO.enumCase(Value, "AllowShortType", FormatStyle::RTBS_AllowShortType); + IO.enumCase(Value, "Automatic", FormatStyle::RTBS_Automatic); IO.enumCase(Value, "ExceptShortType", FormatStyle::RTBS_ExceptShortType); IO.enumCase(Value, "All", FormatStyle::RTBS_All); IO.enumCase(Value, "TopLevel", FormatStyle::RTBS_TopLevel); diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp index 3da75ffa936a5..73ce3860993e9 100644 --- a/clang/lib/Format/TokenAnnotator.cpp +++ b/clang/lib/Format/TokenAnnotator.cpp @@ -3432,7 +3432,7 @@ bool TokenAnnotator::mustBreakForReturnType(const AnnotatedLine &Line) const { switch (Style.AlwaysBreakAfterReturnType) { case FormatStyle::RTBS_None: - case FormatStyle::RTBS_AllowShortType: + case FormatStyle::RTBS_Automatic: case FormatStyle::RTBS_ExceptShortType: return false; case FormatStyle::RTBS_All: diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp index 13fd75718123b..3ebf1f3861fbf 100644 --- a/clang/unittests/Format/ConfigParseTest.cpp +++ b/clang/unittests/Format/ConfigParseTest.cpp @@ -697,8 +697,8 @@ TEST(ConfigParseTest, ParsesConfiguration) { Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_All; CHECK_PARSE("AlwaysBreakAfterReturnType: None", AlwaysBreakAfterReturnType, FormatStyle::RTBS_None); - CHECK_PARSE("AlwaysBreakAfterReturnType: AllowShortType", - AlwaysBreakAfterReturnType, FormatStyle::RTBS_AllowShortType); + CHECK_PARSE("AlwaysBreakAfterReturnType: Automatic", + AlwaysBreakAfterReturnType, FormatStyle::RTBS_Automatic); CHECK_PARSE("AlwaysBreakAfterReturnType: ExceptShortType", AlwaysBreakAfterReturnType, FormatStyle::RTBS_ExceptShortType); CHECK_PARSE("AlwaysBreakAfterReturnType: All", AlwaysBreakAfterReturnType, diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 5a7209b5d6a55..ff9cb82106642 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -9884,7 +9884,7 @@ TEST_F(FormatTest, ReturnTypeBreakingStyle) { Style); // It is now allowed to break after a short return type if necessary. - Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_AllowShortType; + Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_Automatic; verifyFormat("class A {\n" " int f() { return 1; }\n" " int g();\n" >From 8365ed9820c7d16e513888ddbb11c908d086973e Mon Sep 17 00:00:00 2001 From: rmarker <rmar...@outlook.com> Date: Thu, 1 Feb 2024 21:06:08 +1030 Subject: [PATCH 16/17] Fix typo. --- clang/docs/ClangFormatStyleOptions.rst | 2 +- clang/include/clang/Format/Format.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst index 9b7a5d4658a69..33fd6c0286783 100644 --- a/clang/docs/ClangFormatStyleOptions.rst +++ b/clang/docs/ClangFormatStyleOptions.rst @@ -1553,7 +1553,7 @@ the configuration (without a prefix: ``Auto``). LongName::AnotherLongName(); * ``RTBS_ExceptShortType`` (in configuration: ``ExceptShortType``) - Same as ``Automatic`` above, expect that there is no break after short + Same as ``Automatic`` above, except that there is no break after short return types. .. code-block:: c++ diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h index 9ebef1abf7cff..0e710a69b1002 100644 --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -927,7 +927,7 @@ struct FormatStyle { /// LongName::AnotherLongName(); /// \endcode RTBS_Automatic, - /// Same as ``Automatic`` above, expect that there is no break after short + /// Same as ``Automatic`` above, except that there is no break after short /// return types. /// \code /// class A { >From 76ea64622b303dbeaca75c654a668d231b0858f1 Mon Sep 17 00:00:00 2001 From: rmarker <rmar...@outlook.com> Date: Fri, 2 Feb 2024 13:37:24 +1030 Subject: [PATCH 17/17] Remove some superfluous code from tests. --- clang/unittests/Format/FormatTest.cpp | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index ff9cb82106642..49d12551863e6 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -9944,16 +9944,12 @@ TEST_F(FormatTest, ReturnTypeBreakingStyle) { verifyFormat("class B {\n" " int f() { return 1; }\n" " int g();\n" - " long\n" - " foooooooooooooooooooooooooooo::baaaaaaaaaaaaaaaaaaaar();\n" "};\n" "int\n" "f() {\n" " return 1;\n" "}\n" - "int g();\n" - "int\n" - "foooooooooooooooooooooooooooo::baaaaaaaaaaaaaaaaaaaaar();", + "int g();", Style); // Top-level definitions and declarations should have the return type moved @@ -9962,8 +9958,6 @@ TEST_F(FormatTest, ReturnTypeBreakingStyle) { verifyFormat("class C {\n" " int f() { return 1; }\n" " int g();\n" - " long\n" - " foooooooooooooooooooooooooooo::baaaaaaaaaaaaaaaaaaaar();\n" "};\n" "int\n" "f() {\n" @@ -9984,16 +9978,12 @@ TEST_F(FormatTest, ReturnTypeBreakingStyle) { " return 1;\n" " }\n" " int g();\n" - " long\n" - " foooooooooooooooooooooooooooo::baaaaaaaaaaaaaaaaaaaar();\n" "};\n" "int\n" "f() {\n" " return 1;\n" "}\n" - "int g();\n" - "int\n" - "foooooooooooooooooooooooooooo::baaaaaaaaaaaaaaaaaaaaar();", + "int g();", Style); verifyFormat("const char *\n" "f(void) {\n" // Break here. _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits