[PATCH] Implement string literal breaking on unbreakable token sequences.

[Daniel Jasper]

================
Comment at: lib/Format/Format.cpp:763
@@ -762,1 +762,3 @@
+                                bool DryRun,
+                                unsigned TrailingUnbreakableLength) {
     llvm::OwningPtr<BreakableToken> Token;
----------------
consider 'UnbreakableTailLength'

================
Comment at: lib/Format/Format.cpp:756
@@ -756,1 +755,3 @@
+    return breakProtrudingToken(Current, State, DryRun,
+                                /*TrailingUnbreakableLength=*/ 0);
   }
----------------
This might be a good use case for a default parameter!?

================
Comment at: lib/Format/Format.cpp:790
@@ +789,3 @@
+      // Thus, we recursively search backwards to try to find a breakable token.
+      if (State.Column <= getColumnLimit() ||
+          Current.CanBreakBefore || !Current.Parent)
----------------
To me, this if is not self-explanatory. The comment above basically describes the subsequent recursive call.

As for the details:
- Why is the "State.Column <= getColumnLimit()" not the first thing breakProtrudingToken does?
- If we can break before the current token, then we will find the same solution using a different path in analyzeSolutionSpace(). Shouldn't we then return a very high penalty here so this branch is not used?
- If this token does not have a parent, doesn't that mean that we failed and should return a very high penalty (as opposed to 0)? Or is the penalty for the protruding characters added elsewhere?


================
Comment at: lib/Format/Format.cpp:835
@@ -819,1 +834,3 @@
     if (BreakInserted) {
+      // FIXME: Do we want to add TrailingUnbreakableLength here?
+      State.Column = PositionAfterLastLineInToken;
----------------
I'd give yes 2:1 odds :-)

Testcase might look like:

  #define A \
    string s = "String with exactly 72 chars"; \
    int i;

================
Comment at: unittests/Format/FormatTest.cpp:3935
@@ -3934,1 +3934,3 @@
 
+TEST_F(FormatTest, BreakStringLiteralsBeforeUnbreakableTokenSequence) {
+  EXPECT_EQ("someFunction(\"aaaaaaaaaa\"\n"
----------------
Just FYI, using trailing line comments might give you slightly longer tokens before which clang-format won't break.


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

BRANCH
  break-strings-off-by-one

ARCANIST PROJECT
  clang