[clang] [clang-format] Fix the indent of the ternary operator when AlignOperands and BreakBeforeTernaryOperators is specified. (PR #100860)

Hirofumi Nakamura via cfe-commits cfe-commits at lists.llvm.org
Sat Jul 27 01:27:08 PDT 2024


https://github.com/hnakamura5 created https://github.com/llvm/llvm-project/pull/100860

Trying to fix https://github.com/llvm/llvm-project/issues/98559 .

This PR modifies the ad-hoc tweaks introduced by https://reviews.llvm.org/D50078 .
The change is limited to the case `AlignOperands ` and `BreakBeforeTernaryOperators`  are specified (default in LLVM style).

```
int foo = bar > baz
              ? 42
              : 99;
```

↓ This PR makes,

```
int foo = bar > baz
          ? 42
          : 99;
```

For the case `BreakBeforeTernaryOperators` is not specified, I'm not sure such style is desirable.
```
int foo = bar > baz ?
          42 :
          99;
```
So I did not touch the case at first. It may be a point of discussion.

>From 2d2695767cbd853bc2327b2f640fdeeeb41f43f6 Mon Sep 17 00:00:00 2001
From: hnakamura5 <k.nakamura.hirofumi at gmail.com>
Date: Mon, 22 Jul 2024 23:00:45 +0900
Subject: [PATCH] [clang-format] Fix the indent of the ternary operator when
 AlignOperands and  BreakBeforeTernaryOperators is specified.

---
 clang/lib/Format/ContinuationIndenter.cpp     |  11 +-
 clang/unittests/Format/FormatTest.cpp         | 138 +++++++++---------
 .../unittests/Format/FormatTestRawStrings.cpp |  15 +-
 3 files changed, 84 insertions(+), 80 deletions(-)

diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index b07360425ca6e..c01fe6940bf22 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -1347,8 +1347,14 @@ unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) {
       //    * always un-indent by the operator when
       //    BreakBeforeTernaryOperators=true
       unsigned Indent = CurrentState.Indent;
-      if (Style.AlignOperands != FormatStyle::OAS_DontAlign)
+      if (Style.AlignOperands != FormatStyle::OAS_DontAlign &&
+          !Style.BreakBeforeTernaryOperators) {
         Indent -= Style.ContinuationIndentWidth;
+      }
+      if (Style.BreakBeforeTernaryOperators &&
+          CurrentState.IsChainedConditional) {
+        Indent -= 2;
+      }
       if (Style.BreakBeforeTernaryOperators && CurrentState.UnindentOperator)
         Indent -= 2;
       return Indent;
@@ -1773,7 +1779,8 @@ void ContinuationIndenter::moveStatePastFakeLParens(LineState &State,
         !CurrentState.IsWrappedConditional) {
       NewParenState.IsChainedConditional = true;
       NewParenState.UnindentOperator = State.Stack.back().UnindentOperator;
-    } else if (PrecedenceLevel == prec::Conditional ||
+    } else if ((PrecedenceLevel == prec::Conditional &&
+                !Style.BreakBeforeTernaryOperators) ||
                (!SkipFirstExtraIndent && PrecedenceLevel > prec::Assignment &&
                 !Current.isTrailingComment())) {
       NewParenState.Indent += Style.ContinuationIndentWidth;
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index d01ce137b8fcb..bdd657db06712 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -7332,12 +7332,11 @@ TEST_F(FormatTest, ExpressionIndentationBreakingBeforeOperators) {
                "};",
                Style);
   verifyFormat("return abc\n"
-               "         ? foo(\n"
-               "             a,\n"
-               "             b,\n"
-               "             bar(\n"
-               "               abc))\n"
-               "         : g(abc);",
+               "       ? foo(\n"
+               "           a,\n"
+               "           b,\n"
+               "           bar(abc))\n"
+               "       : g(abc);",
                Style);
 }
 
@@ -9410,20 +9409,20 @@ TEST_F(FormatTest, BreaksConditionalExpressions) {
                "    : aaaaaaaaaaaaaaaaaaaaaaaaaaa;");
   verifyFormat("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa aaaaaa =\n"
                "    aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
-               "        ? aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
-               "        : aaaaaaaaaaaaaaaa;");
+               "    ? aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
+               "    : aaaaaaaaaaaaaaaa;");
   verifyFormat(
       "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa == aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
       "    ? aaaaaaaaaaaaaaa\n"
       "    : aaaaaaaaaaaaaaa;");
   verifyFormat("f(aaaaaaaaaaaaaaaa == // force break\n"
-               "          aaaaaaaaa\n"
-               "      ? b\n"
-               "      : c);");
+               "      aaaaaaaaa\n"
+               "  ? b\n"
+               "  : c);");
   verifyFormat("return aaaa == bbbb\n"
-               "           // comment\n"
-               "           ? aaaa\n"
-               "           : bbbb;");
+               "       // comment\n"
+               "       ? aaaa\n"
+               "       : bbbb;");
   verifyFormat("unsigned Indent =\n"
                "    format(TheLine.First,\n"
                "           IndentForLevel[TheLine.Level] >= 0\n"
@@ -9432,21 +9431,20 @@ TEST_F(FormatTest, BreaksConditionalExpressions) {
                "           TheLine.InPPDirective, PreviousEndOfLineColumn);",
                getLLVMStyleWithColumns(60));
   verifyFormat("bool aaaaaa = aaaaaaaaaaaaa //\n"
-               "                  ? aaaaaaaaaaaaaaa\n"
-               "                  : bbbbbbbbbbbbbbb //\n"
-               "                        ? ccccccccccccccc\n"
-               "                        : ddddddddddddddd;");
+               "              ? aaaaaaaaaaaaaaa\n"
+               "              : bbbbbbbbbbbbbbb //\n"
+               "                    ? ccccccccccccccc\n"
+               "                    : ddddddddddddddd;");
   verifyFormat("bool aaaaaa = aaaaaaaaaaaaa //\n"
-               "                  ? aaaaaaaaaaaaaaa\n"
-               "                  : (bbbbbbbbbbbbbbb //\n"
-               "                         ? ccccccccccccccc\n"
-               "                         : ddddddddddddddd);");
-  verifyFormat(
-      "int aaaaaaaaaaaaaaaaaaaaaaaaaaa = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
-      "                                      ? aaaaaaaaaaaaaaaaaaaaaaaaa +\n"
-      "                                            aaaaaaaaaaaaaaaaaaaaa +\n"
-      "                                            aaaaaaaaaaaaaaaaaaaaa\n"
-      "                                      : aaaaaaaaaa;");
+               "              ? aaaaaaaaaaaaaaa\n"
+               "              : (bbbbbbbbbbbbbbb //\n"
+               "                 ? ccccccccccccccc\n"
+               "                 : ddddddddddddddd);");
+  verifyFormat("int aaaaaaaaaaaaaaaaaaaaaaaaaaa =\n"
+               "    aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
+               "    ? aaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaa + "
+               "aaaaaaaaaaaaaaaaaaaaa\n"
+               "    : aaaaaaaaaa;");
   verifyFormat(
       "aaaaaa = aaaaaaaaaaaa ? aaaaaaaaaa ? aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
       "                                   : aaaaaaaaaaaaaaaaaaaaaa\n"
@@ -9480,23 +9478,23 @@ TEST_F(FormatTest, BreaksConditionalExpressions) {
 
   // Assignments in conditional expressions. Apparently not uncommon :-(.
   verifyFormat("return a != b\n"
-               "           // comment\n"
-               "           ? a = b\n"
-               "           : a = b;");
+               "       // comment\n"
+               "       ? a = b\n"
+               "       : a = b;");
   verifyFormat("return a != b\n"
-               "           // comment\n"
-               "           ? a = a != b\n"
-               "                     // comment\n"
-               "                     ? a = b\n"
-               "                     : a\n"
-               "           : a;");
+               "       // comment\n"
+               "       ? a = a != b\n"
+               "             // comment\n"
+               "             ? a = b\n"
+               "             : a\n"
+               "       : a;");
   verifyFormat("return a != b\n"
-               "           // comment\n"
-               "           ? a\n"
-               "           : a = a != b\n"
-               "                     // comment\n"
-               "                     ? a = b\n"
-               "                     : a;");
+               "       // comment\n"
+               "       ? a\n"
+               "       : a = a != b\n"
+               "             // comment\n"
+               "             ? a = b\n"
+               "             : a;");
 
   // Chained conditionals
   FormatStyle Style = getLLVMStyleWithColumns(70);
@@ -9531,26 +9529,26 @@ TEST_F(FormatTest, BreaksConditionalExpressions) {
                "                        : (aaa ? bbb : ccc);",
                Style);
   verifyFormat(
-      "return aaaaaaaaaaaaaaaa ? (aaaaaaaaaaaaaaaaa ? bbbbbbbbbbbbbbbbbb\n"
-      "                                             : cccccccccccccccccc)\n"
+      "return aaaaaaaaaaaaaaaa\n"
+      "       ? (aaaaaaaaaaaaaaaaa ? bbbbbbbbbbbbbbbbbb : cccccccccccccccccc)\n"
       "       : bbbbbbbbbbbbbb ? 2222222222222222\n"
       "                        : 3333333333333333;",
       Style);
   verifyFormat(
-      "return aaaaaaaaa        ? (aaaaaaaaaaaaaaaaa ? bbbbbbbbbbbbbbbbbb\n"
-      "                                             : cccccccccccccccccc)\n"
+      "return aaaaaaaaa\n"
+      "       ? (aaaaaaaaaaaaaaaaa ? bbbbbbbbbbbbbbbbbb : cccccccccccccccccc)\n"
       "       : bbbbbbbbbbbbbb ? 2222222222222222\n"
       "                        : 3333333333333333;",
       Style);
   verifyFormat(
-      "return aaaaaaaaa        ? a = (aaaaaaaaaaaaa ? bbbbbbbbbbbbbbbbbb\n"
-      "                                             : dddddddddddddddddd)\n"
+      "return aaaaaaaaa\n"
+      "       ? a = (aaaaaaaaaaaaa ? bbbbbbbbbbbbbbbbbb : dddddddddddddddddd)\n"
       "       : bbbbbbbbbbbbbb ? 2222222222222222\n"
       "                        : 3333333333333333;",
       Style);
   verifyFormat(
-      "return aaaaaaaaa        ? a + (aaaaaaaaaaaaa ? bbbbbbbbbbbbbbbbbb\n"
-      "                                             : dddddddddddddddddd)\n"
+      "return aaaaaaaaa\n"
+      "       ? a + (aaaaaaaaaaaaa ? bbbbbbbbbbbbbbbbbb : dddddddddddddddddd)\n"
       "       : bbbbbbbbbbbbbb ? 2222222222222222\n"
       "                        : 3333333333333333;",
       Style);
@@ -9588,27 +9586,27 @@ TEST_F(FormatTest, BreaksConditionalExpressions) {
       "                        : 3333333333333333;",
       Style);
   verifyFormat(
-      "return aaaaaaaaaaaaaaaa ? aaaaaaaaaaaaaaaaaa ? bbbbbbbbbbbbbbbbbb\n"
-      "                                             : cccccccccccccccccc\n"
+      "return aaaaaaaaaaaaaaaa\n"
+      "       ? aaaaaaaaaaaaaaaaaa ? bbbbbbbbbbbbbbbbbb : cccccccccccccccccc\n"
       "       : bbbbbbbbbbbbbb ? 2222222222222222\n"
       "                        : 3333333333333333;",
       Style);
   verifyFormat(
-      "return aaaaaaaaaaaaaaaa ? aaaaaaaaaaaaaaaaaa ? bbbbbbbbbbbbbbbbbb\n"
-      "                          : cccccccccccccccc ? dddddddddddddddddd\n"
-      "                                             : eeeeeeeeeeeeeeeeee\n"
+      "return aaaaaaaaaaaaaaaa ? aaaaaaaaaaaaaaaaaa     ? bbbbbbbbbbbbbbbbbb\n"
+      "                              : cccccccccccccccc ? dddddddddddddddddd\n"
+      "                                                 : eeeeeeeeeeeeeeeeee\n"
       "       : bbbbbbbbbbbbbb ? 2222222222222222\n"
       "                        : 3333333333333333;",
       Style);
   verifyFormat("return aaaaaaaaaaaaaaaaaaaaa\n"
-               "           ? (aaaaaaaaaaaaaaaaaa   ? bbbbbbbbbbbbbbbbbb\n"
-               "              : cccccccccccccccccc ? dddddddddddddddddd\n"
-               "                                   : eeeeeeeeeeeeeeeeee)\n"
+               "       ? (aaaaaaaaaaaaaaaaaa   ? bbbbbbbbbbbbbbbbbb\n"
+               "          : cccccccccccccccccc ? dddddddddddddddddd\n"
+               "                               : eeeeeeeeeeeeeeeeee)\n"
                "       : bbbbbbbbbbbbbbbbbbb ? 2222222222222222\n"
                "                             : 3333333333333333;",
                Style);
   verifyFormat("return aaaaaaaaaaaaaaaaaaaaaaaaa\n"
-               "           ? aaaaaaaaaaaaaaaaaa ? bbbbbbbbbbbbbbbbbb\n"
+               "       ? aaaaaaaaaaaaaaaaaa     ? bbbbbbbbbbbbbbbbbb\n"
                "             : cccccccccccccccc ? dddddddddddddddddd\n"
                "                                : eeeeeeeeeeeeeeeeee\n"
                "       : bbbbbbbbbbbbbbbbbbbbbbb ? 2222222222222222\n"
@@ -26490,11 +26488,11 @@ TEST_F(FormatTest, MultilineLambdaInConditional) {
 
   Style = getLLVMStyleWithColumns(60);
   verifyFormat("auto aLengthyIdentifier = oneExpressionSoThatWeBreak\n"
-               "                              ? []() {\n"
-               "                                  ;\n"
-               "                                  return 5;\n"
-               "                                }()\n"
-               "                              : 2;",
+               "                          ? []() {\n"
+               "                              ;\n"
+               "                              return 5;\n"
+               "                            }()\n"
+               "                          : 2;",
                Style);
   verifyFormat("auto aLengthyIdentifier =\n"
                "    oneExpressionSoThatWeBreak ? 2 : []() {\n"
@@ -26513,11 +26511,11 @@ TEST_F(FormatTest, MultilineLambdaInConditional) {
                Style);
   verifyFormat("auto aLengthyIdentifier =\n"
                "    oneExpressionSoThatWeBreak\n"
-               "        ? 2\n"
-               "        : []() {\n"
-               "            ;\n"
-               "            return 5;\n"
-               "          };",
+               "    ? 2\n"
+               "    : []() {\n"
+               "        ;\n"
+               "        return 5;\n"
+               "      };",
                Style);
 }
 
diff --git a/clang/unittests/Format/FormatTestRawStrings.cpp b/clang/unittests/Format/FormatTestRawStrings.cpp
index 0615fb1fad4c5..c21172c6ac7ad 100644
--- a/clang/unittests/Format/FormatTestRawStrings.cpp
+++ b/clang/unittests/Format/FormatTestRawStrings.cpp
@@ -642,8 +642,8 @@ auto S=R"pb(item_1:1)pb"+R"pb(item_2:2)pb"+R"pb(item_3:3)pb";
 
   expect_eq(R"test(
 auto S = (count < 3)
-             ? R"pb(item_1: 1)pb"
-             : R"pb(item_2: 2)pb";
+         ? R"pb(item_1: 1)pb"
+         : R"pb(item_2: 2)pb";
 )test",
             format(R"test(
 auto S=(count<3)?R"pb(item_1:1)pb":R"pb(item_2:2)pb";
@@ -651,10 +651,9 @@ auto S=(count<3)?R"pb(item_1:1)pb":R"pb(item_2:2)pb";
                    getRawStringPbStyleWithColumns(40)));
 
   expect_eq(R"test(
-auto S =
-    (count < 3)
-        ? R"pb(item_1: 1, item_2: 2)pb"
-        : R"pb(item_3: 3)pb";
+auto S = (count < 3)
+         ? R"pb(item_1: 1, item_2: 2)pb"
+         : R"pb(item_3: 3)pb";
 )test",
             format(R"test(
 auto S=(count<3)?R"pb(item_1:1,item_2:2)pb":R"pb(item_3:3)pb";
@@ -664,8 +663,8 @@ auto S=(count<3)?R"pb(item_1:1,item_2:2)pb":R"pb(item_3:3)pb";
   expect_eq(R"test(
 auto S =
     (count < 3)
-        ? R"pb(item_1: 1)pb"
-        : R"pb(item_2: 2, item_3: 3)pb";
+    ? R"pb(item_1: 1)pb"
+    : R"pb(item_2: 2, item_3: 3)pb";
 )test",
             format(R"test(
 auto S=(count<3)?R"pb(item_1:1)pb":R"pb(item_2:2,item_3:3)pb";



More information about the cfe-commits mailing list