[PATCH] Don't break string literals inside preprocessor directives.

[Alexander Kornienko]

  It turns out, that macro definitions reside in separate lines, so the current patch doesn't affect string literal breaking in macro definitions. I've added a comment regarding this and extended the test.

  I've also changed the formatting of gcc-style line directives to put a space between hashes and line numbers: # 111 "aaaaaaaaaaaaa" 2 3.

Hi djasper,

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

CHANGE SINCE LAST DIFF
  http://llvm-reviews.chandlerc.com/D1813?vs=4607&id=4729#toc

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

Index: lib/Format/ContinuationIndenter.cpp
===================================================================
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -689,10 +689,15 @@
                       tok::utf8_string_literal, tok::utf16_string_literal,
                       tok::utf32_string_literal) &&
       Current.Type != TT_ImplicitStringLiteral) {
+    // Don't break string literals inside preprocessor directives (except for
+    // #define directives, as their contents are stored in separate lines and
+    // are not affected by this check).
+    // This way we avoid breaking code with line directives and unknown
+    // preprocessor directives that contain long string literals.
+    if (State.Line->Type == LT_PreprocessorDirective) return 0;
     // Exempts unterminated string literals from line breaking. The user will
     // likely want to terminate the string before any line breaking is done.
-    if (Current.IsUnterminatedLiteral)
-      return 0;
+    if (Current.IsUnterminatedLiteral) return 0;
 
     StringRef Text = Current.TokenText;
     StringRef Prefix;
Index: lib/Format/TokenAnnotator.cpp
===================================================================
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -464,6 +464,10 @@
       return;
     // Hashes in the middle of a line can lead to any strange token
     // sequence.
+    if (CurrentToken->Tok.is(tok::numeric_constant)) {
+      CurrentToken->SpacesRequiredBefore = 1;
+      return;
+    }
     if (CurrentToken->Tok.getIdentifierInfo() == NULL)
       return;
     switch (CurrentToken->Tok.getIdentifierInfo()->getPPKeywordID()) {
@@ -1044,7 +1048,8 @@
       Current->SpacesRequiredBefore = Style.SpacesBeforeTrailingComments;
     else
       Current->SpacesRequiredBefore =
-          spaceRequiredBefore(Line, *Current) ? 1 : 0;
+          std::max(Current->SpacesRequiredBefore,
+                   spaceRequiredBefore(Line, *Current) ? 1U : 0);
 
     Current->MustBreakBefore =
         Current->MustBreakBefore || mustBreakBefore(Line, *Current);
Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1861,9 +1861,24 @@
       "                   OwningPtr<FileOutputBuffer> &buffer) = 0;");
 }
 
-TEST_F(FormatTest, LayoutUnknownPPDirective) {
-  EXPECT_EQ("#123 \"A string literal\"",
+TEST_F(FormatTest, BreaksStringLiteralsOnlyInDefine) {
+  verifyFormat("# 1111 \"/aaaaaaaaa/aaaaaaaaaaaaaaaaaaa/aaaaaaaa.cpp\" 2 3",
+               getLLVMStyleWithColumns(40));
+  verifyFormat("#line 11111 \"/aaaaaaaaa/aaaaaaaaaaaaaaaaaaa/aaaaaaaa.cpp\"",
+               getLLVMStyleWithColumns(40));
+  EXPECT_EQ("#define Q                              \\\n"
+            "  \"/aaaaaaaaa/aaaaaaaaaaaaaaaaaaa/\"    \\\n"
+            "  \"aaaaaaaa.cpp\"",
+            format("#define Q \"/aaaaaaaaa/aaaaaaaaaaaaaaaaaaa/aaaaaaaa.cpp\"",
+                   getLLVMStyleWithColumns(40)));
+}
+
+TEST_F(FormatTest, UnderstandsLinePPDirective) {
+  EXPECT_EQ("# 123 \"A string literal\"",
             format("   #     123    \"A string literal\""));
+}
+
+TEST_F(FormatTest, LayoutUnknownPPDirective) {
   EXPECT_EQ("#;", format("#;"));
   verifyFormat("#\n;\n;\n;");
 }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D1813.2.patch
Type: text/x-patch
Size: 3357 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131008/ebc45fbd/attachment.bin>