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

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-format

Author: Gedare Bloom (gedare)

<details>
<summary>Changes</summary>

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 &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"

``````````

</details>


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