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

[Alexander Kornienko]

  Correctly calculate the number of columns used by the comment (by its first line).

Hi djasper,

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

CHANGE SINCE LAST DIFF
  http://llvm-reviews.chandlerc.com/D1146?vs=2816&id=2817#toc

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

Index: lib/Format/Format.cpp
===================================================================
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -904,6 +904,10 @@
       // Only break up default narrow strings.
       if (!Current.TokenText.startswith("\""))
         return 0;
+      // Don't break string literals with escaped newlines. We're not likely to
+      // get them correctly, so better don't spoil them.
+      if (Current.TokenText.find("\\\n") != StringRef::npos)
+        return 0;
 
       Token.reset(new BreakableStringLiteral(Current, StartColumn,
                                              Line.InPPDirective, Encoding));
@@ -914,6 +918,25 @@
     } else if (Current.Type == TT_LineComment &&
                (Current.Previous == NULL ||
                 Current.Previous->Type != TT_ImplicitStringLiteral)) {
+      // Don't break line comments with escaped newlines. These look like
+      // separate line comments, but in fact contain consecutive lines including
+      // leading whitespace and '//' markers.
+      //
+      // TODO: If we want to handle them correctly, we'll need to adjust leading
+      // whitespace in consecutive lines when changing indentation of the first
+      // line similar to what we do with block comments.
+      StringRef::size_type EscapedNewlinePos = Current.TokenText.find("\\\n");
+      if (EscapedNewlinePos != StringRef::npos) {
+        unsigned Column =
+            StartColumn +
+            encoding::getCodePointCount(
+                Current.TokenText.substr(0, EscapedNewlinePos), Encoding) + 1;
+        State.Column = Column;
+        return (Column > getColumnLimit())
+                   ? Style.PenaltyExcessCharacter * (Column - getColumnLimit())
+                   : 0;
+      }
+
       Token.reset(new BreakableLineComment(Current, StartColumn,
                                            Line.InPPDirective, Encoding));
     } else {
Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -921,6 +921,30 @@
                    getLLVMStyleWithColumns(20)));
 }
 
+TEST_F(FormatTest, DontSplitLineCommentsWithEscapedNewlines) {
+  EXPECT_EQ("// aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\\n"
+            "// aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\\n"
+            "// aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+            format("// aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\\n"
+                   "// aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\\n"
+                   "// aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"));
+  EXPECT_EQ("int a; // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\\\n"
+            "       // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\\\n"
+            "       // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA",
+            format("int a; // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\\\n"
+                   "       // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\\\n"
+                   "       // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA",
+                   getLLVMStyleWithColumns(50)));
+  EXPECT_EQ("int\n"
+            "a; // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\\\n"
+            "       // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\\\n"
+            "       // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA",
+            format("int a; // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\\\n"
+                   "       // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\\\n"
+                   "       // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA",
+                   getLLVMStyleWithColumns(49)));
+}
+
 TEST_F(FormatTest, PriorityOfCommentBreaking) {
   EXPECT_EQ("if (xxx == yyy && // aaaaaaaaaaaa\n"
             "                  // bbbbbbbbb\n"
@@ -4991,6 +5015,16 @@
       format("#define A \"some text other\";", AlignLeft));
 }
 
+TEST_F(FormatTest, DontSplitStringLiteralsWithEscapedNewlines) {
+  EXPECT_EQ("aaaaaaaaaaa =\n"
+            "    \"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\\n"
+            "  aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\\n"
+            "  aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\";",
+            format("aaaaaaaaaaa = \"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\\n"
+                   "  aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\\n"
+                   "  aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\";"));
+}
+
 TEST_F(FormatTest, SkipsUnknownStringLiterals) {
   EXPECT_EQ(
       "u8\"unsupported literal\";",
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D1146.3.patch
Type: text/x-patch
Size: 4470 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130715/3308e446/attachment.bin>