[flang] [clang] [llvm] [lld] [clang-tools-extra] [lldb] [mlir] [compiler-rt] [libcxx] [clang-format] Add ShortReturnTypeColumn option. (PR #78011)

via llvm-commits llvm-commits at lists.llvm.org
Sat Jan 20 21:09:26 PST 2024


https://github.com/rmarker updated https://github.com/llvm/llvm-project/pull/78011

>From c4d28f82e108f9f12ccd0375e2a3502025b8c1e8 Mon Sep 17 00:00:00 2001
From: rmarker <rmarker at outlook.com>
Date: Thu, 11 Jan 2024 15:01:18 +1030
Subject: [PATCH 1/3] [clang-format] Add ShortReturnTypeLength option.

---
 clang/docs/ClangFormatStyleOptions.rst     |  8 ++++
 clang/docs/ReleaseNotes.rst                |  1 +
 clang/include/clang/Format/Format.h        |  8 ++++
 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, 66 insertions(+), 1 deletion(-)

diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst
index ac9a0b70ed5daa4..3255ceb0aba75b4 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -4994,6 +4994,14 @@ the configuration (without a prefix: ``Auto``).
        int bar;                           int bar;
      } // namespace b                   } // namespace b
 
+.. _ShortReturnTypeLength:
+
+**ShortReturnTypeLength** (``Unsigned``) :versionbadge:`clang-format 18` :ref:`¶ <ShortReturnTypeLength>`
+  When AlwaysBreakAfterReturnType is None, line breaks are prevented after
+  short return types. This configures the character limit for a type to be
+  regarded as short. Note that this isn't the length of the type itself,
+  but the column where it finishes. I.e. it includes indentation, etc.
+
 .. _SortIncludes:
 
 **SortIncludes** (``SortIncludesOptions``) :versionbadge:`clang-format 3.8` :ref:`¶ <SortIncludes>`
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 3cbce1be1594376..04bf5cd4e768f34 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1131,6 +1131,7 @@ clang-format
 - Add ``BreakAdjacentStringLiterals`` option.
 - Add ``ObjCPropertyAttributeOrder`` which can be used to sort ObjC property
   attributes (like ``nonatomic, strong, nullable``).
+- Add ``ShortReturnTypeLength`` option.
 - Add ``.clang-format-ignore`` files.
 - Add ``AlignFunctionPointers`` sub-option for ``AlignConsecutiveDeclarations``.
 
diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index 5ffd63ee73fc361..f94d68f2cf2a853 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -3928,6 +3928,13 @@ struct FormatStyle {
   /// \version 13
   unsigned ShortNamespaceLines;
 
+  /// When AlwaysBreakAfterReturnType is None, line breaks are prevented after
+  /// short return types. This configures the character limit for a type to be
+  /// regarded as short. Note that this isn't the length of the type itself,
+  /// but the column where it finishes. I.e. it includes indentation, etc.
+  /// \version 18
+  unsigned ShortReturnTypeLength;
+
   /// Include sorting options.
   enum SortIncludesOptions : int8_t {
     /// Includes are never sorted.
@@ -4890,6 +4897,7 @@ struct FormatStyle {
            RequiresExpressionIndentation == R.RequiresExpressionIndentation &&
            SeparateDefinitionBlocks == R.SeparateDefinitionBlocks &&
            ShortNamespaceLines == R.ShortNamespaceLines &&
+           ShortReturnTypeLength == R.ShortReturnTypeLength &&
            SortIncludes == R.SortIncludes &&
            SortJavaStaticImport == R.SortJavaStaticImport &&
            SpaceAfterCStyleCast == R.SpaceAfterCStyleCast &&
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index 102504182c4505b..bc0748ec52e6769 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.ShortReturnTypeLength) {
     if (Style.AlwaysBreakAfterReturnType == FormatStyle::RTBS_None)
       return false;
   }
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index ff5ed6c306f383b..20ffbeef7e9a6e9 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -1083,6 +1083,7 @@ template <> struct MappingTraits<FormatStyle> {
                    Style.RequiresExpressionIndentation);
     IO.mapOptional("SeparateDefinitionBlocks", Style.SeparateDefinitionBlocks);
     IO.mapOptional("ShortNamespaceLines", Style.ShortNamespaceLines);
+    IO.mapOptional("ShortReturnTypeLength", Style.ShortReturnTypeLength);
     IO.mapOptional("SortIncludes", Style.SortIncludes);
     IO.mapOptional("SortJavaStaticImport", Style.SortJavaStaticImport);
     IO.mapOptional("SortUsingDeclarations", Style.SortUsingDeclarations);
@@ -1554,6 +1555,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
   LLVMStyle.RequiresExpressionIndentation = FormatStyle::REI_OuterScope;
   LLVMStyle.SeparateDefinitionBlocks = FormatStyle::SDS_Leave;
   LLVMStyle.ShortNamespaceLines = 1;
+  LLVMStyle.ShortReturnTypeLength = 5;
   LLVMStyle.SortIncludes = FormatStyle::SI_CaseSensitive;
   LLVMStyle.SortJavaStaticImport = FormatStyle::SJSIO_Before;
   LLVMStyle.SortUsingDeclarations = FormatStyle::SUD_LexicographicNumeric;
diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp
index 18ecba270e3455a..dcd8f768d1ab5ad 100644
--- a/clang/unittests/Format/ConfigParseTest.cpp
+++ b/clang/unittests/Format/ConfigParseTest.cpp
@@ -258,6 +258,7 @@ TEST(ConfigParseTest, ParsesConfiguration) {
   CHECK_PARSE("PenaltyExcessCharacter: 1234", PenaltyExcessCharacter, 1234u);
   CHECK_PARSE("PenaltyReturnTypeOnItsOwnLine: 1234",
               PenaltyReturnTypeOnItsOwnLine, 1234u);
+  CHECK_PARSE("ShortReturnTypeLength: 1234", ShortReturnTypeLength, 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 340ae39cb22b036..47b1018172590ac 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -12398,6 +12398,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"
+               "      FunctionDefinition();\n"
+               "}",
+               ShortReturnType);
+
+  ShortReturnType.ShortReturnTypeLength = 0;
+  verifyFormat("Type\n"
+               "Loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo"
+               "ooooooooong::\n"
+               "    FunctionDeclaration();",
+               ShortReturnType);
+  verifyFormat("struct S {\n"
+               "  Type\n"
+               "  "
+               "Loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo"
+               "oooooooooooong::\n"
+               "      FunctionDefinition();\n"
+               "}",
+               ShortReturnType);
+
+  ShortReturnType.ShortReturnTypeLength = 7;
+  verifyFormat("Type "
+               "Loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo"
+               "ooooooooong::\n"
+               "    FunctionDeclaration();",
+               ShortReturnType);
+  verifyFormat("struct S {\n"
+               "  Type "
+               "Loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo"
+               "oooooooooooong::\n"
+               "      FunctionDefinition();\n"
+               "}",
+               ShortReturnType);
 }
 
 TEST_F(FormatTest, FormatsAccessModifiers) {

>From 6a9fd69c88c7bd8b2df529ec20ee228c6a6efb97 Mon Sep 17 00:00:00 2001
From: rmarker <rmarker at outlook.com>
Date: Sun, 14 Jan 2024 14:37:08 +1030
Subject: [PATCH 2/3] Rename option to ShortReturnTypeColumn.

Also, improvements to the corresponding documentation.
---
 clang/docs/ClangFormatStyleOptions.rst     | 17 +++++++++++------
 clang/docs/ReleaseNotes.rst                |  2 +-
 clang/include/clang/Format/Format.h        | 16 ++++++++++------
 clang/lib/Format/ContinuationIndenter.cpp  |  2 +-
 clang/lib/Format/Format.cpp                |  4 ++--
 clang/unittests/Format/ConfigParseTest.cpp |  2 +-
 clang/unittests/Format/FormatTest.cpp      |  4 ++--
 7 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst
index 3255ceb0aba75b4..59437010c3349b3 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -4994,13 +4994,18 @@ the configuration (without a prefix: ``Auto``).
        int bar;                           int bar;
      } // namespace b                   } // namespace b
 
-.. _ShortReturnTypeLength:
+.. _ShortReturnTypeColumn:
 
-**ShortReturnTypeLength** (``Unsigned``) :versionbadge:`clang-format 18` :ref:`¶ <ShortReturnTypeLength>`
-  When AlwaysBreakAfterReturnType is None, line breaks are prevented after
-  short return types. This configures the character limit for a type to be
-  regarded as short. Note that this isn't the length of the type itself,
-  but the column where it finishes. I.e. it includes indentation, etc.
+**ShortReturnTypeColumn** (``Unsigned``) :versionbadge:`clang-format 18` :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.
 
 .. _SortIncludes:
 
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 04bf5cd4e768f34..d21db5bd4b59964 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1131,7 +1131,7 @@ clang-format
 - Add ``BreakAdjacentStringLiterals`` option.
 - Add ``ObjCPropertyAttributeOrder`` which can be used to sort ObjC property
   attributes (like ``nonatomic, strong, nullable``).
-- Add ``ShortReturnTypeLength`` option.
+- Add ``ShortReturnTypeColumn`` option.
 - Add ``.clang-format-ignore`` files.
 - Add ``AlignFunctionPointers`` sub-option for ``AlignConsecutiveDeclarations``.
 
diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index f94d68f2cf2a853..416f61f4be4a241 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -3928,12 +3928,16 @@ struct FormatStyle {
   /// \version 13
   unsigned ShortNamespaceLines;
 
-  /// When AlwaysBreakAfterReturnType is None, line breaks are prevented after
-  /// short return types. This configures the character limit for a type to be
-  /// regarded as short. Note that this isn't the length of the type itself,
-  /// but the column where it finishes. I.e. it includes indentation, etc.
+  /// 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 18
-  unsigned ShortReturnTypeLength;
+  unsigned ShortReturnTypeColumn;
 
   /// Include sorting options.
   enum SortIncludesOptions : int8_t {
@@ -4897,7 +4901,7 @@ struct FormatStyle {
            RequiresExpressionIndentation == R.RequiresExpressionIndentation &&
            SeparateDefinitionBlocks == R.SeparateDefinitionBlocks &&
            ShortNamespaceLines == R.ShortNamespaceLines &&
-           ShortReturnTypeLength == R.ShortReturnTypeLength &&
+           ShortReturnTypeColumn == R.ShortReturnTypeColumn &&
            SortIncludes == R.SortIncludes &&
            SortJavaStaticImport == R.SortJavaStaticImport &&
            SpaceAfterCStyleCast == R.SpaceAfterCStyleCast &&
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index bc0748ec52e6769..ad783ea167a3b54 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.ShortReturnTypeLength) {
+      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 20ffbeef7e9a6e9..a45fc24657368f3 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -1083,7 +1083,7 @@ template <> struct MappingTraits<FormatStyle> {
                    Style.RequiresExpressionIndentation);
     IO.mapOptional("SeparateDefinitionBlocks", Style.SeparateDefinitionBlocks);
     IO.mapOptional("ShortNamespaceLines", Style.ShortNamespaceLines);
-    IO.mapOptional("ShortReturnTypeLength", Style.ShortReturnTypeLength);
+    IO.mapOptional("ShortReturnTypeColumn", Style.ShortReturnTypeColumn);
     IO.mapOptional("SortIncludes", Style.SortIncludes);
     IO.mapOptional("SortJavaStaticImport", Style.SortJavaStaticImport);
     IO.mapOptional("SortUsingDeclarations", Style.SortUsingDeclarations);
@@ -1555,7 +1555,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
   LLVMStyle.RequiresExpressionIndentation = FormatStyle::REI_OuterScope;
   LLVMStyle.SeparateDefinitionBlocks = FormatStyle::SDS_Leave;
   LLVMStyle.ShortNamespaceLines = 1;
-  LLVMStyle.ShortReturnTypeLength = 5;
+  LLVMStyle.ShortReturnTypeColumn = 5;
   LLVMStyle.SortIncludes = FormatStyle::SI_CaseSensitive;
   LLVMStyle.SortJavaStaticImport = FormatStyle::SJSIO_Before;
   LLVMStyle.SortUsingDeclarations = FormatStyle::SUD_LexicographicNumeric;
diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp
index dcd8f768d1ab5ad..3708281dcbd9f47 100644
--- a/clang/unittests/Format/ConfigParseTest.cpp
+++ b/clang/unittests/Format/ConfigParseTest.cpp
@@ -258,7 +258,7 @@ TEST(ConfigParseTest, ParsesConfiguration) {
   CHECK_PARSE("PenaltyExcessCharacter: 1234", PenaltyExcessCharacter, 1234u);
   CHECK_PARSE("PenaltyReturnTypeOnItsOwnLine: 1234",
               PenaltyReturnTypeOnItsOwnLine, 1234u);
-  CHECK_PARSE("ShortReturnTypeLength: 1234", ShortReturnTypeLength, 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 47b1018172590ac..2436170cdc2cded 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -12414,7 +12414,7 @@ TEST_F(FormatTest, BreaksLongDeclarations) {
                "}",
                ShortReturnType);
 
-  ShortReturnType.ShortReturnTypeLength = 0;
+  ShortReturnType.ShortReturnTypeColumn = 0;
   verifyFormat("Type\n"
                "Loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo"
                "ooooooooong::\n"
@@ -12429,7 +12429,7 @@ TEST_F(FormatTest, BreaksLongDeclarations) {
                "}",
                ShortReturnType);
 
-  ShortReturnType.ShortReturnTypeLength = 7;
+  ShortReturnType.ShortReturnTypeColumn = 7;
   verifyFormat("Type "
                "Loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo"
                "ooooooooong::\n"

>From bac78b0ab6d998c8c1a3cc5576923f7d25795b0c Mon Sep 17 00:00:00 2001
From: rmarker <rmarker at outlook.com>
Date: Sun, 14 Jan 2024 15:24:01 +1030
Subject: [PATCH 3/3] Improve consistency of name used in test.

---
 clang/unittests/Format/FormatTest.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 2436170cdc2cded..9b26f61927f9325 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -12410,7 +12410,7 @@ TEST_F(FormatTest, BreaksLongDeclarations) {
                "  "
                "Loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo"
                "oooooooooooong::\n"
-               "      FunctionDefinition();\n"
+               "      FunctionDeclaration();\n"
                "}",
                ShortReturnType);
 
@@ -12425,7 +12425,7 @@ TEST_F(FormatTest, BreaksLongDeclarations) {
                "  "
                "Loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo"
                "oooooooooooong::\n"
-               "      FunctionDefinition();\n"
+               "      FunctionDeclaration();\n"
                "}",
                ShortReturnType);
 
@@ -12439,7 +12439,7 @@ TEST_F(FormatTest, BreaksLongDeclarations) {
                "  Type "
                "Loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo"
                "oooooooooooong::\n"
-               "      FunctionDefinition();\n"
+               "      FunctionDeclaration();\n"
                "}",
                ShortReturnType);
 }



More information about the llvm-commits mailing list