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

[Daniel Jasper via Phabricator via cfe-commits]

djasper added a comment.

In https://reviews.llvm.org/D32478#759347, @Typz wrote:

> In https://reviews.llvm.org/D32478#758258, @djasper wrote:
>
> > When you say "this doesn't happen in tests", do you mean this never happens when there are parentheses around the expression?
>
>
> By 'test' I meant 'conditional construct' : it happens when there are parentheses around the expression, but it does not happen when these parentheses are the parentheses from a `if (...)`


Are you sure? From reading the code, it seems that this happens exactly after "=" and "return". What's the behavior for function calls?

  function(aaaaaaa //
  + bbbbb);

Or for expressions with just parens?

  int a = (aaaaaa //
  + bbbbbb);

What if doing this would violate the ContinuationIndentWidth?

  T t = aaaaa //
  && bbbbb;



================
Comment at: lib/Format/ContinuationIndenter.cpp:759
+    return State.Stack.back().Indent - Current.Tok.getLength()
+        - Current.SpacesRequiredBefore;
   if (State.Stack.back().Indent == State.FirstIndent && PreviousNonComment &&
----------------
Fix style.


================
Comment at: lib/Format/ContinuationIndenter.cpp:949
+         Previous->is(tok::kw_return)))
+      NewParenState.UnindentOperator = true;
 
----------------
I am not yet convinced you need a new flag in ParenState here. I guess you need it because the operators can have varying length and so you cannot just change LastSpace here?


https://reviews.llvm.org/D32478