[clang-format] Fix bug in block comment reflow that joins * and /

[owenpan]

Summary

This patch fixes PR41213.

[MyDeveloperDay]

Thanks for the patch, is this case only limited to *<tab>

[owenpan]

@MyDeveloperDay In theory, any whitespace character other than a blank might trigger the bug, but in practice, we only need to handle the tab, IMO.

[owenpan]

There are some corner cases this patch doesn't cover.

[owenpan]

Updated the patch to cover all scenarios in which the bug might be triggered.

[MyDeveloperDay]

LGTM

[owenpan]

Updated the test cases to make them precise and more varied.

cfe/trunk/lib/Format/BreakableToken.h

 
 class BreakableBlockComment : public BreakableComment {
 public:
   BreakableBlockComment(const FormatToken &Token, unsigned StartColumn,
                         unsigned OriginalStartColumn, bool FirstInLine,
                         bool InPPDirective, encoding::Encoding Encoding,
                         const FormatStyle &Style, bool UseCRLF);
 
+  Split getSplit(unsigned LineIndex, unsigned TailOffset, unsigned ColumnLimit,
+                 unsigned ContentStartColumn,
+                 llvm::Regex &CommentPragmasRegex) const override;
   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;
   void insertBreak(unsigned LineIndex, unsigned TailOffset, Split Split,

cfe/trunk/lib/Format/BreakableToken.cpp

for (StringRef KnownPrefix : KnownPrefixes) {
     }
   }
   return LongestPrefix;
 }
 
 static BreakableToken::Split
 getCommentSplit(StringRef Text, unsigned ContentStartColumn,
                 unsigned ColumnLimit, unsigned TabWidth,
-                encoding::Encoding Encoding, const FormatStyle &Style) {
+                encoding::Encoding Encoding, const FormatStyle &Style,
+                bool DecorationEndsWithStar = false) {
   LLVM_DEBUG(llvm::dbgs() << "Comment split: \"" << Text
                           << "\", Column limit: " << ColumnLimit
                           << ", Content start: " << ContentStartColumn << "\n");
   if (ColumnLimit <= ContentStartColumn + 1)
     return BreakableToken::Split(StringRef::npos, 0);
 
   unsigned MaxSplit = ColumnLimit - ContentStartColumn + 1;
   unsigned MaxSplitBytes = 0;
getCommentSplit(StringRef Text, unsigned ContentStartColumn,
   if (SpaceOffset != StringRef::npos && SpaceOffset != 0) {
     // adaptStartOfLine will break after lines starting with /** if the comment
     // is broken anywhere. Avoid emitting this break twice here.
     // Example: in /** longtextcomesherethatbreaks */ (with ColumnLimit 20) will
     // insert a break after /**, so this code must not insert the same break.
     if (SpaceOffset == 1 && Text[SpaceOffset - 1] == '*')
       return BreakableToken::Split(StringRef::npos, 0);
     StringRef BeforeCut = Text.substr(0, SpaceOffset).rtrim(Blanks);
-    StringRef AfterCut = Text.substr(SpaceOffset).ltrim(Blanks);
+    StringRef AfterCut = Text.substr(SpaceOffset);
+    // Don't trim the leading blanks if it would create a */ after the break.
+    if (!DecorationEndsWithStar || AfterCut.size() <= 1 || AfterCut[1] != '/')
+      AfterCut = AfterCut.ltrim(Blanks);
     return BreakableToken::Split(BeforeCut.size(),
                                  AfterCut.begin() - BeforeCut.end());
   }
   return BreakableToken::Split(StringRef::npos, 0);
 }
 
 static BreakableToken::Split
 getStringSplit(StringRef Text, unsigned UsedColumns, unsigned ColumnLimit,
LLVM_DEBUG({
     for (size_t i = 0; i < Lines.size(); ++i) {
       llvm::dbgs() << i << " |" << Content[i] << "| "
                    << "CC=" << ContentColumn[i] << "| "
                    << "IN=" << (Content[i].data() - Lines[i].data()) << "\n";
     }
   });
 }
 
+BreakableToken::Split
+BreakableBlockComment::getSplit(unsigned LineIndex, unsigned TailOffset,
+                           unsigned ColumnLimit, unsigned ContentStartColumn,
+                           llvm::Regex &CommentPragmasRegex) const {
+  // Don't break lines matching the comment pragmas regex.
+  if (CommentPragmasRegex.match(Content[LineIndex]))
+    return Split(StringRef::npos, 0);
+  return getCommentSplit(Content[LineIndex].substr(TailOffset),
+                         ContentStartColumn, ColumnLimit, Style.TabWidth,
+                         Encoding, Style, Decoration.endswith("*"));
+}
+
 void BreakableBlockComment::adjustWhitespace(unsigned LineIndex,
                                              int IndentDelta) {
   // When in a preprocessor directive, the trailing backslash in a block comment
   // is not needed, but can serve a purpose of uniformity with necessary escaped
   // newlines outside the comment. In this case we remove it here before
   // trimming the trailing whitespace. The backslash will be re-added later when
   // inserting a line break.
   size_t EndOfPreviousLine = Lines[LineIndex - 1].size();

cfe/trunk/unittests/Format/FormatTest.cpp

TEST_F(FormatTest, UnderstandPragmaOption) {
 
   EXPECT_EQ("#pragma option -C -A", format("#pragma    option   -C   -A"));
 }
 
 TEST_F(FormatTest, OptimizeBreakPenaltyVsExcess) {
   FormatStyle Style = getLLVMStyle();
   Style.ColumnLimit = 20;
 
+  // See PR41213
+  EXPECT_EQ("/*\n"
+            " *\t9012345\n"
+            " * /8901\n"
+            " */",
+            format("/*\n"
+                   " *\t9012345 /8901\n"
+                   " */",
+                   Style));
+  EXPECT_EQ("/*\n"
+            " *345678\n"
+            " *\t/8901\n"
+            " */",
+            format("/*\n"
+                   " *345678\t/8901\n"
+                   " */",
+                   Style));
+
   verifyFormat("int a; // the\n"
                "       // comment", Style);
   EXPECT_EQ("int a; /* first line\n"
             "        * second\n"
             "        * line third\n"
             "        * line\n"
             "        */",
             format("int a; /* first line\n"