Don't insert confusing line breaks in comparisons.

In general, clang-format breaks after an operator if the LHS spans
multiple lines. Otherwise, this can lead to confusing effects and
effectively hide the operator precendence, e.g. in

if (aaaaaaaaaaaaaa ==
        bbbbbbbbbbbbbb && c) { ...

This patch removes this rule for comparisons, if the LHS is not a binary
expression itself as many users were wondering why clang-format inserts
an unnecessary linebreak.

Before:
if (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(
        aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) >
    5) { ...

After:
if (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(
        aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) > 5) { ...

In the long run, we might:
- Want to do this for other binary expressions as well.
- Do this only if the RHS is short or even only if it is a literal.

llvm-svn: 185530

1 files changed, 16 insertions, 2 deletions

diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 9cfb04162e8..ef871389fb6 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -1066,9 +1066,23 @@ private:
       return true;
 
     // If we need to break somewhere inside the LHS of a binary expression, we
-    // should also break after the operator.
+    // should also break after the operator. Otherwise, the formatting would
+    // hide the operator precedence, e.g. in:
+    //   if (aaaaaaaaaaaaaa ==
+    //           bbbbbbbbbbbbbb && c) {..
+    // For comparisons, we only apply this rule, if the LHS is a binary
+    // expression itself as otherwise, the line breaks seem superfluous.
+    // We need special cases for ">>" which we have split into two ">" while
+    // lexing in order to make template parsing easier.
+    bool IsComparison = (Previous.getPrecedence() == prec::Relational ||
+                         Previous.getPrecedence() == prec::Equality) &&
+                        Previous.Previous &&
+                        Previous.Previous->Type != TT_BinaryOperator; // For >>.
+    bool LHSIsBinaryExpr =
+        Previous.Previous && Previous.Previous->FakeRParens > 0;
     if (Previous.Type == TT_BinaryOperator &&
-        Current.Type != TT_BinaryOperator && // Special case for ">>".
+        (!IsComparison || LHSIsBinaryExpr) &&
+        Current.Type != TT_BinaryOperator && // For >>.
         !Current.isTrailingComment() &&
         !Previous.isOneOf(tok::lessless, tok::question) &&
         Previous.getPrecedence() != prec::Assignment &&