Skip to content

clang-format: ensure ternary operands are aligned#196697

Merged
HazardyKnusperkeks merged 1 commit intollvm:mainfrom
zeule:feature/align-ternary
May 10, 2026
Merged

clang-format: ensure ternary operands are aligned#196697
HazardyKnusperkeks merged 1 commit intollvm:mainfrom
zeule:feature/align-ternary

Conversation

@zeule
Copy link
Copy Markdown
Contributor

@zeule zeule commented May 9, 2026

Set ParentState::AlignedTo for ternary operands.

@llvmorg-github-actions
Copy link
Copy Markdown

@llvm/pr-subscribers-clang-format

Author: Eugene Shalygin (zeule)

Changes

Set ParentState::AlignedTo for ternary operands.


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

2 Files Affected:

  • (modified) clang/lib/Format/ContinuationIndenter.cpp (+15)
  • (modified) clang/unittests/Format/AlignmentTest.cpp (+10)
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index b2f799bb33b01..7b3bf98670c1b 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -1276,6 +1276,21 @@ unsigned ContinuationIndenter::addTokenOnNewLine(LineState &State,
     CurrentState.BreakBeforeParameter = false;
     CurrentState.AlignedTo = &Current;
   }
+  if (!CurrentState.AlignedTo && Current.is(TT_ConditionalExpr)) {
+    switch (Style.AlignOperands) {
+    case FormatStyle::OAS_DontAlign:
+      break;
+    case FormatStyle::OAS_Align:
+        CurrentState.AlignedTo = Current.is(tok::question)
+                                     ? Current.getPrevious(tok::equal)
+                                     : Current.getPrevious(tok::question);
+      break;
+    case FormatStyle::OAS_AlignAfterOperator:
+      if (Current.is(tok::colon))
+        CurrentState.AlignedTo = Current.getPrevious(tok::question);
+      break;
+    }
+  }
 
   if (!DryRun) {
     unsigned MaxEmptyLinesToKeep = Style.MaxEmptyLinesToKeep + 1;
diff --git a/clang/unittests/Format/AlignmentTest.cpp b/clang/unittests/Format/AlignmentTest.cpp
index 971ceeefef582..9421a4c933b9e 100644
--- a/clang/unittests/Format/AlignmentTest.cpp
+++ b/clang/unittests/Format/AlignmentTest.cpp
@@ -3599,6 +3599,16 @@ TEST_F(AlignmentTest, ContinuedAligned) {
                "\t},\n"
                "\tvariant);",
                Style);
+
+  Style.ColumnLimit = 40;
+  Style.IndentWidth = Style.TabWidth = Style.ContinuationIndentWidth = 8;
+
+  verifyFormat("void f() {\n"
+               "\tint aaaaaaaaaaaaaaaaaaaa =\n"
+               "\t\t000000000000000001 ? 2\n"
+               "\t\t                   : 3;\n"
+               "}",
+               Style);
 }
 
 } // namespace

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 9, 2026

✅ With the latest revision this PR passed the C/C++ code formatter.

CurrentState.BreakBeforeParameter = false;
CurrentState.AlignedTo = &Current;
}
if (!CurrentState.AlignedTo && Current.is(TT_ConditionalExpr)) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if you ignore the previous value of AlignedTo?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I simply want to avoid repeatedly setting it. If it is set at this point, it was set by this very code fragment during the previous invocation of addTokensOnNewLine(). I think there is no harm to leave the old value, because the token stream isn't changing.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assignment of AlignedTo is always unconditional in this function, I think we should keep it that way for consistency.

Copy link
Copy Markdown
Contributor Author

@zeule zeule May 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That certainly makes reading and reasoning simpler. Updated the code.

Set ParentState::AlignedTo for ternary operands.
@zeule zeule force-pushed the feature/align-ternary branch from a0d72ba to 04b0990 Compare May 10, 2026 13:59
@HazardyKnusperkeks HazardyKnusperkeks enabled auto-merge (squash) May 10, 2026 14:14
@HazardyKnusperkeks HazardyKnusperkeks merged commit b2f37f4 into llvm:main May 10, 2026
9 of 10 checks passed
@zeule
Copy link
Copy Markdown
Contributor Author

zeule commented May 10, 2026

Thanks!

@zeule zeule deleted the feature/align-ternary branch May 10, 2026 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants