[clang] [polly] [clang-format] Add AllowShortType option for AlwaysBreakAfterReturnType. (PR #78011)

2024-01-27 Thread via cfe-commits 

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

>From a1312a0a463bb946f336977b5b01ef7afbede678 Mon Sep 17 00:00:00 2001
From: rmarker 
Date: Thu, 11 Jan 2024 15:01:18 +1030
Subject: [PATCH 1/8] [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:`¶ `
+  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:`¶ `
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 {
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.ShortNamespaceLi

[clang] [polly] [clang-format] Add AllowShortType option for AlwaysBreakAfterReturnType. (PR #78011)

2024-01-27 Thread via cfe-commits 

rmarker wrote:

Updated FormatTests to account for the change to the style.
(Forgetting them is what I get for coding late).

This should help when evaluating the scope of the change.

https://github.com/llvm/llvm-project/pull/78011
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [polly] [clang-format] Add AllowShortType option for AlwaysBreakAfterReturnType. (PR #78011)

2024-01-28 Thread Björn Schäpers via cfe-commits 


@@ -587,7 +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) &&

HazardyKnusperkeks wrote:

I know Owen proposed this. But I have to note, that I personally really don't 
like greater or lesser with enums.

https://github.com/llvm/llvm-project/pull/78011
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [polly] [clang-format] Add AllowShortType option for AlwaysBreakAfterReturnType. (PR #78011)

2024-01-28 Thread Björn Schäpers via cfe-commits 


@@ -922,8 +922,23 @@ struct FormatStyle {
 ///   };
 ///   int f();
 ///   int f() { return 1; }
+///   int f::
+///   bar();
 /// \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
+///   
f::bar();
+/// \endcode
+RTBS_AllowShortType,
 /// Always break after the return type.
 /// \code
 ///   class A {

HazardyKnusperkeks wrote:

Shouldn't it be added in this block also?

https://github.com/llvm/llvm-project/pull/78011
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [polly] [clang-format] Add AllowShortType option for AlwaysBreakAfterReturnType. (PR #78011)

2024-01-28 Thread Björn Schäpers via cfe-commits 


@@ -922,8 +922,23 @@ struct FormatStyle {
 ///   };
 ///   int f();
 ///   int f() { return 1; }
+///   int f::
+///   bar();
 /// \endcode
 RTBS_None,
+/// Break after return type automatically, while allowing a break after
+/// short return types.

HazardyKnusperkeks wrote:

This should be rephrased, I know what the option means, but I couldn't figure 
it out of the description.

https://github.com/llvm/llvm-project/pull/78011
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [polly] [clang-format] Add AllowShortType option for AlwaysBreakAfterReturnType. (PR #78011)

2024-01-28 Thread Björn Schäpers via cfe-commits 


@@ -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:

HazardyKnusperkeks wrote:

Unrelated.

https://github.com/llvm/llvm-project/pull/78011
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [polly] [clang-format] Add AllowShortType option for AlwaysBreakAfterReturnType. (PR #78011)

2024-01-28 Thread Björn Schäpers via cfe-commits 

HazardyKnusperkeks wrote:

> @mydeveloperday @HazardyKnusperkeks @rymiel this patch fixes a very old bug 
> and will cause behavior changes whether the default is changed to the new 
> `AllowShortType` or left at `None`. Which way should we go?

What would change if the default was kept at `None`? I don't see it.

https://github.com/llvm/llvm-project/pull/78011
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [polly] [clang-format] Add AllowShortType option for AlwaysBreakAfterReturnType. (PR #78011)

2024-01-29 Thread via cfe-commits 

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

>From a1312a0a463bb946f336977b5b01ef7afbede678 Mon Sep 17 00:00:00 2001
From: rmarker 
Date: Thu, 11 Jan 2024 15:01:18 +1030
Subject: [PATCH 1/9] [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:`¶ `
+  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:`¶ `
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 {
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.ShortNamespaceLi

[clang] [polly] [clang-format] Add AllowShortType option for AlwaysBreakAfterReturnType. (PR #78011)

2024-01-29 Thread via cfe-commits 

rmarker wrote:

> > > @mydeveloperday @HazardyKnusperkeks @rymiel this patch fixes a very old 
> > > bug and will cause behavior changes whether the default is changed to the 
> > > new `AllowShortType` or left at `None`. Which way should we go?
> > 
> > 
> > Now I'm leaning toward keeping the existing (buggy) behavior of `None` and 
> > using the new `AllowShortType` to allow wrapping after short return types.
> 
> Buggy would be no break? That would be what I expect from `None`. :) So +1 
> from me.

`None` prevents breaks after short return types. The buggy behaviour is that it 
doesn't take leading indentation into account when determining if a return type 
is short or not. This means that if there is indentation, it wouldn't think a 
type is short that otherwise would be, and would thus allow a break after the 
return type.
Fixing the bug would cause return types in more situations to be identified as 
short and thus prevent breaks previously allowed (changing the previous 
behaviour).

 ```
 // Buggy None
void LongName::
  AnotherLongName();
class C {
   void
   LongName::AnotherLongName();
};

// Fixed None
void LongName::
  AnotherLongName();
class C {
   void LongName::
AnotherLongName();
};
 ```

https://github.com/llvm/llvm-project/pull/78011
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits