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/10] [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 4dc0de3a90f2650..7836cc8f1c9bb5b 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 5330cd9caad8011..669b420fe21ec15 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 bc9eecd42f9ebfd..7fd574c98a3944f 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 a3eb9138b218335..3f9c0cc815745c3 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 ff326dc784783b2..35478fac7b49629 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 2a8d79359a49b40..baf892b43320d16 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 e5e763edf5b5bf6..5325af89b90ee6b 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/10] 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 3f9c0cc815745c3..3201bc0e7dc2c12 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 5325af89b90ee6b..778d98a4493b61c 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 c21c9bfe6079056..43de93d219b145c 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/10] 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 7836cc8f1c9bb5b..9201a152b11625b 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 669b420fe21ec15..56befd7eefb2fa8 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 7fd574c98a3944f..02f1c6c8b30e97c 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 3201bc0e7dc2c12..ec0fc7fb9243bd6 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 35478fac7b49629..a1034ce7c3266b9 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 25fcceb87864379..81d866e8a2f4705 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 baf892b43320d16..8b1b289508a5860 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 778d98a4493b61c..c2f45767519346e 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/10] 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 9201a152b11625b..de3a75df9a0c45c 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 ec0fc7fb9243bd6..47a2bb02aabb8f5 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 c2f45767519346e..bd0fc45cee9fd41 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/10] 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 75d4a044ef19f68..e41891f07de2e32 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/10] 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 a1034ce7c3266b9..404307e70fe7fe3 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/10] 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 7526a294c6bafed..fb7b1740ea2535b 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 a2f8f3329991864..a293b7074f3bd87 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 69257c603877ea4..3da6738b64584de 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 c62cb2a85c835cd..c14b01116ec406c 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 938d3f149677ba1..8af4986dc1bbe3c 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 a226cc2a1b25065..885b5ee9a1faeeb 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 e32a69d47f69c1e..de9b9bbc2cf5cb2 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 8ee2b66339adbce..751b073b7f24f2c 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/10] 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 bd0fc45cee9fd41..b50ceae1965bde8 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 c249f4d9333fd07..eb9c5f22439e709 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 43de93d219b145c..c21c9bfe6079056 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/10] 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 e41891f07de2e32..75d4a044ef19f68 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/10] 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 de3a75df9a0c45c..b60d91c82cdf98c 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 02f1c6c8b30e97c..4ede7c19c4471e9 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, }; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits