[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)
Gedare Bloom via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 18 06:52:53 PDT 2023
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 <gedare at rtems.org>
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 &WhitespaceManager::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 &C) {
+ 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 &C) {
+ 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 bbbbbbb = 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 = int *(A::*)() &;\n"
+ "using RRFn = double (A::*)() &&;",
+ Alignment);
// PAS_Right
verifyFormat("void SomeFunction(int parameter = 0) {\n"
>From 976d5b9de4d20364e81b418b56fc7801a484d909 Mon Sep 17 00:00:00 2001
From: Gedare Bloom <gedare at rtems.org>
Date: Wed, 18 Oct 2023 07:47:54 -0600
Subject: [PATCH 2/3] Do not align declarations that happen after an
assignment.
---
clang/lib/Format/WhitespaceManager.cpp | 40 ++------------------------
clang/lib/Format/WhitespaceManager.h | 3 +-
2 files changed, 3 insertions(+), 40 deletions(-)
diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp
index e6bc0963b40dc75..741ccd36863ab01 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -108,10 +108,9 @@ const tooling::Replacements &WhitespaceManager::generateReplacements() {
calculateLineBreakInformation();
alignConsecutiveMacros();
alignConsecutiveShortCaseStatements();
- alignConsecutiveDeclarationsPreAssignment();
+ alignConsecutiveDeclarations();
alignConsecutiveBitFields();
alignConsecutiveAssignments();
- alignConsecutiveDeclarationsPostAssignment();
alignChainedConditionals();
alignTrailingComments();
alignEscapedNewlines();
@@ -974,7 +973,7 @@ void WhitespaceManager::alignConsecutiveShortCaseStatements() {
Changes);
}
-void WhitespaceManager::alignConsecutiveDeclarationsPreAssignment() {
+void WhitespaceManager::alignConsecutiveDeclarations() {
if (!Style.AlignConsecutiveDeclarations.Enabled)
return;
@@ -1009,41 +1008,6 @@ void WhitespaceManager::alignConsecutiveDeclarationsPreAssignment() {
Changes, /*StartAt=*/0, Style.AlignConsecutiveDeclarations);
}
-void WhitespaceManager::alignConsecutiveDeclarationsPostAssignment() {
- if (!Style.AlignConsecutiveDeclarations.Enabled)
- return;
-
- AlignTokens(
- Style,
- [](Change const &C) {
- 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))
- 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::alignChainedConditionals() {
if (Style.BreakBeforeTernaryOperators) {
AlignTokens(
diff --git a/clang/lib/Format/WhitespaceManager.h b/clang/lib/Format/WhitespaceManager.h
index 19bd4a3a6f7791f..df7e9add1cd446f 100644
--- a/clang/lib/Format/WhitespaceManager.h
+++ b/clang/lib/Format/WhitespaceManager.h
@@ -227,8 +227,7 @@ class WhitespaceManager {
void alignConsecutiveBitFields();
/// Align consecutive declarations over all \c Changes.
- void alignConsecutiveDeclarationsPreAssignment();
- void alignConsecutiveDeclarationsPostAssignment();
+ void alignConsecutiveDeclarations();
/// Align consecutive declarations over all \c Changes.
void alignChainedConditionals();
>From 8c42f5ff1adc75778604b1b5420874a851f1da4d Mon Sep 17 00:00:00 2001
From: Gedare Bloom <gedare at rtems.org>
Date: Wed, 18 Oct 2023 07:52:39 -0600
Subject: [PATCH 3/3] FormatTests: fix alignment after assignment
---
clang/unittests/Format/FormatTest.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 1dc05ff9de5f0fa..5cd4411061806d9 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -18788,8 +18788,8 @@ TEST_F(FormatTest, AlignConsecutiveDeclarations) {
"using RFn = int (A::*)() &;\n"
"using RRFn = int (A::*)() &&;",
Alignment);
- verifyFormat("using Fn = int (A::*)();\n"
- "using RFn = int *(A::*)() &;\n"
+ verifyFormat("using Fn = int (A::*)();\n"
+ "using RFn = int *(A::*)() &;\n"
"using RRFn = double (A::*)() &&;",
Alignment);
More information about the cfe-commits
mailing list