[PATCH] D40068: Implement more accurate penalty & trade-offs while breaking protruding tokens.

Krasimir Georgiev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 15 03:34:02 PST 2017


krasimir added inline comments.


================
Comment at: lib/Format/ContinuationIndenter.cpp:1517
     ReflowInProgress = SplitBefore.first != StringRef::npos;
+    DEBUG(if (ReflowInProgress) llvm::dbgs() << "  Reflowing.\n");
     TailOffset =
----------------
I'd rather
```
DEBUG({
  if (ReflowInProgress)
    llvm::dbgs() << "  Reflowing.\n";
});
```


================
Comment at: unittests/Format/FormatTest.cpp:8007
+            "    \"aaabbbcc ddde \"\n"
+            "    \"efff\");",
             format("someFunction(\"aaabbbcc ddde efff\");",
----------------
Why did the string got on a newline here?


================
Comment at: unittests/Format/FormatTestComments.cpp:2447
             "      a);",
-            format("a = f(/* long long */ a);", getLLVMStyleWithColumns(15)));
+            format("a = f(/* long long */ a);", getLLVMStyleWithColumns(16)));
 
----------------
Please also leave the old test case, which is about the `*/` just going over the column limit if not put on a newline.


================
Comment at: unittests/Format/FormatTestComments.cpp:2456
                    "       */a);",
-                   getLLVMStyleWithColumns(15)));
+                   getLLVMStyleWithColumns(16)));
 
----------------
Why change `15` to `16`? The same for the subsequent cases.


================
Comment at: unittests/Format/FormatTestComments.cpp:2481
       "      b);",
-      format("a = f(a, /* long long */ b);", getLLVMStyleWithColumns(15)));
+      format("a = f(a, /* long long */ b);", getLLVMStyleWithColumns(16)));
 
----------------
Please also leave the old test case, which is about the `*/` just going over the column limit if not put on a newline.


================
Comment at: unittests/Format/FormatTestComments.cpp:2488
       "      (1 + 1));",
-      format("a = f(a, /* long long */ (1 + 1));", getLLVMStyleWithColumns(15)));
+      format("a = f(a, /* long long */ (1 + 1));", getLLVMStyleWithColumns(16)));
 }
----------------
Please also leave the old test case, which is about the `*/` just going over the column limit if not put on a newline.


https://reviews.llvm.org/D40068





More information about the cfe-commits mailing list