[cfe-commits] [PATCH] Formatter: Give '.' after ')' a smaller split penalty than ', '

[Nico Weber]

Hi djasper,

').' is likely part of a builder pattern statement, like

int foo() {
  return llvm::StringSwitch<Reference::Kind>(name)
    .StartsWith(".eh_frame_hdr", ORDER_EH_FRAMEHDR)
    .StartsWith(".eh_frame", ORDER_EH_FRAME)
    .StartsWith(".init", ORDER_INIT)
    .StartsWith(".fini", ORDER_FINI)
    .StartsWith(".hash", ORDER_HASH)
    .Default(ORDER_TEXT);
}

(from PR14818). This is currently formatted poorly, because splitting after ',' is much cheaper than splitting before '.'. This patch gives '.' a smaller penalty than ',' if it's after a ')'.

http://llvm-reviews.chandlerc.com/D310

Files:
  lib/Format/Format.cpp
  unittests/Format/FormatTest.cpp

Index: lib/Format/Format.cpp
===================================================================
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -618,8 +618,11 @@
     if (Level != prec::Unknown)
       return Level;
 
-    if (Right.is(tok::arrow) || Right.is(tok::period))
+    if (Right.is(tok::arrow) || Right.is(tok::period)) {
+      if (Left.is(tok::r_paren))
+        return 10; // Should be smaller than tok::comma above.
       return 150;
+    }
 
     return 3;
   }
Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -905,9 +905,8 @@
       "         aaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n"
       "             aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa)));");
   verifyGoogleFormat(
-      "aaaaaaaaaaaaaaa(aaaaaaaaa,\n"
-      "                aaaaaaaaa,\n"
-      "                aaaaaaaaaaaaaaaaaaaaaaa).aaaaaaaaaaaaaaaaaa();");
+      "aaaaaaaaaaaaaaa(aaaaaaaaa, aaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaa)\n"
+      "    .aaaaaaaaaaaaaaaaaa();");
   verifyGoogleFormat(
       "somefunction(someotherFunction(ddddddddddddddddddddddddddddddddddd,\n"
       "                               ddddddddddddddddddddddddddddd),\n"
@@ -922,6 +921,15 @@
                      "  a);");
 }
 
+TEST_F(FormatTest, FormatsBuilderPattern) {
+  verifyFormat(
+      "return llvm::StringSwitch<Reference::Kind>(name)\n"
+      "       .StartsWith(\".eh_frame_hdr\", ORDER_EH_FRAMEHDR)\n"
+      "       .StartsWith(\".eh_frame\", ORDER_EH_FRAME).StartsWith(\".init\", ORDER_INIT)\n"
+      "       .StartsWith(\".fini\", ORDER_FINI).StartsWith(\".hash\", ORDER_HASH)\n"
+      "       .Default(ORDER_TEXT);\n");
+}
+
 TEST_F(FormatTest, DoesNotBreakTrailingAnnotation) {
   verifyFormat("void aaaaaaaaaaaa(int aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa)\n"
                "    GUARDED_BY(aaaaaaaaaaaaa);");
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D310.1.patch
Type: text/x-patch
Size: 1951 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130117/e51589bb/attachment.bin>