[clang-format] Fix bug in reflow of block comments containing CR/LF

[owenpan]

Summary

This patch fixes PR36119.

[MyDeveloperDay]

This looks logical to me, seems to fit with what `WhitespaceManager::appendNewlineText` is doing

LGTM

[owenpan]

Removes a no longer needed hack that worked around this bug when computing StartOfLine in the BreakableBlockComment::adjustWhitespace function.

Also updates the test case.

[owenpan]

Removes a redundant test case.

[JVApen]

Any chance we can still get this in LLVM 11?

clang/lib/Format/BreakableToken.h

protected:
   StringRef ReflowPrefix = " ";
 };
 
 class BreakableBlockComment : public BreakableComment {
 public:
   BreakableBlockComment(const FormatToken &Token, unsigned StartColumn,
                         unsigned OriginalStartColumn, bool FirstInLine,
                         bool InPPDirective, encoding::Encoding Encoding,
-                        const FormatStyle &Style);
+                        const FormatStyle &Style, bool UseCRLF);
 
   unsigned getRangeLength(unsigned LineIndex, unsigned Offset,
                           StringRef::size_type Length,
                           unsigned StartColumn) const override;
   unsigned getRemainingLength(unsigned LineIndex, unsigned Offset,
                               unsigned StartColumn) const override;
   unsigned getContentStartColumn(unsigned LineIndex, bool Break) const override;
   unsigned getContentIndent(unsigned LineIndex) const override;

clang/lib/Format/BreakableToken.cpp

return Content.size() >= 2 && !hasSpecialMeaningPrefix &&
          // Note that this is UTF-8 safe, since if isPunctuation(Content[0]) is
          // true, then the first code point must be 1 byte long.
          (!isPunctuation(Content[0]) || !isPunctuation(Content[1]));
 }
 
 BreakableBlockComment::BreakableBlockComment(
     const FormatToken &Token, unsigned StartColumn,
     unsigned OriginalStartColumn, bool FirstInLine, bool InPPDirective,
-    encoding::Encoding Encoding, const FormatStyle &Style)
+    encoding::Encoding Encoding, const FormatStyle &Style, bool UseCRLF)
     : 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,
+                                                  UseCRLF ? "\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());
EndOfPreviousLine =
       Lines[LineIndex - 1].find_last_not_of(Blanks, EndOfPreviousLine);
   if (EndOfPreviousLine == StringRef::npos)
     EndOfPreviousLine = 0;
   else
     ++EndOfPreviousLine;
   // Calculate the start of the non-whitespace text in the current line.
   size_t StartOfLine = Lines[LineIndex].find_first_not_of(Blanks);
   if (StartOfLine == StringRef::npos)
-    StartOfLine = Lines[LineIndex].rtrim("\r\n").size();
+    StartOfLine = Lines[LineIndex].size();
 
   StringRef Whitespace = Lines[LineIndex].substr(0, StartOfLine);
   // Adjust Lines to only contain relevant text.
   size_t PreviousContentOffset =
       Content[LineIndex - 1].data() - Lines[LineIndex - 1].data();
   Content[LineIndex - 1] = Lines[LineIndex - 1].substr(
       PreviousContentOffset, EndOfPreviousLine - PreviousContentOffset);
   Content[LineIndex] = Lines[LineIndex].substr(StartOfLine);

clang/lib/Format/ContinuationIndenter.cpp

if (!Style.ReflowComments ||
         // If a comment token switches formatting, like
         // /* clang-format on */, we don't want to break it further,
         // but we may still want to adjust its indentation.
         switchesFormatting(Current)) {
       return nullptr;
     }
     return llvm::make_unique<BreakableBlockComment>(
         Current, StartColumn, Current.OriginalColumn, !Current.Previous,
-        State.Line->InPPDirective, Encoding, Style);
+        State.Line->InPPDirective, Encoding, Style, Whitespaces.useCRLF());
   } else if (Current.is(TT_LineComment) &&
              (Current.Previous == nullptr ||
               Current.Previous->isNot(TT_ImplicitStringLiteral))) {
     if (!Style.ReflowComments ||
         CommentPragmasRegex.match(Current.TokenText.substr(2)) ||
         switchesFormatting(Current))
       return nullptr;
     return llvm::make_unique<BreakableLineCommentSection>(

clang/lib/Format/WhitespaceManager.h

 ///
 /// There may be multiple calls to \c breakToken for a given token.
 class WhitespaceManager {
 public:
   WhitespaceManager(const SourceManager &SourceMgr, const FormatStyle &Style,
                     bool UseCRLF)
       : SourceMgr(SourceMgr), Style(Style), UseCRLF(UseCRLF) {}
 
+  bool useCRLF() const { return UseCRLF; }
+
   /// Replaces the whitespace in front of \p Tok. Only call once for
   /// each \c AnnotatedToken.
   ///
   /// \p StartOfTokenColumn is the column at which the token will start after
   /// this replacement. It is needed for determining how \p Spaces is turned
   /// into tabs and spaces for some format styles.
   void replaceWhitespace(FormatToken &Tok, unsigned Newlines, unsigned Spaces,
                          unsigned StartOfTokenColumn,

clang/unittests/Format/FormatTest.cpp

EXPECT_EQ("/*\r\n"
             "should not introduce\r\n"
             "an extra carriage return\r\n"
             "*/\r\n",
             format("/*\r\n"
                    "multi line block comments\r\n"
                    "should not introduce\r\n"
                    "an extra carriage return\r\n"
                    "*/\r\n"));
+  EXPECT_EQ("/*\r\n"
+            "\r\n"
+            "*/",
+            format("/*\r\n"
+                   " \r\n"
+                   "*/"));
+  EXPECT_EQ("/*\r\n"
+            "\r\n"
+            "*/",
+            format("/*\r\n"
+                   "    \r\r\r\n"
+                   "*/"));
 }
 
 TEST_F(FormatTest, MunchSemicolonAfterBlocks) {
   verifyFormat("MY_CLASS(C) {\n"
                "  int i;\n"
                "  int j;\n"
                "};");
 }