[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

[Daniel Jasper via Phabricator via cfe-commits]

djasper added a comment.

When you say "this doesn't happen in tests", do you mean this never happens when there are parentheses around the expression?



================
Comment at: unittests/Format/FormatTest.cpp:2476
       "bool value = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
-      "                     + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
-      "                     + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
-      "                 == aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
-      "                            * bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb\n"
-      "                        + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb\n"
-      "             && aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
-      "                        * aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
-      "                    > ccccccccccccccccccccccccccccccccccccccccc;",
+      "                   + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
+      "                   + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
----------------
Typz wrote:
> Typz wrote:
> > djasper wrote:
> > > This looks very inconsistent to me.
> > not sure what you mean, I do not really understand how this expression was aligned before the patch...
> > it is not so much better in this case with the patch, but the '&&' is actually right-aligned with the '=' sign.
> Seeing the test just before, I see (when breaking after operators) that the operands are actually right-aligned, e.g. all operators are on the same column.
> 
> So should it not be the same when breaking before the operator as well (independently from my patch, actually)?
> 
>   bool value = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
>                      + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
>                      + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
>                 == aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
>                          * bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb\n"
>                      + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb\n"
>             && aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
>                      * aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
>                  > ccccccccccccccccccccccccccccccccccccccccc;
> 
> Not sure I like this right-alignment thing, but at least I start to understand how we get this output (and this may be another option to prefer left-alignment?)
The right alignment is not relevant. I think I just got "playful" when writing this test.

What's happening here is that we align the operators of subsequent lines to the previous operand. You are right that this is not intuitive wrt. the option's name, but it is the behavior that we intended and that we had seen in styles that use this.

Now, what we also do is add another ContinuationIndentWidth to highlight operator precedence. Basically think of this as replacing parentheses that you would put there instead:

  int x = (aaaaa
           * bbbbb) // The "*" is intendet a space because of the paren.
          + ccccc;

  int x = aaaaa
              * bbbbb // Instead we use ContinuationIndentWidth here.
          + ccccc;

What I mean about this being inconsistent is that now you align the highest-level operands because as you say that's what's expected from the option, but you still don't align other precedences, which seems wrong. If you really wanted to align all operands, that would look like:

  bool value = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
             + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
             + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
            == aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
             * bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
             + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
            && aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
             * aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
             > ccccccccccccccccccccccccccccccccccccccccc;

But then, that's really a tough expression to understand. I mean, this is a fabricated example and possibly doesn't happen often in real life, but I am slightly worried about it and the inconsistency here.


https://reviews.llvm.org/D32478