[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2024-01-10 Thread Owen Pan via cfe-commits

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


[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2024-01-09 Thread Gedare Bloom via cfe-commits

https://github.com/gedare updated 
https://github.com/llvm/llvm-project/pull/69340

>From b4c8809a948799be51a35b10e4d9d303b78a98db Mon Sep 17 00:00:00 2001
From: Gedare Bloom 
Date: Thu, 9 Nov 2023 09:30:24 -0700
Subject: [PATCH 01/10] Revert "Revert "[clang-format] Fix align consecutive
 declarations over function pointers""

This reverts commit 7bc1031c474ebb2216a5432273dafe4d1490fbce.
---
 clang/lib/Format/WhitespaceManager.cpp |  2 +-
 clang/unittests/Format/FormatTest.cpp  | 10 ++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/clang/lib/Format/WhitespaceManager.cpp 
b/clang/lib/Format/WhitespaceManager.cpp
index 3bc6915b8df0a7..4bf4699756f77d 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -979,7 +979,7 @@ void WhitespaceManager::alignConsecutiveDeclarations() {
   AlignTokens(
   Style,
   [](Change const ) {
-if (C.Tok->is(TT_FunctionDeclarationName))
+if (C.Tok->isOneOf(TT_FunctionDeclarationName, TT_FunctionTypeLParen))
   return true;
 if (C.Tok->isNot(TT_StartOfName))
   return false;
diff --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 25ef5c680af862..73ed6ee0f67bb2 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -2050,6 +2050,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int *d;\n"
"Const unsigned int \n"
"const unsigned int \n"
+   "int*f1(int *a, int , int &);\n"
+   "double *(*f2)(int *a, double &);\n"
"const unsigned&\n"
"Const unsigned  h;",
Style);
@@ -2095,6 +2097,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int* d;\n"
"Const unsigned int& e;\n"
"const unsigned int& f;\n"
+   "int*f1(int* a, int& b, int&& c);\n"
+   "double* (*f2)(int* a, double&& b);\n"
"const unsigned&&g;\n"
"Const unsigned  h;",
Style);
@@ -2120,6 +2124,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int *d;\n"
"Const unsigned int& e;\n"
"const unsigned int& f;\n"
+   "int*f1(int *a, int& b, int&& c);\n"
+   "double *(*f2)(int *a, double&& b);\n"
"const unsigned  g;\n"
"Const unsigned  h;",
Style);
@@ -2160,6 +2166,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int*  d;\n"
"Const unsigned int & e;\n"
"const unsigned int & f;\n"
+   "int* f1(int* a, int & b, int && c);\n"
+   "double*  (*f2)(int* a, double && b);\n"
"const unsigned &&g;\n"
"Const unsigned   h;",
Style);
@@ -2185,6 +2193,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int * d;\n"
"Const unsigned int  \n"
"const unsigned int  \n"
+   "int *f1(int * a, int , int &);\n"
+   "double * (*f2)(int * a, double &);\n"
"const unsigned &\n"
"Const unsigned   h;",
Style);

>From e5bcaf5f1062ab8a6f800a6473e544f0505accfc Mon Sep 17 00:00:00 2001
From: Gedare Bloom 
Date: Tue, 17 Oct 2023 08:21:55 -0600
Subject: [PATCH 02/10] Split alignment of declarations around assignment

Function pointers are detected as a type of declaration using
FunctionTypeLParen. They are aligned based on rules for
AlignConsecutiveDeclarations. When a function pointer is on the
right-hand side of an assignment, the alignment of the function pointer
can result in excessive whitespace padding due to the ordering of
alignment, as the alignment processes a line from left-to-right and
first aligns the declarations before and after the assignment operator,
and then aligns the assignment operator. Injection of whitespace by
alignment of declarations after the equal sign followed by alignment
of the equal sign results in the excessive whitespace.

Fixes #68079.
---
 clang/lib/Format/WhitespaceManager.cpp | 3 +++
 clang/unittests/Format/FormatTest.cpp  | 9 +
 2 files changed, 12 insertions(+)

diff --git a/clang/lib/Format/WhitespaceManager.cpp 
b/clang/lib/Format/WhitespaceManager.cpp
index 4bf4699756f77d..d210d4cfb0c6a6 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -979,6 +979,9 @@ void WhitespaceManager::alignConsecutiveDeclarations() {
   AlignTokens(
   Style,
   [](Change const ) {
+for 

[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2024-01-09 Thread Owen Pan via cfe-commits


@@ -1079,6 +1079,7 @@ clang-format
 - Add ``ObjCPropertyAttributeOrder`` which can be used to sort ObjC property
   attributes (like ``nonatomic, strong, nullable``).
 - Add ``.clang-format-ignore`` files.
+- Add ``AlignFunctionPointers`` sub-option for 
``AlignConsecutiveDeclarations''.

owenca wrote:

```suggestion
- Add ``AlignFunctionPointers`` sub-option for ``AlignConsecutiveDeclarations``.
```

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


[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2024-01-09 Thread Owen Pan via cfe-commits

https://github.com/owenca approved this pull request.


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


[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2024-01-09 Thread Gedare Bloom via cfe-commits

gedare wrote:

> Please also update the release notes. LGTM otherwise.

Fixed the comments and added a release note.

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


[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2024-01-09 Thread Gedare Bloom via cfe-commits

https://github.com/gedare updated 
https://github.com/llvm/llvm-project/pull/69340

>From b4c8809a948799be51a35b10e4d9d303b78a98db Mon Sep 17 00:00:00 2001
From: Gedare Bloom 
Date: Thu, 9 Nov 2023 09:30:24 -0700
Subject: [PATCH 1/9] Revert "Revert "[clang-format] Fix align consecutive
 declarations over function pointers""

This reverts commit 7bc1031c474ebb2216a5432273dafe4d1490fbce.
---
 clang/lib/Format/WhitespaceManager.cpp |  2 +-
 clang/unittests/Format/FormatTest.cpp  | 10 ++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/clang/lib/Format/WhitespaceManager.cpp 
b/clang/lib/Format/WhitespaceManager.cpp
index 3bc6915b8df0a7..4bf4699756f77d 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -979,7 +979,7 @@ void WhitespaceManager::alignConsecutiveDeclarations() {
   AlignTokens(
   Style,
   [](Change const ) {
-if (C.Tok->is(TT_FunctionDeclarationName))
+if (C.Tok->isOneOf(TT_FunctionDeclarationName, TT_FunctionTypeLParen))
   return true;
 if (C.Tok->isNot(TT_StartOfName))
   return false;
diff --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 25ef5c680af862..73ed6ee0f67bb2 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -2050,6 +2050,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int *d;\n"
"Const unsigned int \n"
"const unsigned int \n"
+   "int*f1(int *a, int , int &);\n"
+   "double *(*f2)(int *a, double &);\n"
"const unsigned&\n"
"Const unsigned  h;",
Style);
@@ -2095,6 +2097,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int* d;\n"
"Const unsigned int& e;\n"
"const unsigned int& f;\n"
+   "int*f1(int* a, int& b, int&& c);\n"
+   "double* (*f2)(int* a, double&& b);\n"
"const unsigned&&g;\n"
"Const unsigned  h;",
Style);
@@ -2120,6 +2124,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int *d;\n"
"Const unsigned int& e;\n"
"const unsigned int& f;\n"
+   "int*f1(int *a, int& b, int&& c);\n"
+   "double *(*f2)(int *a, double&& b);\n"
"const unsigned  g;\n"
"Const unsigned  h;",
Style);
@@ -2160,6 +2166,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int*  d;\n"
"Const unsigned int & e;\n"
"const unsigned int & f;\n"
+   "int* f1(int* a, int & b, int && c);\n"
+   "double*  (*f2)(int* a, double && b);\n"
"const unsigned &&g;\n"
"Const unsigned   h;",
Style);
@@ -2185,6 +2193,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int * d;\n"
"Const unsigned int  \n"
"const unsigned int  \n"
+   "int *f1(int * a, int , int &);\n"
+   "double * (*f2)(int * a, double &);\n"
"const unsigned &\n"
"Const unsigned   h;",
Style);

>From e5bcaf5f1062ab8a6f800a6473e544f0505accfc Mon Sep 17 00:00:00 2001
From: Gedare Bloom 
Date: Tue, 17 Oct 2023 08:21:55 -0600
Subject: [PATCH 2/9] Split alignment of declarations around assignment

Function pointers are detected as a type of declaration using
FunctionTypeLParen. They are aligned based on rules for
AlignConsecutiveDeclarations. When a function pointer is on the
right-hand side of an assignment, the alignment of the function pointer
can result in excessive whitespace padding due to the ordering of
alignment, as the alignment processes a line from left-to-right and
first aligns the declarations before and after the assignment operator,
and then aligns the assignment operator. Injection of whitespace by
alignment of declarations after the equal sign followed by alignment
of the equal sign results in the excessive whitespace.

Fixes #68079.
---
 clang/lib/Format/WhitespaceManager.cpp | 3 +++
 clang/unittests/Format/FormatTest.cpp  | 9 +
 2 files changed, 12 insertions(+)

diff --git a/clang/lib/Format/WhitespaceManager.cpp 
b/clang/lib/Format/WhitespaceManager.cpp
index 4bf4699756f77d..d210d4cfb0c6a6 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -979,6 +979,9 @@ void WhitespaceManager::alignConsecutiveDeclarations() {
   AlignTokens(
   Style,
   [](Change const ) {
+for 

[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2024-01-06 Thread Owen Pan via cfe-commits


@@ -76,40 +76,49 @@ template <> struct 
MappingTraits {
 FormatStyle::AlignConsecutiveStyle(
 {/*Enabled=*/false, /*AcrossEmptyLines=*/false,
  /*AcrossComments=*/false, /*AlignCompound=*/false,
+ /*AlignFunctionPointers=*/false,
  /*PadOperators=*/true}));

owenca wrote:

```suggestion
 /*AlignFunctionPointers=*/false, /*PadOperators=*/true}));
```

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


[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2024-01-06 Thread Owen Pan via cfe-commits


@@ -978,7 +978,14 @@ void WhitespaceManager::alignConsecutiveDeclarations() {
 
   AlignTokens(
   Style,
-  [](Change const ) {
+  [&](Change const ) {
+if (Style.AlignConsecutiveDeclarations.AlignFunctionPointers) {
+  for (FormatToken *Prev = C.Tok->Previous; Prev; Prev = 
Prev->Previous)

owenca wrote:

```suggestion
  for (const auto *Prev = C.Tok->Previous; Prev; Prev = Prev->Previous)
```

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


[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2024-01-06 Thread Owen Pan via cfe-commits


@@ -225,6 +225,22 @@ struct FormatStyle {
 ///   bbb = 2;
 /// \endcode
 bool AlignCompound;
+/// Only for ``AlignConsecutiveDeclarations``. Whether function pointers
+/// are aligned.

owenca wrote:

```suggestion
/// Only for ``AlignConsecutiveDeclarations``. Whether function pointers are
/// aligned.
```
Please regenerate the rst file after reflowing the comment.

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


[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2024-01-06 Thread Owen Pan via cfe-commits


@@ -76,40 +76,49 @@ template <> struct 
MappingTraits {
 FormatStyle::AlignConsecutiveStyle(
 {/*Enabled=*/false, /*AcrossEmptyLines=*/false,
  /*AcrossComments=*/false, /*AlignCompound=*/false,
+ /*AlignFunctionPointers=*/false,
  /*PadOperators=*/true}));
 IO.enumCase(Value, "Consecutive",
 FormatStyle::AlignConsecutiveStyle(
 {/*Enabled=*/true, /*AcrossEmptyLines=*/false,
  /*AcrossComments=*/false, /*AlignCompound=*/false,
+ /*AlignFunctionPointers=*/false,
  /*PadOperators=*/true}));

owenca wrote:

Ditto.

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


[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2024-01-06 Thread Owen Pan via cfe-commits

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


[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2024-01-06 Thread Owen Pan via cfe-commits


@@ -76,40 +76,49 @@ template <> struct 
MappingTraits {
 FormatStyle::AlignConsecutiveStyle(
 {/*Enabled=*/false, /*AcrossEmptyLines=*/false,
  /*AcrossComments=*/false, /*AlignCompound=*/false,
+ /*AlignFunctionPointers=*/false,
  /*PadOperators=*/true}));
 IO.enumCase(Value, "Consecutive",
 FormatStyle::AlignConsecutiveStyle(
 {/*Enabled=*/true, /*AcrossEmptyLines=*/false,
  /*AcrossComments=*/false, /*AlignCompound=*/false,
+ /*AlignFunctionPointers=*/false,
  /*PadOperators=*/true}));
 IO.enumCase(Value, "AcrossEmptyLines",
 FormatStyle::AlignConsecutiveStyle(
 {/*Enabled=*/true, /*AcrossEmptyLines=*/true,
  /*AcrossComments=*/false, /*AlignCompound=*/false,
+ /*AlignFunctionPointers=*/false,
  /*PadOperators=*/true}));
-IO.enumCase(Value, "AcrossComments",
-FormatStyle::AlignConsecutiveStyle({/*Enabled=*/true,
-/*AcrossEmptyLines=*/false,
-/*AcrossComments=*/true,
-/*AlignCompound=*/false,
-/*PadOperators=*/true}));
-IO.enumCase(Value, "AcrossEmptyLinesAndComments",
-FormatStyle::AlignConsecutiveStyle({/*Enabled=*/true,
-/*AcrossEmptyLines=*/true,
-/*AcrossComments=*/true,
-/*AlignCompound=*/false,
-/*PadOperators=*/true}));
+IO.enumCase(
+Value, "AcrossComments",
+FormatStyle::AlignConsecutiveStyle({/*Enabled=*/true,
+/*AcrossEmptyLines=*/false,
+/*AcrossComments=*/true,
+/*AlignCompound=*/false,
+/*AlignFunctionPointers=*/false,
+/*PadOperators=*/true}));
+IO.enumCase(
+Value, "AcrossEmptyLinesAndComments",
+FormatStyle::AlignConsecutiveStyle({/*Enabled=*/true,
+/*AcrossEmptyLines=*/true,
+/*AcrossComments=*/true,
+/*AlignCompound=*/false,
+/*AlignFunctionPointers=*/false,
+/*PadOperators=*/true}));
 
 // For backward compatibility.
 IO.enumCase(Value, "true",
 FormatStyle::AlignConsecutiveStyle(
 {/*Enabled=*/true, /*AcrossEmptyLines=*/false,
  /*AcrossComments=*/false, /*AlignCompound=*/false,
+ /*AlignFunctionPointers=*/false,
  /*PadOperators=*/true}));

owenca wrote:

Ditto.

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


[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2024-01-06 Thread Owen Pan via cfe-commits


@@ -76,40 +76,49 @@ template <> struct 
MappingTraits {
 FormatStyle::AlignConsecutiveStyle(
 {/*Enabled=*/false, /*AcrossEmptyLines=*/false,
  /*AcrossComments=*/false, /*AlignCompound=*/false,
+ /*AlignFunctionPointers=*/false,
  /*PadOperators=*/true}));
 IO.enumCase(Value, "Consecutive",
 FormatStyle::AlignConsecutiveStyle(
 {/*Enabled=*/true, /*AcrossEmptyLines=*/false,
  /*AcrossComments=*/false, /*AlignCompound=*/false,
+ /*AlignFunctionPointers=*/false,
  /*PadOperators=*/true}));
 IO.enumCase(Value, "AcrossEmptyLines",
 FormatStyle::AlignConsecutiveStyle(
 {/*Enabled=*/true, /*AcrossEmptyLines=*/true,
  /*AcrossComments=*/false, /*AlignCompound=*/false,
+ /*AlignFunctionPointers=*/false,
  /*PadOperators=*/true}));
-IO.enumCase(Value, "AcrossComments",
-FormatStyle::AlignConsecutiveStyle({/*Enabled=*/true,
-/*AcrossEmptyLines=*/false,
-/*AcrossComments=*/true,
-/*AlignCompound=*/false,
-/*PadOperators=*/true}));
-IO.enumCase(Value, "AcrossEmptyLinesAndComments",
-FormatStyle::AlignConsecutiveStyle({/*Enabled=*/true,
-/*AcrossEmptyLines=*/true,
-/*AcrossComments=*/true,
-/*AlignCompound=*/false,
-/*PadOperators=*/true}));
+IO.enumCase(
+Value, "AcrossComments",
+FormatStyle::AlignConsecutiveStyle({/*Enabled=*/true,
+/*AcrossEmptyLines=*/false,
+/*AcrossComments=*/true,
+/*AlignCompound=*/false,
+/*AlignFunctionPointers=*/false,
+/*PadOperators=*/true}));
+IO.enumCase(
+Value, "AcrossEmptyLinesAndComments",
+FormatStyle::AlignConsecutiveStyle({/*Enabled=*/true,
+/*AcrossEmptyLines=*/true,
+/*AcrossComments=*/true,
+/*AlignCompound=*/false,
+/*AlignFunctionPointers=*/false,
+/*PadOperators=*/true}));
 
 // For backward compatibility.
 IO.enumCase(Value, "true",
 FormatStyle::AlignConsecutiveStyle(
 {/*Enabled=*/true, /*AcrossEmptyLines=*/false,
  /*AcrossComments=*/false, /*AlignCompound=*/false,
+ /*AlignFunctionPointers=*/false,
  /*PadOperators=*/true}));
 IO.enumCase(Value, "false",
 FormatStyle::AlignConsecutiveStyle(
 {/*Enabled=*/false, /*AcrossEmptyLines=*/false,
  /*AcrossComments=*/false, /*AlignCompound=*/false,
+ /*AlignFunctionPointers=*/false,
  /*PadOperators=*/true}));

owenca wrote:

Ditto.

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


[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2024-01-06 Thread Owen Pan via cfe-commits


@@ -76,40 +76,49 @@ template <> struct 
MappingTraits {
 FormatStyle::AlignConsecutiveStyle(
 {/*Enabled=*/false, /*AcrossEmptyLines=*/false,
  /*AcrossComments=*/false, /*AlignCompound=*/false,
+ /*AlignFunctionPointers=*/false,
  /*PadOperators=*/true}));
 IO.enumCase(Value, "Consecutive",
 FormatStyle::AlignConsecutiveStyle(
 {/*Enabled=*/true, /*AcrossEmptyLines=*/false,
  /*AcrossComments=*/false, /*AlignCompound=*/false,
+ /*AlignFunctionPointers=*/false,
  /*PadOperators=*/true}));
 IO.enumCase(Value, "AcrossEmptyLines",
 FormatStyle::AlignConsecutiveStyle(
 {/*Enabled=*/true, /*AcrossEmptyLines=*/true,
  /*AcrossComments=*/false, /*AlignCompound=*/false,
+ /*AlignFunctionPointers=*/false,
  /*PadOperators=*/true}));

owenca wrote:

Ditto.

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


[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2024-01-06 Thread Owen Pan via cfe-commits

https://github.com/owenca commented:

Please also update the release notes. LGTM otherwise.

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


[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2024-01-05 Thread Gedare Bloom via cfe-commits

gedare wrote:

> > @owenca this waits for re-review
> 
> Can you rebase the patch?

done

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


[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2024-01-05 Thread Gedare Bloom via cfe-commits

https://github.com/gedare updated 
https://github.com/llvm/llvm-project/pull/69340

>From 65e3a69e30810683ba76d9a233a9c408fd121120 Mon Sep 17 00:00:00 2001
From: Gedare Bloom 
Date: Thu, 9 Nov 2023 09:30:24 -0700
Subject: [PATCH 1/7] Revert "Revert "[clang-format] Fix align consecutive
 declarations over function pointers""

This reverts commit 7bc1031c474ebb2216a5432273dafe4d1490fbce.
---
 clang/lib/Format/WhitespaceManager.cpp |  2 +-
 clang/unittests/Format/FormatTest.cpp  | 10 ++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/clang/lib/Format/WhitespaceManager.cpp 
b/clang/lib/Format/WhitespaceManager.cpp
index 3bc6915b8df0a7..4bf4699756f77d 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -979,7 +979,7 @@ void WhitespaceManager::alignConsecutiveDeclarations() {
   AlignTokens(
   Style,
   [](Change const ) {
-if (C.Tok->is(TT_FunctionDeclarationName))
+if (C.Tok->isOneOf(TT_FunctionDeclarationName, TT_FunctionTypeLParen))
   return true;
 if (C.Tok->isNot(TT_StartOfName))
   return false;
diff --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 881993ede17c3d..5eef94332e21dd 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -2050,6 +2050,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int *d;\n"
"Const unsigned int \n"
"const unsigned int \n"
+   "int*f1(int *a, int , int &);\n"
+   "double *(*f2)(int *a, double &);\n"
"const unsigned&\n"
"Const unsigned  h;",
Style);
@@ -2095,6 +2097,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int* d;\n"
"Const unsigned int& e;\n"
"const unsigned int& f;\n"
+   "int*f1(int* a, int& b, int&& c);\n"
+   "double* (*f2)(int* a, double&& b);\n"
"const unsigned&&g;\n"
"Const unsigned  h;",
Style);
@@ -2120,6 +2124,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int *d;\n"
"Const unsigned int& e;\n"
"const unsigned int& f;\n"
+   "int*f1(int *a, int& b, int&& c);\n"
+   "double *(*f2)(int *a, double&& b);\n"
"const unsigned  g;\n"
"Const unsigned  h;",
Style);
@@ -2160,6 +2166,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int*  d;\n"
"Const unsigned int & e;\n"
"const unsigned int & f;\n"
+   "int* f1(int* a, int & b, int && c);\n"
+   "double*  (*f2)(int* a, double && b);\n"
"const unsigned &&g;\n"
"Const unsigned   h;",
Style);
@@ -2185,6 +2193,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int * d;\n"
"Const unsigned int  \n"
"const unsigned int  \n"
+   "int *f1(int * a, int , int &);\n"
+   "double * (*f2)(int * a, double &);\n"
"const unsigned &\n"
"Const unsigned   h;",
Style);

>From 72a7b92bafcffad7e3664384fa27967b1e7bc8c0 Mon Sep 17 00:00:00 2001
From: Gedare Bloom 
Date: Tue, 17 Oct 2023 08:21:55 -0600
Subject: [PATCH 2/7] Split alignment of declarations around assignment

Function pointers are detected as a type of declaration using
FunctionTypeLParen. They are aligned based on rules for
AlignConsecutiveDeclarations. When a function pointer is on the
right-hand side of an assignment, the alignment of the function pointer
can result in excessive whitespace padding due to the ordering of
alignment, as the alignment processes a line from left-to-right and
first aligns the declarations before and after the assignment operator,
and then aligns the assignment operator. Injection of whitespace by
alignment of declarations after the equal sign followed by alignment
of the equal sign results in the excessive whitespace.

Fixes #68079.
---
 clang/lib/Format/WhitespaceManager.cpp | 3 +++
 clang/unittests/Format/FormatTest.cpp  | 9 +
 2 files changed, 12 insertions(+)

diff --git a/clang/lib/Format/WhitespaceManager.cpp 
b/clang/lib/Format/WhitespaceManager.cpp
index 4bf4699756f77d..d210d4cfb0c6a6 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -979,6 +979,9 @@ void WhitespaceManager::alignConsecutiveDeclarations() {
   AlignTokens(
   Style,
   [](Change const ) {
+for 

[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2024-01-05 Thread Owen Pan via cfe-commits

owenca wrote:

> @owenca this waits for re-review

Can you rebase the patch?

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


[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2024-01-05 Thread Gedare Bloom via cfe-commits

gedare wrote:

@owenca this waits for re-review 

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


[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2023-12-19 Thread Gedare Bloom via cfe-commits

https://github.com/gedare updated 
https://github.com/llvm/llvm-project/pull/69340

>From 610d0b544d7927af93b6943078df033f154b74f8 Mon Sep 17 00:00:00 2001
From: Gedare Bloom 
Date: Thu, 9 Nov 2023 09:30:24 -0700
Subject: [PATCH 1/7] Revert "Revert "[clang-format] Fix align consecutive
 declarations over function pointers""

This reverts commit 7bc1031c474ebb2216a5432273dafe4d1490fbce.
---
 clang/lib/Format/WhitespaceManager.cpp |  2 +-
 clang/unittests/Format/FormatTest.cpp  | 10 ++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/clang/lib/Format/WhitespaceManager.cpp 
b/clang/lib/Format/WhitespaceManager.cpp
index ff8b1e6e13a3f7..d4d66f0a9b2ab9 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -980,7 +980,7 @@ void WhitespaceManager::alignConsecutiveDeclarations() {
   AlignTokens(
   Style,
   [](Change const ) {
-if (C.Tok->is(TT_FunctionDeclarationName))
+if (C.Tok->isOneOf(TT_FunctionDeclarationName, TT_FunctionTypeLParen))
   return true;
 if (C.Tok->isNot(TT_StartOfName))
   return false;
diff --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 80903e7630c807..19de60a5a4031d 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -2047,6 +2047,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int *d;\n"
"Const unsigned int \n"
"const unsigned int \n"
+   "int*f1(int *a, int , int &);\n"
+   "double *(*f2)(int *a, double &);\n"
"const unsigned&\n"
"Const unsigned  h;",
Style);
@@ -2092,6 +2094,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int* d;\n"
"Const unsigned int& e;\n"
"const unsigned int& f;\n"
+   "int*f1(int* a, int& b, int&& c);\n"
+   "double* (*f2)(int* a, double&& b);\n"
"const unsigned&&g;\n"
"Const unsigned  h;",
Style);
@@ -2117,6 +2121,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int *d;\n"
"Const unsigned int& e;\n"
"const unsigned int& f;\n"
+   "int*f1(int *a, int& b, int&& c);\n"
+   "double *(*f2)(int *a, double&& b);\n"
"const unsigned  g;\n"
"Const unsigned  h;",
Style);
@@ -2157,6 +2163,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int*  d;\n"
"Const unsigned int & e;\n"
"const unsigned int & f;\n"
+   "int* f1(int* a, int & b, int && c);\n"
+   "double*  (*f2)(int* a, double && b);\n"
"const unsigned &&g;\n"
"Const unsigned   h;",
Style);
@@ -2182,6 +2190,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int * d;\n"
"Const unsigned int  \n"
"const unsigned int  \n"
+   "int *f1(int * a, int , int &);\n"
+   "double * (*f2)(int * a, double &);\n"
"const unsigned &\n"
"Const unsigned   h;",
Style);

>From bb386b5e912686fb292fb0de89e124fa54076aa7 Mon Sep 17 00:00:00 2001
From: Gedare Bloom 
Date: Tue, 17 Oct 2023 08:21:55 -0600
Subject: [PATCH 2/7] Split alignment of declarations around assignment

Function pointers are detected as a type of declaration using
FunctionTypeLParen. They are aligned based on rules for
AlignConsecutiveDeclarations. When a function pointer is on the
right-hand side of an assignment, the alignment of the function pointer
can result in excessive whitespace padding due to the ordering of
alignment, as the alignment processes a line from left-to-right and
first aligns the declarations before and after the assignment operator,
and then aligns the assignment operator. Injection of whitespace by
alignment of declarations after the equal sign followed by alignment
of the equal sign results in the excessive whitespace.

Fixes #68079.
---
 clang/lib/Format/WhitespaceManager.cpp | 3 +++
 clang/unittests/Format/FormatTest.cpp  | 9 +
 2 files changed, 12 insertions(+)

diff --git a/clang/lib/Format/WhitespaceManager.cpp 
b/clang/lib/Format/WhitespaceManager.cpp
index d4d66f0a9b2ab9..294a87a6759212 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -980,6 +980,9 @@ void WhitespaceManager::alignConsecutiveDeclarations() {
   AlignTokens(
   Style,
   [](Change const ) {
+for 

[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2023-12-19 Thread Gedare Bloom via cfe-commits

https://github.com/gedare updated 
https://github.com/llvm/llvm-project/pull/69340

>From 610d0b544d7927af93b6943078df033f154b74f8 Mon Sep 17 00:00:00 2001
From: Gedare Bloom 
Date: Thu, 9 Nov 2023 09:30:24 -0700
Subject: [PATCH 1/6] Revert "Revert "[clang-format] Fix align consecutive
 declarations over function pointers""

This reverts commit 7bc1031c474ebb2216a5432273dafe4d1490fbce.
---
 clang/lib/Format/WhitespaceManager.cpp |  2 +-
 clang/unittests/Format/FormatTest.cpp  | 10 ++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/clang/lib/Format/WhitespaceManager.cpp 
b/clang/lib/Format/WhitespaceManager.cpp
index ff8b1e6e13a3f7..d4d66f0a9b2ab9 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -980,7 +980,7 @@ void WhitespaceManager::alignConsecutiveDeclarations() {
   AlignTokens(
   Style,
   [](Change const ) {
-if (C.Tok->is(TT_FunctionDeclarationName))
+if (C.Tok->isOneOf(TT_FunctionDeclarationName, TT_FunctionTypeLParen))
   return true;
 if (C.Tok->isNot(TT_StartOfName))
   return false;
diff --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 80903e7630c807..19de60a5a4031d 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -2047,6 +2047,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int *d;\n"
"Const unsigned int \n"
"const unsigned int \n"
+   "int*f1(int *a, int , int &);\n"
+   "double *(*f2)(int *a, double &);\n"
"const unsigned&\n"
"Const unsigned  h;",
Style);
@@ -2092,6 +2094,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int* d;\n"
"Const unsigned int& e;\n"
"const unsigned int& f;\n"
+   "int*f1(int* a, int& b, int&& c);\n"
+   "double* (*f2)(int* a, double&& b);\n"
"const unsigned&&g;\n"
"Const unsigned  h;",
Style);
@@ -2117,6 +2121,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int *d;\n"
"Const unsigned int& e;\n"
"const unsigned int& f;\n"
+   "int*f1(int *a, int& b, int&& c);\n"
+   "double *(*f2)(int *a, double&& b);\n"
"const unsigned  g;\n"
"Const unsigned  h;",
Style);
@@ -2157,6 +2163,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int*  d;\n"
"Const unsigned int & e;\n"
"const unsigned int & f;\n"
+   "int* f1(int* a, int & b, int && c);\n"
+   "double*  (*f2)(int* a, double && b);\n"
"const unsigned &&g;\n"
"Const unsigned   h;",
Style);
@@ -2182,6 +2190,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int * d;\n"
"Const unsigned int  \n"
"const unsigned int  \n"
+   "int *f1(int * a, int , int &);\n"
+   "double * (*f2)(int * a, double &);\n"
"const unsigned &\n"
"Const unsigned   h;",
Style);

>From bb386b5e912686fb292fb0de89e124fa54076aa7 Mon Sep 17 00:00:00 2001
From: Gedare Bloom 
Date: Tue, 17 Oct 2023 08:21:55 -0600
Subject: [PATCH 2/6] Split alignment of declarations around assignment

Function pointers are detected as a type of declaration using
FunctionTypeLParen. They are aligned based on rules for
AlignConsecutiveDeclarations. When a function pointer is on the
right-hand side of an assignment, the alignment of the function pointer
can result in excessive whitespace padding due to the ordering of
alignment, as the alignment processes a line from left-to-right and
first aligns the declarations before and after the assignment operator,
and then aligns the assignment operator. Injection of whitespace by
alignment of declarations after the equal sign followed by alignment
of the equal sign results in the excessive whitespace.

Fixes #68079.
---
 clang/lib/Format/WhitespaceManager.cpp | 3 +++
 clang/unittests/Format/FormatTest.cpp  | 9 +
 2 files changed, 12 insertions(+)

diff --git a/clang/lib/Format/WhitespaceManager.cpp 
b/clang/lib/Format/WhitespaceManager.cpp
index d4d66f0a9b2ab9..294a87a6759212 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -980,6 +980,9 @@ void WhitespaceManager::alignConsecutiveDeclarations() {
   AlignTokens(
   Style,
   [](Change const ) {
+for 

[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2023-12-19 Thread Gedare Bloom via cfe-commits

https://github.com/gedare updated 
https://github.com/llvm/llvm-project/pull/69340

>From 610d0b544d7927af93b6943078df033f154b74f8 Mon Sep 17 00:00:00 2001
From: Gedare Bloom 
Date: Thu, 9 Nov 2023 09:30:24 -0700
Subject: [PATCH 1/5] Revert "Revert "[clang-format] Fix align consecutive
 declarations over function pointers""

This reverts commit 7bc1031c474ebb2216a5432273dafe4d1490fbce.
---
 clang/lib/Format/WhitespaceManager.cpp |  2 +-
 clang/unittests/Format/FormatTest.cpp  | 10 ++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/clang/lib/Format/WhitespaceManager.cpp 
b/clang/lib/Format/WhitespaceManager.cpp
index ff8b1e6e13a3f7..d4d66f0a9b2ab9 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -980,7 +980,7 @@ void WhitespaceManager::alignConsecutiveDeclarations() {
   AlignTokens(
   Style,
   [](Change const ) {
-if (C.Tok->is(TT_FunctionDeclarationName))
+if (C.Tok->isOneOf(TT_FunctionDeclarationName, TT_FunctionTypeLParen))
   return true;
 if (C.Tok->isNot(TT_StartOfName))
   return false;
diff --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 80903e7630c807..19de60a5a4031d 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -2047,6 +2047,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int *d;\n"
"Const unsigned int \n"
"const unsigned int \n"
+   "int*f1(int *a, int , int &);\n"
+   "double *(*f2)(int *a, double &);\n"
"const unsigned&\n"
"Const unsigned  h;",
Style);
@@ -2092,6 +2094,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int* d;\n"
"Const unsigned int& e;\n"
"const unsigned int& f;\n"
+   "int*f1(int* a, int& b, int&& c);\n"
+   "double* (*f2)(int* a, double&& b);\n"
"const unsigned&&g;\n"
"Const unsigned  h;",
Style);
@@ -2117,6 +2121,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int *d;\n"
"Const unsigned int& e;\n"
"const unsigned int& f;\n"
+   "int*f1(int *a, int& b, int&& c);\n"
+   "double *(*f2)(int *a, double&& b);\n"
"const unsigned  g;\n"
"Const unsigned  h;",
Style);
@@ -2157,6 +2163,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int*  d;\n"
"Const unsigned int & e;\n"
"const unsigned int & f;\n"
+   "int* f1(int* a, int & b, int && c);\n"
+   "double*  (*f2)(int* a, double && b);\n"
"const unsigned &&g;\n"
"Const unsigned   h;",
Style);
@@ -2182,6 +2190,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int * d;\n"
"Const unsigned int  \n"
"const unsigned int  \n"
+   "int *f1(int * a, int , int &);\n"
+   "double * (*f2)(int * a, double &);\n"
"const unsigned &\n"
"Const unsigned   h;",
Style);

>From bb386b5e912686fb292fb0de89e124fa54076aa7 Mon Sep 17 00:00:00 2001
From: Gedare Bloom 
Date: Tue, 17 Oct 2023 08:21:55 -0600
Subject: [PATCH 2/5] Split alignment of declarations around assignment

Function pointers are detected as a type of declaration using
FunctionTypeLParen. They are aligned based on rules for
AlignConsecutiveDeclarations. When a function pointer is on the
right-hand side of an assignment, the alignment of the function pointer
can result in excessive whitespace padding due to the ordering of
alignment, as the alignment processes a line from left-to-right and
first aligns the declarations before and after the assignment operator,
and then aligns the assignment operator. Injection of whitespace by
alignment of declarations after the equal sign followed by alignment
of the equal sign results in the excessive whitespace.

Fixes #68079.
---
 clang/lib/Format/WhitespaceManager.cpp | 3 +++
 clang/unittests/Format/FormatTest.cpp  | 9 +
 2 files changed, 12 insertions(+)

diff --git a/clang/lib/Format/WhitespaceManager.cpp 
b/clang/lib/Format/WhitespaceManager.cpp
index d4d66f0a9b2ab9..294a87a6759212 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -980,6 +980,9 @@ void WhitespaceManager::alignConsecutiveDeclarations() {
   AlignTokens(
   Style,
   [](Change const ) {
+for 

[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2023-12-19 Thread Gedare Bloom via cfe-commits

https://github.com/gedare updated 
https://github.com/llvm/llvm-project/pull/69340

>From 610d0b544d7927af93b6943078df033f154b74f8 Mon Sep 17 00:00:00 2001
From: Gedare Bloom 
Date: Thu, 9 Nov 2023 09:30:24 -0700
Subject: [PATCH 1/4] Revert "Revert "[clang-format] Fix align consecutive
 declarations over function pointers""

This reverts commit 7bc1031c474ebb2216a5432273dafe4d1490fbce.
---
 clang/lib/Format/WhitespaceManager.cpp |  2 +-
 clang/unittests/Format/FormatTest.cpp  | 10 ++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/clang/lib/Format/WhitespaceManager.cpp 
b/clang/lib/Format/WhitespaceManager.cpp
index ff8b1e6e13a3f7..d4d66f0a9b2ab9 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -980,7 +980,7 @@ void WhitespaceManager::alignConsecutiveDeclarations() {
   AlignTokens(
   Style,
   [](Change const ) {
-if (C.Tok->is(TT_FunctionDeclarationName))
+if (C.Tok->isOneOf(TT_FunctionDeclarationName, TT_FunctionTypeLParen))
   return true;
 if (C.Tok->isNot(TT_StartOfName))
   return false;
diff --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 80903e7630c807..19de60a5a4031d 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -2047,6 +2047,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int *d;\n"
"Const unsigned int \n"
"const unsigned int \n"
+   "int*f1(int *a, int , int &);\n"
+   "double *(*f2)(int *a, double &);\n"
"const unsigned&\n"
"Const unsigned  h;",
Style);
@@ -2092,6 +2094,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int* d;\n"
"Const unsigned int& e;\n"
"const unsigned int& f;\n"
+   "int*f1(int* a, int& b, int&& c);\n"
+   "double* (*f2)(int* a, double&& b);\n"
"const unsigned&&g;\n"
"Const unsigned  h;",
Style);
@@ -2117,6 +2121,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int *d;\n"
"Const unsigned int& e;\n"
"const unsigned int& f;\n"
+   "int*f1(int *a, int& b, int&& c);\n"
+   "double *(*f2)(int *a, double&& b);\n"
"const unsigned  g;\n"
"Const unsigned  h;",
Style);
@@ -2157,6 +2163,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int*  d;\n"
"Const unsigned int & e;\n"
"const unsigned int & f;\n"
+   "int* f1(int* a, int & b, int && c);\n"
+   "double*  (*f2)(int* a, double && b);\n"
"const unsigned &&g;\n"
"Const unsigned   h;",
Style);
@@ -2182,6 +2190,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int * d;\n"
"Const unsigned int  \n"
"const unsigned int  \n"
+   "int *f1(int * a, int , int &);\n"
+   "double * (*f2)(int * a, double &);\n"
"const unsigned &\n"
"Const unsigned   h;",
Style);

>From bb386b5e912686fb292fb0de89e124fa54076aa7 Mon Sep 17 00:00:00 2001
From: Gedare Bloom 
Date: Tue, 17 Oct 2023 08:21:55 -0600
Subject: [PATCH 2/4] Split alignment of declarations around assignment

Function pointers are detected as a type of declaration using
FunctionTypeLParen. They are aligned based on rules for
AlignConsecutiveDeclarations. When a function pointer is on the
right-hand side of an assignment, the alignment of the function pointer
can result in excessive whitespace padding due to the ordering of
alignment, as the alignment processes a line from left-to-right and
first aligns the declarations before and after the assignment operator,
and then aligns the assignment operator. Injection of whitespace by
alignment of declarations after the equal sign followed by alignment
of the equal sign results in the excessive whitespace.

Fixes #68079.
---
 clang/lib/Format/WhitespaceManager.cpp | 3 +++
 clang/unittests/Format/FormatTest.cpp  | 9 +
 2 files changed, 12 insertions(+)

diff --git a/clang/lib/Format/WhitespaceManager.cpp 
b/clang/lib/Format/WhitespaceManager.cpp
index d4d66f0a9b2ab9..294a87a6759212 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -980,6 +980,9 @@ void WhitespaceManager::alignConsecutiveDeclarations() {
   AlignTokens(
   Style,
   [](Change const ) {
+for 

[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2023-11-17 Thread via cfe-commits

https://github.com/mydeveloperday approved this pull request.


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


[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2023-11-09 Thread Gedare Bloom via cfe-commits

gedare wrote:

> See [#66857 
> (comment)](https://github.com/llvm/llvm-project/issues/66857#issuecomment-1793022188).
>  Can you make this a suboption, e.g. `AlignFunctionPointers`? We can add 
> another (e.g. `AlignConstructorsDestructors`) for ctors and dtors in the 
> future.

Yes, I will work on this.

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


[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2023-11-09 Thread Owen Pan via cfe-commits

https://github.com/owenca commented:

See https://github.com/llvm/llvm-project/issues/66857#issuecomment-1793022188. 
Can you make this a suboption, e.g. `AlignFunctionPointers`? We can add another 
(e.g. `AlignConstructorsDestructors`) for ctors and dtors in the future.

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


[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2023-11-09 Thread Gedare Bloom via cfe-commits

https://github.com/gedare updated 
https://github.com/llvm/llvm-project/pull/69340

>From 610d0b544d7927af93b6943078df033f154b74f8 Mon Sep 17 00:00:00 2001
From: Gedare Bloom 
Date: Thu, 9 Nov 2023 09:30:24 -0700
Subject: [PATCH 1/3] Revert "Revert "[clang-format] Fix align consecutive
 declarations over function pointers""

This reverts commit 7bc1031c474ebb2216a5432273dafe4d1490fbce.
---
 clang/lib/Format/WhitespaceManager.cpp |  2 +-
 clang/unittests/Format/FormatTest.cpp  | 10 ++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/clang/lib/Format/WhitespaceManager.cpp 
b/clang/lib/Format/WhitespaceManager.cpp
index ff8b1e6e13a3f77..d4d66f0a9b2ab95 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -980,7 +980,7 @@ void WhitespaceManager::alignConsecutiveDeclarations() {
   AlignTokens(
   Style,
   [](Change const ) {
-if (C.Tok->is(TT_FunctionDeclarationName))
+if (C.Tok->isOneOf(TT_FunctionDeclarationName, TT_FunctionTypeLParen))
   return true;
 if (C.Tok->isNot(TT_StartOfName))
   return false;
diff --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 80903e7630c8073..19de60a5a4031d0 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -2047,6 +2047,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int *d;\n"
"Const unsigned int \n"
"const unsigned int \n"
+   "int*f1(int *a, int , int &);\n"
+   "double *(*f2)(int *a, double &);\n"
"const unsigned&\n"
"Const unsigned  h;",
Style);
@@ -2092,6 +2094,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int* d;\n"
"Const unsigned int& e;\n"
"const unsigned int& f;\n"
+   "int*f1(int* a, int& b, int&& c);\n"
+   "double* (*f2)(int* a, double&& b);\n"
"const unsigned&&g;\n"
"Const unsigned  h;",
Style);
@@ -2117,6 +2121,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int *d;\n"
"Const unsigned int& e;\n"
"const unsigned int& f;\n"
+   "int*f1(int *a, int& b, int&& c);\n"
+   "double *(*f2)(int *a, double&& b);\n"
"const unsigned  g;\n"
"Const unsigned  h;",
Style);
@@ -2157,6 +2163,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int*  d;\n"
"Const unsigned int & e;\n"
"const unsigned int & f;\n"
+   "int* f1(int* a, int & b, int && c);\n"
+   "double*  (*f2)(int* a, double && b);\n"
"const unsigned &&g;\n"
"Const unsigned   h;",
Style);
@@ -2182,6 +2190,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int * d;\n"
"Const unsigned int  \n"
"const unsigned int  \n"
+   "int *f1(int * a, int , int &);\n"
+   "double * (*f2)(int * a, double &);\n"
"const unsigned &\n"
"Const unsigned   h;",
Style);

>From bb386b5e912686fb292fb0de89e124fa54076aa7 Mon Sep 17 00:00:00 2001
From: Gedare Bloom 
Date: Tue, 17 Oct 2023 08:21:55 -0600
Subject: [PATCH 2/3] Split alignment of declarations around assignment

Function pointers are detected as a type of declaration using
FunctionTypeLParen. They are aligned based on rules for
AlignConsecutiveDeclarations. When a function pointer is on the
right-hand side of an assignment, the alignment of the function pointer
can result in excessive whitespace padding due to the ordering of
alignment, as the alignment processes a line from left-to-right and
first aligns the declarations before and after the assignment operator,
and then aligns the assignment operator. Injection of whitespace by
alignment of declarations after the equal sign followed by alignment
of the equal sign results in the excessive whitespace.

Fixes #68079.
---
 clang/lib/Format/WhitespaceManager.cpp | 3 +++
 clang/unittests/Format/FormatTest.cpp  | 9 +
 2 files changed, 12 insertions(+)

diff --git a/clang/lib/Format/WhitespaceManager.cpp 
b/clang/lib/Format/WhitespaceManager.cpp
index d4d66f0a9b2ab95..294a87a6759212c 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -980,6 +980,9 @@ void WhitespaceManager::alignConsecutiveDeclarations() {
   AlignTokens(
   Style,
   [](Change const ) {
+

[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2023-11-09 Thread Gedare Bloom via cfe-commits

gedare wrote:

force push was required to layer the revert commit correctly.

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


[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2023-11-09 Thread Gedare Bloom via cfe-commits

https://github.com/gedare updated 
https://github.com/llvm/llvm-project/pull/69340

>From 610d0b544d7927af93b6943078df033f154b74f8 Mon Sep 17 00:00:00 2001
From: Gedare Bloom 
Date: Thu, 9 Nov 2023 09:30:24 -0700
Subject: [PATCH 1/2] Revert "Revert "[clang-format] Fix align consecutive
 declarations over function pointers""

This reverts commit 7bc1031c474ebb2216a5432273dafe4d1490fbce.
---
 clang/lib/Format/WhitespaceManager.cpp |  2 +-
 clang/unittests/Format/FormatTest.cpp  | 10 ++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/clang/lib/Format/WhitespaceManager.cpp 
b/clang/lib/Format/WhitespaceManager.cpp
index ff8b1e6e13a3f77..d4d66f0a9b2ab95 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -980,7 +980,7 @@ void WhitespaceManager::alignConsecutiveDeclarations() {
   AlignTokens(
   Style,
   [](Change const ) {
-if (C.Tok->is(TT_FunctionDeclarationName))
+if (C.Tok->isOneOf(TT_FunctionDeclarationName, TT_FunctionTypeLParen))
   return true;
 if (C.Tok->isNot(TT_StartOfName))
   return false;
diff --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 80903e7630c8073..19de60a5a4031d0 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -2047,6 +2047,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int *d;\n"
"Const unsigned int \n"
"const unsigned int \n"
+   "int*f1(int *a, int , int &);\n"
+   "double *(*f2)(int *a, double &);\n"
"const unsigned&\n"
"Const unsigned  h;",
Style);
@@ -2092,6 +2094,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int* d;\n"
"Const unsigned int& e;\n"
"const unsigned int& f;\n"
+   "int*f1(int* a, int& b, int&& c);\n"
+   "double* (*f2)(int* a, double&& b);\n"
"const unsigned&&g;\n"
"Const unsigned  h;",
Style);
@@ -2117,6 +2121,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int *d;\n"
"Const unsigned int& e;\n"
"const unsigned int& f;\n"
+   "int*f1(int *a, int& b, int&& c);\n"
+   "double *(*f2)(int *a, double&& b);\n"
"const unsigned  g;\n"
"Const unsigned  h;",
Style);
@@ -2157,6 +2163,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int*  d;\n"
"Const unsigned int & e;\n"
"const unsigned int & f;\n"
+   "int* f1(int* a, int & b, int && c);\n"
+   "double*  (*f2)(int* a, double && b);\n"
"const unsigned &&g;\n"
"Const unsigned   h;",
Style);
@@ -2182,6 +2190,8 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
"const unsigned int * d;\n"
"Const unsigned int  \n"
"const unsigned int  \n"
+   "int *f1(int * a, int , int &);\n"
+   "double * (*f2)(int * a, double &);\n"
"const unsigned &\n"
"Const unsigned   h;",
Style);

>From bb386b5e912686fb292fb0de89e124fa54076aa7 Mon Sep 17 00:00:00 2001
From: Gedare Bloom 
Date: Tue, 17 Oct 2023 08:21:55 -0600
Subject: [PATCH 2/2] Split alignment of declarations around assignment

Function pointers are detected as a type of declaration using
FunctionTypeLParen. They are aligned based on rules for
AlignConsecutiveDeclarations. When a function pointer is on the
right-hand side of an assignment, the alignment of the function pointer
can result in excessive whitespace padding due to the ordering of
alignment, as the alignment processes a line from left-to-right and
first aligns the declarations before and after the assignment operator,
and then aligns the assignment operator. Injection of whitespace by
alignment of declarations after the equal sign followed by alignment
of the equal sign results in the excessive whitespace.

Fixes #68079.
---
 clang/lib/Format/WhitespaceManager.cpp | 3 +++
 clang/unittests/Format/FormatTest.cpp  | 9 +
 2 files changed, 12 insertions(+)

diff --git a/clang/lib/Format/WhitespaceManager.cpp 
b/clang/lib/Format/WhitespaceManager.cpp
index d4d66f0a9b2ab95..294a87a6759212c 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -980,6 +980,9 @@ void WhitespaceManager::alignConsecutiveDeclarations() {
   AlignTokens(
   Style,
   [](Change const ) {
+

[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2023-10-27 Thread Owen Pan via cfe-commits

https://github.com/owenca requested changes to this pull request.

Please see 
[here](https://github.com/llvm/llvm-project/issues/68079#issuecomment-1776826021).

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


[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2023-10-27 Thread Owen Pan via cfe-commits

owenca wrote:

> @gedare can you fix the merge conflict on this one?

We don't want to merge this yet. See 
[here](https://github.com/llvm/llvm-project/issues/68079#issuecomment-1776826021).

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


[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2023-10-27 Thread Tobias Hieta via cfe-commits

tru wrote:

@gedare can you fix the merge conflict on this one?

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


[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2023-10-18 Thread Björn Schäpers via cfe-commits

https://github.com/HazardyKnusperkeks approved this pull request.


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


[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2023-10-18 Thread Gedare Bloom via cfe-commits

https://github.com/gedare updated 
https://github.com/llvm/llvm-project/pull/69340

>From c5d25f8de3de16ff3ed873446da017e9c26e0767 Mon Sep 17 00:00:00 2001
From: Gedare Bloom 
Date: Tue, 17 Oct 2023 08:21:55 -0600
Subject: [PATCH 1/3] [clang-format]: Split alignment of declarations around
 assignment

Function pointers are detected as a type of declaration using
FunctionTypeLParen. They are aligned based on rules for
AlignConsecutiveDeclarations. When a function pointer is on the
right-hand side of an assignment, the alignment of the function pointer
can result in excessive whitespace padding due to the ordering of
alignment, as the alignment processes a line from left-to-right and
first aligns the declarations before and after the assignment operator,
and then aligns the assignment operator. Injection of whitespace by
alignment of declarations after the equal sign followed by alignment
of the equal sign results in the excessive whitespace.

Fixes #68079.
---
 clang/lib/Format/WhitespaceManager.cpp | 43 --
 clang/lib/Format/WhitespaceManager.h   |  3 +-
 clang/unittests/Format/FormatTest.cpp  |  9 ++
 3 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/clang/lib/Format/WhitespaceManager.cpp 
b/clang/lib/Format/WhitespaceManager.cpp
index dc81060671c1712..e6bc0963b40dc75 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -108,9 +108,10 @@ const tooling::Replacements 
::generateReplacements() {
   calculateLineBreakInformation();
   alignConsecutiveMacros();
   alignConsecutiveShortCaseStatements();
-  alignConsecutiveDeclarations();
+  alignConsecutiveDeclarationsPreAssignment();
   alignConsecutiveBitFields();
   alignConsecutiveAssignments();
+  alignConsecutiveDeclarationsPostAssignment();
   alignChainedConditionals();
   alignTrailingComments();
   alignEscapedNewlines();
@@ -973,13 +974,51 @@ void 
WhitespaceManager::alignConsecutiveShortCaseStatements() {
  Changes);
 }
 
-void WhitespaceManager::alignConsecutiveDeclarations() {
+void WhitespaceManager::alignConsecutiveDeclarationsPreAssignment() {
   if (!Style.AlignConsecutiveDeclarations.Enabled)
 return;
 
   AlignTokens(
   Style,
   [](Change const ) {
+for (FormatToken *Prev = C.Tok->Previous; Prev; Prev = Prev->Previous)
+  if (Prev->is(tok::equal))
+return false;
+if (C.Tok->isOneOf(TT_FunctionDeclarationName, TT_FunctionTypeLParen))
+  return true;
+if (C.Tok->isNot(TT_StartOfName))
+  return false;
+if (C.Tok->Previous &&
+C.Tok->Previous->is(TT_StatementAttributeLikeMacro))
+  return false;
+// Check if there is a subsequent name that starts the same 
declaration.
+for (FormatToken *Next = C.Tok->Next; Next; Next = Next->Next) {
+  if (Next->is(tok::comment))
+continue;
+  if (Next->is(TT_PointerOrReference))
+return false;
+  if (!Next->Tok.getIdentifierInfo())
+break;
+  if (Next->isOneOf(TT_StartOfName, TT_FunctionDeclarationName,
+tok::kw_operator)) {
+return false;
+  }
+}
+return true;
+  },
+  Changes, /*StartAt=*/0, Style.AlignConsecutiveDeclarations);
+}
+
+void WhitespaceManager::alignConsecutiveDeclarationsPostAssignment() {
+  if (!Style.AlignConsecutiveDeclarations.Enabled)
+return;
+
+  AlignTokens(
+  Style,
+  [](Change const ) {
+for (FormatToken *Next = C.Tok->Next; Next; Next = Next->Next)
+  if (Next->is(tok::equal))
+return false;
 if (C.Tok->isOneOf(TT_FunctionDeclarationName, TT_FunctionTypeLParen))
   return true;
 if (C.Tok->isNot(TT_StartOfName))
diff --git a/clang/lib/Format/WhitespaceManager.h 
b/clang/lib/Format/WhitespaceManager.h
index df7e9add1cd446f..19bd4a3a6f7791f 100644
--- a/clang/lib/Format/WhitespaceManager.h
+++ b/clang/lib/Format/WhitespaceManager.h
@@ -227,7 +227,8 @@ class WhitespaceManager {
   void alignConsecutiveBitFields();
 
   /// Align consecutive declarations over all \c Changes.
-  void alignConsecutiveDeclarations();
+  void alignConsecutiveDeclarationsPreAssignment();
+  void alignConsecutiveDeclarationsPostAssignment();
 
   /// Align consecutive declarations over all \c Changes.
   void alignChainedConditionals();
diff --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 963fb8f4d441618..1dc05ff9de5f0fa 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -18783,6 +18783,15 @@ TEST_F(FormatTest, AlignConsecutiveDeclarations) {
" \"bb\"};\n"
"int   bbb = 0;",
Alignment);
+  // http://llvm.org/PR68079
+  verifyFormat("using Fn   = int (A::*)();\n"
+   "using RFn  = int (A::*)() &;\n"
+

[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2023-10-18 Thread Gedare Bloom via cfe-commits

https://github.com/gedare updated 
https://github.com/llvm/llvm-project/pull/69340

>From c5d25f8de3de16ff3ed873446da017e9c26e0767 Mon Sep 17 00:00:00 2001
From: Gedare Bloom 
Date: Tue, 17 Oct 2023 08:21:55 -0600
Subject: [PATCH 1/2] [clang-format]: Split alignment of declarations around
 assignment

Function pointers are detected as a type of declaration using
FunctionTypeLParen. They are aligned based on rules for
AlignConsecutiveDeclarations. When a function pointer is on the
right-hand side of an assignment, the alignment of the function pointer
can result in excessive whitespace padding due to the ordering of
alignment, as the alignment processes a line from left-to-right and
first aligns the declarations before and after the assignment operator,
and then aligns the assignment operator. Injection of whitespace by
alignment of declarations after the equal sign followed by alignment
of the equal sign results in the excessive whitespace.

Fixes #68079.
---
 clang/lib/Format/WhitespaceManager.cpp | 43 --
 clang/lib/Format/WhitespaceManager.h   |  3 +-
 clang/unittests/Format/FormatTest.cpp  |  9 ++
 3 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/clang/lib/Format/WhitespaceManager.cpp 
b/clang/lib/Format/WhitespaceManager.cpp
index dc81060671c1712..e6bc0963b40dc75 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -108,9 +108,10 @@ const tooling::Replacements 
::generateReplacements() {
   calculateLineBreakInformation();
   alignConsecutiveMacros();
   alignConsecutiveShortCaseStatements();
-  alignConsecutiveDeclarations();
+  alignConsecutiveDeclarationsPreAssignment();
   alignConsecutiveBitFields();
   alignConsecutiveAssignments();
+  alignConsecutiveDeclarationsPostAssignment();
   alignChainedConditionals();
   alignTrailingComments();
   alignEscapedNewlines();
@@ -973,13 +974,51 @@ void 
WhitespaceManager::alignConsecutiveShortCaseStatements() {
  Changes);
 }
 
-void WhitespaceManager::alignConsecutiveDeclarations() {
+void WhitespaceManager::alignConsecutiveDeclarationsPreAssignment() {
   if (!Style.AlignConsecutiveDeclarations.Enabled)
 return;
 
   AlignTokens(
   Style,
   [](Change const ) {
+for (FormatToken *Prev = C.Tok->Previous; Prev; Prev = Prev->Previous)
+  if (Prev->is(tok::equal))
+return false;
+if (C.Tok->isOneOf(TT_FunctionDeclarationName, TT_FunctionTypeLParen))
+  return true;
+if (C.Tok->isNot(TT_StartOfName))
+  return false;
+if (C.Tok->Previous &&
+C.Tok->Previous->is(TT_StatementAttributeLikeMacro))
+  return false;
+// Check if there is a subsequent name that starts the same 
declaration.
+for (FormatToken *Next = C.Tok->Next; Next; Next = Next->Next) {
+  if (Next->is(tok::comment))
+continue;
+  if (Next->is(TT_PointerOrReference))
+return false;
+  if (!Next->Tok.getIdentifierInfo())
+break;
+  if (Next->isOneOf(TT_StartOfName, TT_FunctionDeclarationName,
+tok::kw_operator)) {
+return false;
+  }
+}
+return true;
+  },
+  Changes, /*StartAt=*/0, Style.AlignConsecutiveDeclarations);
+}
+
+void WhitespaceManager::alignConsecutiveDeclarationsPostAssignment() {
+  if (!Style.AlignConsecutiveDeclarations.Enabled)
+return;
+
+  AlignTokens(
+  Style,
+  [](Change const ) {
+for (FormatToken *Next = C.Tok->Next; Next; Next = Next->Next)
+  if (Next->is(tok::equal))
+return false;
 if (C.Tok->isOneOf(TT_FunctionDeclarationName, TT_FunctionTypeLParen))
   return true;
 if (C.Tok->isNot(TT_StartOfName))
diff --git a/clang/lib/Format/WhitespaceManager.h 
b/clang/lib/Format/WhitespaceManager.h
index df7e9add1cd446f..19bd4a3a6f7791f 100644
--- a/clang/lib/Format/WhitespaceManager.h
+++ b/clang/lib/Format/WhitespaceManager.h
@@ -227,7 +227,8 @@ class WhitespaceManager {
   void alignConsecutiveBitFields();
 
   /// Align consecutive declarations over all \c Changes.
-  void alignConsecutiveDeclarations();
+  void alignConsecutiveDeclarationsPreAssignment();
+  void alignConsecutiveDeclarationsPostAssignment();
 
   /// Align consecutive declarations over all \c Changes.
   void alignChainedConditionals();
diff --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 963fb8f4d441618..1dc05ff9de5f0fa 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -18783,6 +18783,15 @@ TEST_F(FormatTest, AlignConsecutiveDeclarations) {
" \"bb\"};\n"
"int   bbb = 0;",
Alignment);
+  // http://llvm.org/PR68079
+  verifyFormat("using Fn   = int (A::*)();\n"
+   "using RFn  = int (A::*)() &;\n"
+

[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2023-10-17 Thread Björn Schäpers via cfe-commits


@@ -973,13 +974,51 @@ void 
WhitespaceManager::alignConsecutiveShortCaseStatements() {
  Changes);
 }
 
-void WhitespaceManager::alignConsecutiveDeclarations() {
+void WhitespaceManager::alignConsecutiveDeclarationsPreAssignment() {
   if (!Style.AlignConsecutiveDeclarations.Enabled)
 return;
 
   AlignTokens(
   Style,
   [](Change const ) {
+for (FormatToken *Prev = C.Tok->Previous; Prev; Prev = Prev->Previous)

HazardyKnusperkeks wrote:

Could you extract that check and use the same function for the alignment? I'd 
not like the code duplication.

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


[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2023-10-17 Thread Björn Schäpers via cfe-commits


@@ -18783,6 +18783,15 @@ TEST_F(FormatTest, AlignConsecutiveDeclarations) {
" \"bb\"};\n"
"int   bbb = 0;",
Alignment);
+  // http://llvm.org/PR68079
+  verifyFormat("using Fn   = int (A::*)();\n"
+   "using RFn  = int (A::*)() &;\n"
+   "using RRFn = int (A::*)() &&;",
+   Alignment);
+  verifyFormat("using Fn   = int(A::*)();\n"

HazardyKnusperkeks wrote:

See https://github.com/llvm/llvm-project/issues/68079#issuecomment-1767116044

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


[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2023-10-17 Thread via cfe-commits

llvmbot wrote:




@llvm/pr-subscribers-clang-format

Author: Gedare Bloom (gedare)


Changes

Function pointers are detected as a type of declaration using 
FunctionTypeLParen. They are aligned based on rules for 
AlignConsecutiveDeclarations. When a function pointer is on the right-hand side 
of an assignment, the alignment of the function pointer can result in excessive 
whitespace padding due to the ordering of alignment, as the alignment processes 
a line from left-to-right and first aligns the declarations before and after 
the assignment operator, and then aligns the assignment operator. Injection of 
whitespace by alignment of declarations after the equal sign followed by 
alignment of the equal sign results in the excessive whitespace.

Fixes #68079.

---
Full diff: https://github.com/llvm/llvm-project/pull/69340.diff


3 Files Affected:

- (modified) clang/lib/Format/WhitespaceManager.cpp (+41-2) 
- (modified) clang/lib/Format/WhitespaceManager.h (+2-1) 
- (modified) clang/unittests/Format/FormatTest.cpp (+9) 


``diff
diff --git a/clang/lib/Format/WhitespaceManager.cpp 
b/clang/lib/Format/WhitespaceManager.cpp
index dc81060671c1712..e6bc0963b40dc75 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -108,9 +108,10 @@ const tooling::Replacements 
::generateReplacements() {
   calculateLineBreakInformation();
   alignConsecutiveMacros();
   alignConsecutiveShortCaseStatements();
-  alignConsecutiveDeclarations();
+  alignConsecutiveDeclarationsPreAssignment();
   alignConsecutiveBitFields();
   alignConsecutiveAssignments();
+  alignConsecutiveDeclarationsPostAssignment();
   alignChainedConditionals();
   alignTrailingComments();
   alignEscapedNewlines();
@@ -973,13 +974,51 @@ void 
WhitespaceManager::alignConsecutiveShortCaseStatements() {
  Changes);
 }
 
-void WhitespaceManager::alignConsecutiveDeclarations() {
+void WhitespaceManager::alignConsecutiveDeclarationsPreAssignment() {
   if (!Style.AlignConsecutiveDeclarations.Enabled)
 return;
 
   AlignTokens(
   Style,
   [](Change const ) {
+for (FormatToken *Prev = C.Tok->Previous; Prev; Prev = Prev->Previous)
+  if (Prev->is(tok::equal))
+return false;
+if (C.Tok->isOneOf(TT_FunctionDeclarationName, TT_FunctionTypeLParen))
+  return true;
+if (C.Tok->isNot(TT_StartOfName))
+  return false;
+if (C.Tok->Previous &&
+C.Tok->Previous->is(TT_StatementAttributeLikeMacro))
+  return false;
+// Check if there is a subsequent name that starts the same 
declaration.
+for (FormatToken *Next = C.Tok->Next; Next; Next = Next->Next) {
+  if (Next->is(tok::comment))
+continue;
+  if (Next->is(TT_PointerOrReference))
+return false;
+  if (!Next->Tok.getIdentifierInfo())
+break;
+  if (Next->isOneOf(TT_StartOfName, TT_FunctionDeclarationName,
+tok::kw_operator)) {
+return false;
+  }
+}
+return true;
+  },
+  Changes, /*StartAt=*/0, Style.AlignConsecutiveDeclarations);
+}
+
+void WhitespaceManager::alignConsecutiveDeclarationsPostAssignment() {
+  if (!Style.AlignConsecutiveDeclarations.Enabled)
+return;
+
+  AlignTokens(
+  Style,
+  [](Change const ) {
+for (FormatToken *Next = C.Tok->Next; Next; Next = Next->Next)
+  if (Next->is(tok::equal))
+return false;
 if (C.Tok->isOneOf(TT_FunctionDeclarationName, TT_FunctionTypeLParen))
   return true;
 if (C.Tok->isNot(TT_StartOfName))
diff --git a/clang/lib/Format/WhitespaceManager.h 
b/clang/lib/Format/WhitespaceManager.h
index df7e9add1cd446f..19bd4a3a6f7791f 100644
--- a/clang/lib/Format/WhitespaceManager.h
+++ b/clang/lib/Format/WhitespaceManager.h
@@ -227,7 +227,8 @@ class WhitespaceManager {
   void alignConsecutiveBitFields();
 
   /// Align consecutive declarations over all \c Changes.
-  void alignConsecutiveDeclarations();
+  void alignConsecutiveDeclarationsPreAssignment();
+  void alignConsecutiveDeclarationsPostAssignment();
 
   /// Align consecutive declarations over all \c Changes.
   void alignChainedConditionals();
diff --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 963fb8f4d441618..1dc05ff9de5f0fa 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -18783,6 +18783,15 @@ TEST_F(FormatTest, AlignConsecutiveDeclarations) {
" \"bb\"};\n"
"int   bbb = 0;",
Alignment);
+  // http://llvm.org/PR68079
+  verifyFormat("using Fn   = int (A::*)();\n"
+   "using RFn  = int (A::*)() &;\n"
+   "using RRFn = int (A::*)() &&;",
+   Alignment);
+  verifyFormat("using Fn   = int(A::*)();\n"
+   "using RFn 

[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)

2023-10-17 Thread Gedare Bloom via cfe-commits

https://github.com/gedare created 
https://github.com/llvm/llvm-project/pull/69340

Function pointers are detected as a type of declaration using 
FunctionTypeLParen. They are aligned based on rules for 
AlignConsecutiveDeclarations. When a function pointer is on the right-hand side 
of an assignment, the alignment of the function pointer can result in excessive 
whitespace padding due to the ordering of alignment, as the alignment processes 
a line from left-to-right and first aligns the declarations before and after 
the assignment operator, and then aligns the assignment operator. Injection of 
whitespace by alignment of declarations after the equal sign followed by 
alignment of the equal sign results in the excessive whitespace.

Fixes #68079.

>From c5d25f8de3de16ff3ed873446da017e9c26e0767 Mon Sep 17 00:00:00 2001
From: Gedare Bloom 
Date: Tue, 17 Oct 2023 08:21:55 -0600
Subject: [PATCH] [clang-format]: Split alignment of declarations around
 assignment

Function pointers are detected as a type of declaration using
FunctionTypeLParen. They are aligned based on rules for
AlignConsecutiveDeclarations. When a function pointer is on the
right-hand side of an assignment, the alignment of the function pointer
can result in excessive whitespace padding due to the ordering of
alignment, as the alignment processes a line from left-to-right and
first aligns the declarations before and after the assignment operator,
and then aligns the assignment operator. Injection of whitespace by
alignment of declarations after the equal sign followed by alignment
of the equal sign results in the excessive whitespace.

Fixes #68079.
---
 clang/lib/Format/WhitespaceManager.cpp | 43 --
 clang/lib/Format/WhitespaceManager.h   |  3 +-
 clang/unittests/Format/FormatTest.cpp  |  9 ++
 3 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/clang/lib/Format/WhitespaceManager.cpp 
b/clang/lib/Format/WhitespaceManager.cpp
index dc81060671c1712..e6bc0963b40dc75 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -108,9 +108,10 @@ const tooling::Replacements 
::generateReplacements() {
   calculateLineBreakInformation();
   alignConsecutiveMacros();
   alignConsecutiveShortCaseStatements();
-  alignConsecutiveDeclarations();
+  alignConsecutiveDeclarationsPreAssignment();
   alignConsecutiveBitFields();
   alignConsecutiveAssignments();
+  alignConsecutiveDeclarationsPostAssignment();
   alignChainedConditionals();
   alignTrailingComments();
   alignEscapedNewlines();
@@ -973,13 +974,51 @@ void 
WhitespaceManager::alignConsecutiveShortCaseStatements() {
  Changes);
 }
 
-void WhitespaceManager::alignConsecutiveDeclarations() {
+void WhitespaceManager::alignConsecutiveDeclarationsPreAssignment() {
   if (!Style.AlignConsecutiveDeclarations.Enabled)
 return;
 
   AlignTokens(
   Style,
   [](Change const ) {
+for (FormatToken *Prev = C.Tok->Previous; Prev; Prev = Prev->Previous)
+  if (Prev->is(tok::equal))
+return false;
+if (C.Tok->isOneOf(TT_FunctionDeclarationName, TT_FunctionTypeLParen))
+  return true;
+if (C.Tok->isNot(TT_StartOfName))
+  return false;
+if (C.Tok->Previous &&
+C.Tok->Previous->is(TT_StatementAttributeLikeMacro))
+  return false;
+// Check if there is a subsequent name that starts the same 
declaration.
+for (FormatToken *Next = C.Tok->Next; Next; Next = Next->Next) {
+  if (Next->is(tok::comment))
+continue;
+  if (Next->is(TT_PointerOrReference))
+return false;
+  if (!Next->Tok.getIdentifierInfo())
+break;
+  if (Next->isOneOf(TT_StartOfName, TT_FunctionDeclarationName,
+tok::kw_operator)) {
+return false;
+  }
+}
+return true;
+  },
+  Changes, /*StartAt=*/0, Style.AlignConsecutiveDeclarations);
+}
+
+void WhitespaceManager::alignConsecutiveDeclarationsPostAssignment() {
+  if (!Style.AlignConsecutiveDeclarations.Enabled)
+return;
+
+  AlignTokens(
+  Style,
+  [](Change const ) {
+for (FormatToken *Next = C.Tok->Next; Next; Next = Next->Next)
+  if (Next->is(tok::equal))
+return false;
 if (C.Tok->isOneOf(TT_FunctionDeclarationName, TT_FunctionTypeLParen))
   return true;
 if (C.Tok->isNot(TT_StartOfName))
diff --git a/clang/lib/Format/WhitespaceManager.h 
b/clang/lib/Format/WhitespaceManager.h
index df7e9add1cd446f..19bd4a3a6f7791f 100644
--- a/clang/lib/Format/WhitespaceManager.h
+++ b/clang/lib/Format/WhitespaceManager.h
@@ -227,7 +227,8 @@ class WhitespaceManager {
   void alignConsecutiveBitFields();
 
   /// Align consecutive declarations over all \c Changes.
-  void alignConsecutiveDeclarations();
+  void alignConsecutiveDeclarationsPreAssignment();
+  void