krasimir created this revision.
Herald added subscribers: cfe-commits, klimek.

TODO


Repository:
  rC Clang

https://reviews.llvm.org/D46757

Files:
  lib/Format/ContinuationIndenter.cpp
  lib/Format/TokenAnnotator.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  unittests/Format/FormatTestProto.cpp
  unittests/Format/FormatTestRawStrings.cpp
  unittests/Format/FormatTestTextProto.cpp

Index: unittests/Format/FormatTestTextProto.cpp
===================================================================
--- unittests/Format/FormatTestTextProto.cpp
+++ unittests/Format/FormatTestTextProto.cpp
@@ -171,17 +171,48 @@
   verifyFormat("msg_field < field_a < field_b <> > >");
   verifyFormat("msg_field: < field_a < field_b: <> > >");
   verifyFormat("msg_field < field_a: OK, field_b: \"OK\" >");
-  verifyFormat("msg_field < field_a: OK field_b: <>, field_c: OK >");
-  verifyFormat("msg_field < field_a { field_b: 1 }, field_c: < f_d: 2 > >");
   verifyFormat("msg_field: < field_a: OK, field_b: \"OK\" >");
-  verifyFormat("msg_field: < field_a: OK field_b: <>, field_c: OK >");
-  verifyFormat("msg_field: < field_a { field_b: 1 }, field_c: < fd_d: 2 > >");
-  verifyFormat("field_a: \"OK\", msg_field: < field_b: 123 >, field_c: {}");
-  verifyFormat("field_a < field_b: 1 >, msg_fid: < fiel_b: 123 >, field_c <>");
-  verifyFormat("field_a < field_b: 1 > msg_fied: < field_b: 123 > field_c <>");
-  verifyFormat("field < field < field: <> >, field <> > field: < field: 1 >");
-
   // Multiple lines tests
+  verifyFormat("msg_field <\n"
+               "  field_a: OK\n"
+               "  field_b: <>,\n"
+               "  field_c: OK\n"
+               ">");
+
+  verifyFormat("msg_field <\n"
+               "  field_a { field_b: 1 },\n"
+               "  field_c: < f_d: 2 >\n"
+               ">");
+
+  verifyFormat("msg_field: <\n"
+               "  field_a: OK\n"
+               "  field_b: <>,\n"
+               "  field_c: OK\n"
+               ">");
+
+  verifyFormat("msg_field: <\n"
+               "  field_a { field_b: 1 },\n"
+               "  field_c: < fd_d: 2 >\n"
+               ">");
+
+  verifyFormat("field_a: \"OK\",\n"
+               "msg_field: < field_b: 123 >,\n"
+               "field_c: {}");
+
+  verifyFormat("field_a < field_b: 1 >,\n"
+               "msg_fid: < fiel_b: 123 >,\n" 
+               "field_c <>");
+
+  verifyFormat("field_a < field_b: 1 >\n"
+               "msg_fied: < field_b: 123 >\n"
+               "field_c <>");
+
+  verifyFormat("field <\n"
+               "  field < field: <> >,\n"
+               "  field <>\n"
+               ">\n"
+               "field: < field: 1 >");
+
   verifyFormat("msg_field <\n"
                "  field_a: OK\n"
                "  field_b: \"OK\"\n"
@@ -242,7 +273,10 @@
                "  field_d: ok\n"
                "}");
 
-  verifyFormat("field_a: < f1: 1, f2: <> >\n"
+  verifyFormat("field_a: <\n"
+               "  f1: 1,\n"
+               "  f2: <>\n"
+               ">\n"
                "field_b <\n"
                "  field_b1: <>\n"
                "  field_b2: ok,\n"
@@ -507,5 +541,102 @@
                ">");
 }
 
+TEST_F(FormatTestTextProto, BreaksEntriesOfSubmessagesContainingSubmessages) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_TextProto);
+  Style.ColumnLimit = 60;
+  // The column limit allows for the keys submessage to be put on 1 line, but we
+  // break it since it contains a submessage an another entry.
+  verifyFormat("key: valueeeeeeee\n"
+               "keys: {\n"
+               "  item: 'aaaaaaaaaaaaaaaa'\n"
+               "  sub <>\n"
+               "}");
+  verifyFormat("key: valueeeeeeee\n"
+               "keys: {\n"
+               "  item: 'aaaaaaaaaaaaaaaa'\n"
+               "  sub {}\n"
+               "}");
+  verifyFormat("key: valueeeeeeee\n"
+               "keys: {\n"
+               "  sub {}\n"
+               "  sub: <>\n"
+               "  sub: []\n"
+               "}");
+  verifyFormat("key: valueeeeeeee\n"
+               "keys: {\n"
+               "  item: 'aaaaaaaaaaa'\n"
+               "  sub { msg: 1 }\n"
+               "}");
+  verifyFormat("key: valueeeeeeee\n"
+               "keys: {\n"
+               "  item: 'aaaaaaaaaaa'\n"
+               "  sub: { msg: 1 }\n"
+               "}");
+  verifyFormat("key: valueeeeeeee\n"
+               "keys: {\n"
+               "  item: 'aaaaaaaaaaa'\n"
+               "  sub < msg: 1 >\n"
+               "}");
+  verifyFormat("key: valueeeeeeee\n"
+               "keys: {\n"
+               "  item: 'aaaaaaaaaaa'\n"
+               "  sub: [ msg: 1 ]\n"
+               "}");
+  verifyFormat("key: valueeeeeeee\n"
+               "keys: <\n"
+               "  item: 'aaaaaaaaaaa'\n"
+               "  sub: [ 1, 2 ]\n"
+               ">");
+  verifyFormat("key: valueeeeeeee\n"
+               "keys: {\n"
+               "  sub {}\n"
+               "  item: 'aaaaaaaaaaaaaaaa'\n"
+               "}");
+  verifyFormat("key: valueeeeeeee\n"
+               "keys: {\n"
+               "  sub: []\n"
+               "  item: 'aaaaaaaaaaaaaaaa'\n"
+               "}");
+  verifyFormat("key: valueeeeeeee\n"
+               "keys: {\n"
+               "  sub <>\n"
+               "  item: 'aaaaaaaaaaaaaaaa'\n"
+               "}");
+  verifyFormat("key: valueeeeeeee\n"
+               "keys: {\n"
+               "  sub { key: value }\n"
+               "  item: 'aaaaaaaaaaaaaaaa'\n"
+               "}");
+  verifyFormat("key: valueeeeeeee\n"
+               "keys: {\n"
+               "  sub: [ 1, 2 ]\n"
+               "  item: 'aaaaaaaaaaaaaaaa'\n"
+               "}");
+  verifyFormat("key: valueeeeeeee\n"
+               "keys: {\n"
+               "  sub < sub_2: {} >\n"
+               "  item: 'aaaaaaaaaaaaaaaa'\n"
+               "}");
+  verifyFormat("key: valueeeeeeee\n"
+               "keys: {\n"
+               "  item: data\n"
+               "  sub: [ 1, 2 ]\n"
+               "  item: 'aaaaaaaaaaaaaaaa'\n"
+               "}");
+  verifyFormat("key: valueeeeeeee\n"
+               "keys: {\n"
+               "  item: data\n"
+               "  sub < sub_2: {} >\n"
+               "  item: 'aaaaaaaaaaaaaaaa'\n"
+               "}");
+  verifyFormat("sub: {\n"
+               "  key: valueeeeeeee\n"
+               "  keys: {\n"
+               "    sub: [ 1, 2 ]\n"
+               "    item: 'aaaaaaaaaaaaaaaa'\n"
+               "  }\n"
+               "}");
+}
+
 } // end namespace tooling
 } // end namespace clang
Index: unittests/Format/FormatTestRawStrings.cpp
===================================================================
--- unittests/Format/FormatTestRawStrings.cpp
+++ unittests/Format/FormatTestRawStrings.cpp
@@ -199,12 +199,6 @@
       format(
           R"test(P p = TP(R"pb(item_1:1 item_2:2)pb");)test",
           getRawStringPbStyleWithColumns(40)));
-  expect_eq(
-      R"test(P p = TP(R"pb(item_1 < 1 > item_2: { 2 })pb");)test",
-      format(
-          R"test(P p = TP(R"pb(item_1<1> item_2:{2})pb");)test",
-          getRawStringPbStyleWithColumns(40)));
-
   // Merge two short lines into one.
   expect_eq(R"test(
 std::string s = R"pb(
@@ -220,6 +214,18 @@
                    getRawStringPbStyleWithColumns(40)));
 }
 
+TEST_F(FormatTestRawStrings, BreaksShortRawStringsWhenNeeded) {
+  // The raw string contains multiple submessage entries, so break for
+  // readability.
+  expect_eq(R"test(
+P p = TP(R"pb(item_1 < 1 >
+              item_2: { 2 })pb");)test",
+      format(
+          R"test(
+P p = TP(R"pb(item_1<1> item_2:{2})pb");)test",
+          getRawStringPbStyleWithColumns(40)));
+}
+
 TEST_F(FormatTestRawStrings, BreaksRawStringsExceedingColumnLimit) {
   expect_eq(R"test(
 P p = TPPPPPPPPPPPPPPP(
Index: unittests/Format/FormatTestProto.cpp
===================================================================
--- unittests/Format/FormatTestProto.cpp
+++ unittests/Format/FormatTestProto.cpp
@@ -493,5 +493,136 @@
                "};");
 }
 
+TEST_F(FormatTestProto, BreaksEntriesOfSubmessagesContainingSubmessages) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_TextProto);
+  Style.ColumnLimit = 60;
+  // The column limit allows for the keys submessage to be put on 1 line, but we
+  // break it since it contains a submessage an another entry.
+  verifyFormat("option (MyProto.options) = {\n"
+               "  key: valueeeeeeee\n"
+               "  keys: {\n"
+               "    item: 'aaaaaaaaaaaaaaaa'\n"
+               "    sub <>\n"
+               "  }\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  key: valueeeeeeee\n"
+               "  keys: {\n"
+               "    item: 'aaaaaaaaaaaaaaaa'\n"
+               "    sub {}\n"
+               "  }\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  key: valueeeeeeee\n"
+               "  keys: {\n"
+               "    sub {}\n"
+               "    sub: <>\n"
+               "    sub: []\n"
+               "  }\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  key: valueeeeeeee\n"
+               "  keys: {\n"
+               "    item: 'aaaaaaaaaaa'\n"
+               "    sub { msg: 1 }\n"
+               "  }\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  key: valueeeeeeee\n"
+               "  keys: {\n"
+               "    item: 'aaaaaaaaaaa'\n"
+               "    sub: { msg: 1 }\n"
+               "  }\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  key: valueeeeeeee\n"
+               "  keys: {\n"
+               "    item: 'aaaaaaaaaaa'\n"
+               "    sub < msg: 1 >\n"
+               "  }\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  key: valueeeeeeee\n"
+               "  keys: {\n"
+               "    item: 'aaaaaaaaaaa'\n"
+               "    sub: [ msg: 1 ]\n"
+               "  }\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  key: valueeeeeeee\n"
+               "  keys: <\n"
+               "    item: 'aaaaaaaaaaa'\n"
+               "    sub: [ 1, 2 ]\n"
+               "  >\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  key: valueeeeeeee\n"
+               "  keys: {\n"
+               "    sub {}\n"
+               "    item: 'aaaaaaaaaaaaaaaa'\n"
+               "  }\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  key: valueeeeeeee\n"
+               "  keys: {\n"
+               "    sub: []\n"
+               "    item: 'aaaaaaaaaaaaaaaa'\n"
+               "  }\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  key: valueeeeeeee\n"
+               "  keys: {\n"
+               "    sub <>\n"
+               "    item: 'aaaaaaaaaaaaaaaa'\n"
+               "  }\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  key: valueeeeeeee\n"
+               "  keys: {\n"
+               "    sub { key: value }\n"
+               "    item: 'aaaaaaaaaaaaaaaa'\n"
+               "  }\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  key: valueeeeeeee\n"
+               "  keys: {\n"
+               "    sub: [ 1, 2 ]\n"
+               "    item: 'aaaaaaaaaaaaaaaa'\n"
+               "  }\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  key: valueeeeeeee\n"
+               "  keys: {\n"
+               "    sub < sub_2: {} >\n"
+               "    item: 'aaaaaaaaaaaaaaaa'\n"
+               "  }\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  key: valueeeeeeee\n"
+               "  keys: {\n"
+               "    item: data\n"
+               "    sub: [ 1, 2 ]\n"
+               "    item: 'aaaaaaaaaaaaaaaa'\n"
+               "  }\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  key: valueeeeeeee\n"
+               "  keys: {\n"
+               "    item: data\n"
+               "    sub < sub_2: {} >\n"
+               "    item: 'aaaaaaaaaaaaaaaa'\n"
+               "  }\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  sub: {\n"
+               "    key: valueeeeeeee\n"
+               "    keys: {\n"
+               "      sub: [ 1, 2 ]\n"
+               "      item: 'aaaaaaaaaaaaaaaa'\n"
+               "    }\n"
+               "  }\n"
+               "}");
+}
+
 } // end namespace tooling
 } // end namespace clang
Index: lib/Format/UnwrappedLineFormatter.cpp
===================================================================
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -14,6 +14,10 @@
 #include <queue>
 
 #define DEBUG_TYPE "format-formatter"
+#define DBG(A)                                                                 \
+  DEBUG({                                                                      \
+    llvm::dbgs() << __func__ << ":" << __LINE__ << #A << " = " << A << "\n";   \
+  });
 
 namespace clang {
 namespace format {
Index: lib/Format/TokenAnnotator.cpp
===================================================================
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -19,6 +19,10 @@
 #include "llvm/Support/Debug.h"
 
 #define DEBUG_TYPE "format-token-annotator"
+#define DBG(A)                                                                 \
+  DEBUG({                                                                      \
+    llvm::dbgs() << __func__ << ":" << __LINE__ << #A << " = " << A << "\n";   \
+  });
 
 namespace clang {
 namespace format {
@@ -2914,6 +2918,59 @@
   if (Right.is(TT_ProtoExtensionLSquare))
     return true;
 
+  // If a submessage A contains a submessage B and has at least 2 entries,
+  // put all of the entries of A on separate lines by forcing the selector of
+  // the submessage to be put on a newline.
+  //
+  // Example: these can stay on one line:
+  // a { scalar_1: 1 scalar_2: 2 }
+  // a { b { key: value } }
+  //
+  // and these need to be broken:
+  // a {
+  //   scalar: 1
+  //   b { key: value }
+  // }
+  //
+  // Be careful to exclude cases like [proto.ext] { ..., since the r_square is
+  // a TT_SelectorName there.
+  if ((Style.Language == FormatStyle::LK_Proto ||
+       Style.Language == FormatStyle::LK_TextProto) &&
+      Right.is(TT_SelectorName) && !Right.is(tok::r_square) && Right.Next) {
+    // Look for the scope opener after selector in cases like:
+    // selector { ...
+    // selector: { ...
+    FormatToken *LParen =
+        Right.Next->is(tok::colon) ? Right.Next->Next : Right.Next;
+    if (LParen &&
+        // The scope opener looks like one of {, [, <:
+        // selector { ... }
+        // selector [ ... ]
+        // selector < ... >
+        //
+        // In case of selector { ... }, the l_brace is TT_DictLiteral.
+        // In case of selector {}, the l_brace is not TT_DictLiteral, so we
+        // check for immediately following r_brace.
+        ((LParen->is(tok::l_brace) &&
+          (LParen->is(TT_DictLiteral) ||
+           (LParen->Next && LParen->Next->is(tok::r_brace)))) ||
+         LParen->is(TT_ArrayInitializerLSquare) || LParen->is(tok::less))) {
+      // If the submessage is the second or later entry, we're done.
+      if (Left.ParameterCount != 1)
+        return true;
+      // However, is the submessage is the first entry in a submessage,
+      // ParameterCount is 1. We deal with this case later by detecting an entry
+      // following a closing paren of this submessage.
+    }
+    // If this is an entry immediately following a submessage, it will be
+    // preceeded by a closing paren of that submessage, like in:
+    //     left---.  .---right
+    //            v  v
+    // sub: { ... } key: value
+    if (Left.isOneOf(tok::r_brace, tok::greater, tok::r_square))
+      return true;
+  }
+
   return false;
 }
 
Index: lib/Format/ContinuationIndenter.cpp
===================================================================
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -22,6 +22,10 @@
 #include "llvm/Support/Debug.h"
 
 #define DEBUG_TYPE "format-indenter"
+#define DBG(A)                                                                 \
+  DEBUG({                                                                      \
+    llvm::dbgs() << __func__ << ":" << __LINE__ << #A << " = " << A << "\n";   \
+  });
 
 namespace clang {
 namespace format {
@@ -352,14 +356,31 @@
       State.Stack.back().BreakBeforeParameter && !Current.isTrailingComment() &&
       !Current.isOneOf(tok::r_paren, tok::r_brace))
     return true;
+
+  /*
+  if (Style.Language == FormatStyle::LK_TextProto) {
+    if (Current.is(TT_SelectorName) && Current.Next) {
+      FormatToken *LBrace =
+          Current.Next->is(tok::comma) ? Current.Next->Next : Current.Next;
+      if (LBrace && ((LBrace->is(TT_DictLiteral) && LBrace->is(tok::l_brace)) ||
+                     LBrace->is(TT_ArrayInitializerLSquare) ||
+                     opensProtoMessageField(*LBrace, Style))) {
+        DBG(Current.TokenText);
+        if (Previous.ParameterCount != 1)
+          return true;
+      }
+    }
+  }
+  */
   if (((Previous.is(TT_DictLiteral) && Previous.is(tok::l_brace)) ||
        (Previous.is(TT_ArrayInitializerLSquare) &&
         Previous.ParameterCount > 1) ||
        opensProtoMessageField(Previous, Style)) &&
-      Style.ColumnLimit > 0 &&
-      getLengthToMatchingParen(Previous, State.Stack) + State.Column - 1 >
+      Style.ColumnLimit > 0) {
+    if (getLengthToMatchingParen(Previous, State.Stack) + State.Column - 1 >
           getColumnLimit(State))
-    return true;
+      return true;
+  }
 
   const FormatToken &BreakConstructorInitializersToken =
       Style.BreakConstructorInitializers == FormatStyle::BCIS_AfterColon
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to