[clang-format] Separate block comments with CRLF correctly

[Uran198]

Summary

When formatting the following string:

"/*\r\n"
" * Comment with\r\n"
"    \r\n"
" * blanks.\r\n"
" */\r\n"

clang-format produced:

"/*\r\n"
" * Comment with\r\n"
"    \r\r\n"
" * blanks.\r\n"
" */\r\n"

And when formatting

"#define A(\\\r\n"
"    x) /* \\\r\n"
"a comment \\\r\n"
"inside */ \\\r\n"
"   f();"

with line length 17, clang-format produced:

"#define A(      \\\r"
"    x) /* \\     \\\r"
"a comment     \\ \\\r"
"inside */       \\\r"
"   f();"

So in one case it added additional \r instead of replacing with the blank
line and in another it added additional newline escape character \.

After the change the result are respectively:

"/*\r\n"
" * Comment with\r\n"
"\r\n"
" * blanks
.\r\
n"
" */\r\n"

and

"#define A(x) /* \\\r\n"
"  a comment     \\\r\n"
"  inside */     \\\r\n"
"  f();"

[alexfh]

lib/Format/BreakableToken.cpp

327

FYI, there's a global UseCRLF flag in WhitespaceManager. It may make sense to use it everywhere instead of deciding for each comment. But I'll let actual clang-format maintainers decide on pros and cons of this.

[krasimir]

lib/Format/BreakableToken.cpp

327

In case the text contains both \r\n-s and \n not preceded by \r, this would not break on the \n; neither will a version using WhitespaceManager::UsesCRLF. That is computed by:

bool inputUsesCRLF(StringRef Text) {
  return Text.count('\r') * 2 > Text.count('\n');
}

which implies that we at least tentatively support such mixed cases.

I'd try keeping the split here just by "\n" and stripping a trailing "\r"from each line as a second step.

lib/Format/BreakableToken.cpp

BreakableBlockComment::BreakableBlockComment(
     : BreakableComment(Token, StartColumn, InPPDirective, Encoding, Style),
       DelimitersOnNewline(false),
       UnbreakableTailLength(Token.UnbreakableTailLength) {
   assert(Tok.is(TT_BlockComment) &&
          "block comment section must start with a block comment");
 
   StringRef TokenText(Tok.TokenText);
   assert(TokenText.startswith("/*") && TokenText.endswith("*/"));
-  TokenText.substr(2, TokenText.size() - 4).split(Lines, "\n");
+  TokenText.substr(2, TokenText.size() - 4)
+      .split(Lines, TokenText.count('\r') > 0 ? "\r\n" : "\n");
 
   int IndentDelta = StartColumn - OriginalStartColumn;
   Content.resize(Lines.size());
   Content[0] = Lines[0];
   ContentColumn.resize(Lines.size());
   // Account for the initial '/*'.
   ContentColumn[0] = StartColumn + 2;
   Tokens.resize(Lines.size());

unittests/Format/FormatTestComments.cpp

verifyFormat("aaaaaaaa(/* parameter 1 */ aaaaaa,\n"
                NoBinPacking);
 
   // Aligning block comments in macros.
   verifyGoogleFormat("#define A        \\\n"
                      "  int i;   /*a*/ \\\n"
                      "  int jjj; /*b*/");
 }
 
+TEST_F(FormatTestComments, BlockCommentsWithCLRF) {
+  EXPECT_EQ("/*\r\n"
+            " * Comment with\r\n"
+            "\r\n"
+            " * blanks.\r\n"
+            " */\r\n"
+            "void f() {}",
+            format("/* \r\n"
+                    " * Comment with\r\n"
+                    "     \r\n"
+                    " * blanks.\r\n"
+                    " */\r\n"
+                    "void f() {}"));
+  EXPECT_EQ("#define A(x) /* \\\r\n"
+            "  a comment     \\\r\n"
+            "  inside */     \\\r\n"
+            "  f();",
+            format("#define A(x) /* \\\r\n"
+                   "  a comment     \\\r\n"
+                   "  inside */     \\\r\n"
+                   "  f();",
+                   getLLVMStyleWithColumns(17)));
+}
+
 TEST_F(FormatTestComments, AlignsBlockComments) {
   EXPECT_EQ("/*\n"
             " * Really multi-line\n"
             " * comment.\n"
             " */\n"
             "void f() {}",
             format("  /*\n"
                    "   * Really multi-line\n"