[PATCH] Don't break line comments with escaped newlines.

[Alexander Kornienko]

================
Comment at: lib/Format/Format.cpp:935
@@ +934,3 @@
+            1;
+        State.Column = Column;
+        return (Column > getColumnLimit())
----------------
Daniel Jasper wrote:
> Alexander Kornienko wrote:
> > Daniel Jasper wrote:
> > > I am confused by this. Shouldn't State.Column be set to the remaining length of the comment after the last escaped newline?
> > There's always a line break after the comment, and we don't adjust leading whitespace in consecutive lines, so we don't need to care about anything after the first escaped newline. 
> > 
> > Here I set State.Column to the last column of the first line to ensure proper penalty added if we violate the column limit in the first line. This is what is checked in FormatTest.cpp:938.
> Ok, but aren't you then adding this penalty twice? Once in line 936 below and once when coming out of this function?
Yep, you're right. Removed additional penalty.

================
Comment at: lib/Format/Format.cpp:937
@@ +936,3 @@
+        return (Column > getColumnLimit())
+                   ? Style.PenaltyExcessCharacter * (Column - getColumnLimit())
+                   : 0;
----------------
Daniel Jasper wrote:
> Alexander Kornienko wrote:
> > Daniel Jasper wrote:
> > > With this, you are adding the penalty for the first line (i.e. for the length of the comment up to the first escaped newline).
> > > 
> > > It seems to me that we should either add the penalty for each of the comment's lines (except the last one, which is added later) or for none of them. Otherwise, this is a half-way solution that is only correct for a few edge cases and might confuse people.
> > As I've noted in my previous comment, we don't touch lines after the first one, so we don't need to add penalty for them (it will be constant).
> It is constant until we decide we want to go ahead and shift all the comment's lines (possibly dependent on some other formatting - see your test case). However, I agree that this is not important now and should be constant. However, then I still don't get why you would need to add the penalty for the first line here.
For no reason. Removed.

================
Comment at: unittests/Format/FormatTest.cpp:938
@@ +937,3 @@
+                   getLLVMStyleWithColumns(50)));
+  EXPECT_EQ("int\n"
+            "a; // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\\\n"
----------------
Daniel Jasper wrote:
> Alexander Kornienko wrote:
> > Daniel Jasper wrote:
> > > Maybe add a FIXME as this is not generally desirable. I know that there is one in the code, but having that also in the test is also helpful. E.g. imagine you fix this some day (either directly or by accident doing another change). If you the find this comment on the breaking test, you know that you can just fix the test.
> > These two tests ensure that we calculate columns used by the comment's first line correctly. There's nothing to fix here.
> I know, what the test is meant to do. But in the long run, we'd probably want to fix the indent of the other lines of the comment, no?
Ah, you mean adjusting indentation of other lines. Yes, this deserves a FIXME.


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