[clang] [clang-format] Fix the indent of the ternary operator when AlignOperands and BreakBeforeTernaryOperators is specified. (PR #100860)
via cfe-commits
cfe-commits at lists.llvm.org
Sat Jul 27 01:27:38 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-format
Author: Hirofumi Nakamura (hnakamura5)
<details>
<summary>Changes</summary>
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.
---
Full diff: https://github.com/llvm/llvm-project/pull/100860.diff
3 Files Affected:
- (modified) clang/lib/Format/ContinuationIndenter.cpp (+9-2)
- (modified) clang/unittests/Format/FormatTest.cpp (+68-70)
- (modified) clang/unittests/Format/FormatTestRawStrings.cpp (+7-8)
``````````diff
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";
``````````
</details>
https://github.com/llvm/llvm-project/pull/100860
More information about the cfe-commits
mailing list