[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)
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)
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)
@@ -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)
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)
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)
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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
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)
@@ -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)
@@ -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)
@@ -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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
@@ -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)
@@ -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)
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)
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