[clang-format] use spaces for alignment of binary/ternary expressions with UT_AlignWithSpaces

[fickert]

Summary

Use spaces to align binary and ternary expressions when using AlignOperands and UT_AlignWithSpaces.

[fickert]

This fixes an oversight in the new UT_AlignWithSpaces option (see D75034), which did not correctly identify the alignment of binary/ternary expressions.

[curdeius]

clang/lib/Format/ContinuationIndenter.cpp

1362–1367

You could factor out the part of the condition that doesn't concern AlignOperands to get rid of the repetition wrt. the previous if.

[fickert]

Resolved repetitive conditions in the two consecutive ifs and added a small comment.

[curdeius]

LGTM. Thank you for the patch!

[fickert]

Thanks! I don't have push permissions to the repository so I cannot submit the commit myself (Maximilian Fickert <[e-mail]>).

[Typz]

Could this also be causing https://bugs.llvm.org/show_bug.cgi?id=33896 ?

I'll try to see if changes something in that case...

[Typz]

In D85600#2209545, @Typz wrote:

Could this also be causing https://bugs.llvm.org/show_bug.cgi?id=33896 ?

I'll try to see if changes something in that case...

Actually, I can confirm this is unrelated.

clang/lib/Format/ContinuationIndenter.cpp

1364

bad indent here : should be space-indented.

[fickert]

Yes I think it's unrelated, though this patch should fix the alignment in that example when using the UT_AlignWithSpaces option.

[fickert]

clang/lib/Format/ContinuationIndenter.cpp

1364

Can you clarify? This is formatted with the LLVM clang-format.

[Typz]

clang/lib/Format/ContinuationIndenter.cpp

1364

Phabricator shows some kind of >> symbol at the beginning of this line, which i interpreted as a TAB character, hence my comment.
But indeed no issue after looking at the raw file, sorry about this.

[sylvestre.ledru]

I am not 100% that it is thanks to this patch but reformatting Firefox code with clang-format 11 significantly improves the readability of the usage
of ternary operators. See: https://phabricator.services.mozilla.com/D90795

Thanks for the change!

clang/lib/Format/ContinuationIndenter.cpp

if (Style.AlignOperands == FormatStyle::OAS_AlignAfterOperator &&
         Previous &&
         (Previous->getPrecedence() == prec::Assignment ||
          Previous->is(tok::kw_return) ||
          (*I == prec::Conditional && Previous->is(tok::question) &&
           Previous->is(TT_ConditionalExpr))) &&
         !Newline)
       NewParenState.UnindentOperator = true;
 
+    if (Style.AlignOperands != FormatStyle::OAS_DontAlign && Previous &&
+        (Previous->getPrecedence() == prec::Assignment ||
+         Previous->is(tok::kw_return) ||
+         (*I == prec::Conditional && Previous->is(tok::question) &&
+          Previous->is(TT_ConditionalExpr))) &&
+        !Newline)
+      NewParenState.IsAligned = true;
+
     // Do not indent relative to the fake parentheses inserted for "." or "->".
     // This is a special case to make the following to statements consistent:
     //   OuterFunction(InnerFunctionCall( // break
     //       ParameterToInnerFunction));
     //   OuterFunction(SomeObject.InnerFunctionCall( // break
     //       ParameterToInnerFunction));
     if (*I > prec::Unknown)
       NewParenState.LastSpace = std::max(NewParenState.LastSpace, State.Column);

clang/unittests/Format/FormatTest.cpp

verifyFormat("class Assign {\n"
                "\tvoid f() {\n"
                "\t\tint         x      = 123;\n"
                "\t\tint         random = 4;\n"
                "\t\tstd::string alphabet =\n"
                "\t\t\t\"abcdefghijklmnopqrstuvwxyz\";\n"
                "\t}\n"
                "};",
                Tab);
+  Tab.AlignOperands = FormatStyle::OAS_Align;
+  verifyFormat("int aaaaaaaaaa = bbbbbbbbbbbbbbbbbbbb +\n"
+               "                 cccccccccccccccccccc;",
+               Tab);
+  // no alignment
+  verifyFormat("int aaaaaaaaaa =\n"
+               "\tbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb;",
+               Tab);
+  verifyFormat("return aaaaaaaaaaaaaaaa ? 111111111111111\n"
+               "       : bbbbbbbbbbbbbb ? 222222222222222\n"
+               "                        : 333333333333333;",
+               Tab);
+  Tab.BreakBeforeBinaryOperators = FormatStyle::BOS_All;
+  Tab.AlignOperands = FormatStyle::OAS_AlignAfterOperator;
+  verifyFormat("int aaaaaaaaaa = bbbbbbbbbbbbbbbbbbbb\n"
+               "               + cccccccccccccccccccc;",
+               Tab);
 }
 
 TEST_F(FormatTest, ZeroTabWidth) {
   FormatStyle Tab = getLLVMStyleWithColumns(42);
   Tab.IndentWidth = 8;
   Tab.UseTab = FormatStyle::UT_Never;
   Tab.TabWidth = 0;
   EXPECT_EQ("void a(){\n"