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.hirof...@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"; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
