[clang-format] Align multi line expressions

[sstwcw]

This patch updates the code in the AlignTokens function for finding regions to align. Now it uses the same logic as the AlignTokenSequence function. Previously it only recognized string literals split across multiple lines.

Fixes #171022.

The new way for finding where the expression ends identifies a ternary ? or : operator at the start of the line as continuation of the preceding line. The code around line 577 for figuring out whether there are multiple matches on the same line is updated so that it does not require the first token on a continued line not to match.

The sample in the bug report has /**/. The added test has //. This is because the mess up procedure in the test will remove the line break following /**/.

after, with AlignConsecutiveAssignments enabled

param->fault_depth = grid->jfault * grid->dz; //
grid->corner       = grid->jlid + 1;          //
param->peclet      = param->V                 //
                     * param->L * 1000.0      //
                     / param->kappa;          //

before

param->fault_depth = grid->jfault * grid->dz; //
grid->corner       = grid->jlid + 1;          //
param->peclet      = param->V                 //
                     * param->L * 1000.0      //
                / param->kappa;               //

[llvmbot]

@llvm/pr-subscribers-clang-format

Author: None (sstwcw)

Changes

This patch updates the code in the AlignTokens function for finding regions to align. Now it uses the same logic as the AlignTokenSequence function. Previously it only recognized string literals split across multiple lines.

Fixes #171022.

The new way for finding where the expression ends identifies a ternary ? or : operator at the start of the line as continuation of the preceding line. The code around line 577 for figuring out whether there are multiple matches on the same line is updated so that it does not require the first token on a continued line not to match.

The sample in the bug report has /**/. The added test has //. This is because the mess up procedure in the test will remove the line break following /**/.

after, with AlignConsecutiveAssignments enabled

param->fault_depth = grid->jfault * grid->dz; //
grid->corner       = grid->jlid + 1;          //
param->peclet      = param->V                 //
                     * param->L * 1000.0      //
                     / param->kappa;          //

before

param->fault_depth = grid->jfault * grid->dz; //
grid->corner       = grid->jlid + 1;          //
param->peclet      = param->V                 //
                     * param->L * 1000.0      //
                / param->kappa;               //

---

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

2 Files Affected:

• (modified) clang/lib/Format/WhitespaceManager.cpp (+10-10)
• (modified) clang/unittests/Format/FormatTest.cpp (+16)

diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp
index 6bfcebe73a165..947578cbff9ad 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -15,6 +15,7 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include <algorithm>
+#include <optional>
 
 namespace clang {
 namespace format {
@@ -475,7 +476,7 @@ static unsigned AlignTokens(const FormatStyle &Style, F &&Matches,
   // RightJustify is false.
   unsigned WidthRight = 0;
 
-  // Line number of the start and the end of the current token sequence.
+  // Number of the start and the end of the current token sequence.
   unsigned StartOfSequence = 0;
   unsigned EndOfSequence = 0;
 
@@ -494,8 +495,8 @@ static unsigned AlignTokens(const FormatStyle &Style, F &&Matches,
   unsigned CommasBeforeLastMatch = 0;
   unsigned CommasBeforeMatch = 0;
 
-  // Whether a matching token has been found on the current line.
-  bool FoundMatchOnLine = false;
+  // The column number of the matching token on the current line.
+  std::optional<unsigned> MatchingColumn;
 
   // Whether the current line consists purely of comments.
   bool LineIsComment = true;
@@ -539,17 +540,15 @@ static unsigned AlignTokens(const FormatStyle &Style, F &&Matches,
       // Whether to break the alignment sequence because of a line without a
       // match.
       bool NoMatchBreak =
-          !FoundMatchOnLine && !(LineIsComment && ACS.AcrossComments);
+          !MatchingColumn && !(LineIsComment && ACS.AcrossComments);
 
       if (EmptyLineBreak || NoMatchBreak)
         AlignCurrentSequence();
 
       // A new line starts, re-initialize line status tracking bools.
       // Keep the match state if a string literal is continued on this line.
-      if (I == 0 || CurrentChange.Tok->isNot(tok::string_literal) ||
-          Changes[I - 1].Tok->isNot(tok::string_literal)) {
-        FoundMatchOnLine = false;
-      }
+      if (MatchingColumn && CurrentChange.IndentedFromColumn < *MatchingColumn)
+        MatchingColumn.reset();
       LineIsComment = true;
     }
 
@@ -574,13 +573,14 @@ static unsigned AlignTokens(const FormatStyle &Style, F &&Matches,
 
     // If there is more than one matching token per line, or if the number of
     // preceding commas, do not match anymore, end the sequence.
-    if (FoundMatchOnLine || CommasBeforeMatch != CommasBeforeLastMatch) {
+    if ((CurrentChange.NewlinesBefore == 0U && MatchingColumn) ||
+        CommasBeforeMatch != CommasBeforeLastMatch) {
       MatchedIndices.push_back(I);
       AlignCurrentSequence();
     }
 
     CommasBeforeLastMatch = CommasBeforeMatch;
-    FoundMatchOnLine = true;
+    MatchingColumn = CurrentChange.StartOfTokenColumn;
 
     if (StartOfSequence == 0)
       StartOfSequence = I;
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index bf69d0a56ebfc..5cdac66d1dcbd 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -20855,6 +20855,22 @@ TEST_F(FormatTest, AlignWithLineBreaks) {
                Style);
   // clang-format on
 
+  Style = getLLVMStyle();
+  Style.AlignConsecutiveAssignments.Enabled = true;
+  verifyFormat("param->fault_depth = grid->jfault * grid->dz; //\n"
+               "grid->corner       = grid->jlid + 1;          //\n"
+               "param->peclet      = param->V                 //\n"
+               "                     * param->L * 1000.0      //\n"
+               "                     / param->kappa;          //",
+               Style);
+  Style.AlignOperands = FormatStyle::OAS_AlignAfterOperator;
+  verifyFormat("param->fault_depth = grid->jfault * grid->dz; //\n"
+               "grid->corner       = grid->jlid + 1;          //\n"
+               "param->peclet      = param->V                 //\n"
+               "                   * param->L * 1000.0        //\n"
+               "                   / param->kappa;            //",
+               Style);
+
   Style = getLLVMStyleWithColumns(70);
   Style.AlignConsecutiveDeclarations.Enabled = true;
   verifyFormat(