[clang] 0fad648 - [clang-format] restore indent in conditionals when AlignOperands is DontAlign
Krasimir Georgiev via cfe-commits
cfe-commits at lists.llvm.org
Wed Jun 24 04:12:56 PDT 2020
Author: Krasimir Georgiev
Date: 2020-06-24T13:11:18+02:00
New Revision: 0fad648b65b99b68040fa26c5da9c0bec5b0aa1d
URL: https://github.com/llvm/llvm-project/commit/0fad648b65b99b68040fa26c5da9c0bec5b0aa1d
DIFF: https://github.com/llvm/llvm-project/commit/0fad648b65b99b68040fa26c5da9c0bec5b0aa1d.diff
LOG: [clang-format] restore indent in conditionals when AlignOperands is DontAlign
Summary:
After D50078, we're experiencing unexpected un-indent using a style combining `AlignOperands: DontAlign` with `BreakBeforeTernaryOperators: false`, such as Google's JavaScript style:
```
% bin/clang-format -style=google ~/test.js
aaaaaaaaaaa = bbbbbbbb ? cccccccccccccccccc() :
dddddddddd ? eeeeeeeeeeeeee :
fffff;
```
The issue lies with the interaction of `AlignOperands: DontAlign` and the edited code section in ContinuationIndenter.cpp, which de-dents the intent by `Style.ContinuationIndentWidth`. From [[ https://github.com/llvm/llvm-project/blob/ac3e5c4d93fbe7fb2db3c745c721aff41cc1b851/clang/include/clang/Format/Format.h#L170 | the documentation ]] of AlignOperands: DontAlign:
> The wrapped lines are indented `ContinuationIndentWidth` spaces from the start of the line.
So the de-dent effectively erases the necessary `ContinuationIndentWidth` in that case.
This patch restores the `AlignOperands: DontAlign` behavior, producing:
```
% bin/clang-format -style=google ~/test.js
aaaaaaaaaaa = bbbbbbbb ? cccccccccccccccccc() :
dddddddddd ? eeeeeeeeeeeeee :
fffff;
```
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D82199
Added:
Modified:
clang/lib/Format/ContinuationIndenter.cpp
clang/unittests/Format/FormatTest.cpp
Removed:
################################################################################
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index 2ac43b6cfa4b..b1497651a8fe 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -1041,8 +1041,10 @@ unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) {
// * not remove the 'lead' ContinuationIndentWidth
// * always un-indent by the operator when
// BreakBeforeTernaryOperators=true
- unsigned Indent =
- State.Stack.back().Indent - Style.ContinuationIndentWidth;
+ unsigned Indent = State.Stack.back().Indent;
+ if (Style.AlignOperands != FormatStyle::OAS_DontAlign) {
+ Indent -= Style.ContinuationIndentWidth;
+ }
if (Style.BreakBeforeTernaryOperators &&
State.Stack.back().UnindentOperator)
Indent -= 2;
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index dce71b0f0692..beaa68a24617 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -6281,6 +6281,17 @@ TEST_F(FormatTest, BreaksConditionalExpressions) {
" : bbbbbbbbbbbbbbbbbbbbbbb ? 2222222222222222\n"
" : 3333333333333333;",
Style);
+
+ Style.AlignOperands = FormatStyle::OAS_DontAlign;
+ Style.BreakBeforeTernaryOperators = false;
+ // FIXME: Aligning the question marks is weird given DontAlign.
+ // Consider disabling this alignment in this case. Also check whether this
+ // will render the adjustment from https://reviews.llvm.org/D82199
+ // unnecessary.
+ verifyFormat("int x = aaaaaaaaaaaaaaa ? aaaaaaaaaaaaaaaaaa :\n"
+ " bbbb ? cccccccccccccccccc :\n"
+ " ddddd;\n",
+ Style);
}
TEST_F(FormatTest, BreaksConditionalExpressionsAfterOperator) {
More information about the cfe-commits
mailing list