[clang-format] Fix SpacesInLineCommentPrefix deleting tokens.

[curdeius]

Summary

Fixes https://github.com/llvm/llvm-project/issues/53799.

[curdeius]

I thought about using signed ints for Minimum and Maximum in the style, but then, we won't be able to use -1 for Maximum the same way as now and signed int makes no sense for Minimum...

clang/lib/Format/BreakableToken.cpp

799–800

Not strictly necessary, but without the cast we have a signed/unsigned mismatch warning on comparison.

[HazardyKnusperkeks]

Never would have thought about that.

clang/lib/Format/BreakableToken.cpp

 
 BreakableLineCommentSection::BreakableLineCommentSection(
     const FormatToken &Token, unsigned StartColumn, bool InPPDirective,
     encoding::Encoding Encoding, const FormatStyle &Style)
     : BreakableComment(Token, StartColumn, InPPDirective, Encoding, Style) {
   assert(Tok.is(TT_LineComment) &&
          "line comment section must start with a line comment");
   FormatToken *LineTok = nullptr;
+  const int Minimum = Style.SpacesInLineCommentPrefix.Minimum;
   // How many spaces we changed in the first line of the section, this will be
   // applied in all following lines
   int FirstLineSpaceChange = 0;
   for (const FormatToken *CurrentTok = &Tok;
        CurrentTok && CurrentTok->is(TT_LineComment);
        CurrentTok = CurrentTok->Next) {
     LastLineTok = LineTok;
     StringRef TokenText(CurrentTok->TokenText);
     assert((TokenText.startswith("//") || TokenText.startswith("#")) &&
            "unsupported line comment prefix, '//' and '#' are supported");
     size_t FirstLineIndex = Lines.size();
     TokenText.split(Lines, "\n");
     Content.resize(Lines.size());
     ContentColumn.resize(Lines.size());
     PrefixSpaceChange.resize(Lines.size());
     Tokens.resize(Lines.size());
     Prefix.resize(Lines.size());
     OriginalPrefix.resize(Lines.size());
     for (size_t i = FirstLineIndex, e = Lines.size(); i < e; ++i) {
       Lines[i] = Lines[i].ltrim(Blanks);
       StringRef IndentPrefix = getLineCommentIndentPrefix(Lines[i], Style);
       OriginalPrefix[i] = IndentPrefix;
-      const unsigned SpacesInPrefix = llvm::count(IndentPrefix, ' ');
+      const int SpacesInPrefix = llvm::count(IndentPrefix, ' ');
 
       // This lambda also considers multibyte character that is not handled in
       // functions like isPunctuation provided by CharInfo.
       const auto NoSpaceBeforeFirstCommentChar = [&]() {
         assert(Lines[i].size() > IndentPrefix.size());
         const char FirstCommentChar = Lines[i][IndentPrefix.size()];
         const unsigned FirstCharByteSize =
             encoding::getCodePointNumBytes(FirstCommentChar, Encoding);
         return encoding::columnWidth(
                    Lines[i].substr(IndentPrefix.size(), FirstCharByteSize),
                    Encoding) == 1 &&
                (FirstCommentChar == '\\' || isPunctuation(FirstCommentChar) ||
                 isHorizontalWhitespace(FirstCommentChar));
       };
 
       // On the first line of the comment section we calculate how many spaces
       // are to be added or removed, all lines after that just get only the
       // change and we will not look at the maximum anymore. Additionally to the
       // actual first line, we calculate that when the non space Prefix changes,
       // e.g. from "///" to "//".
       if (i == 0 || OriginalPrefix[i].rtrim(Blanks) !=
                         OriginalPrefix[i - 1].rtrim(Blanks)) {
-        if (SpacesInPrefix < Style.SpacesInLineCommentPrefix.Minimum &&
-            Lines[i].size() > IndentPrefix.size() &&
+        if (SpacesInPrefix < Minimum && Lines[i].size() > IndentPrefix.size() &&
             !NoSpaceBeforeFirstCommentChar()) {
-          FirstLineSpaceChange =
-              Style.SpacesInLineCommentPrefix.Minimum - SpacesInPrefix;
-        } else if (SpacesInPrefix > Style.SpacesInLineCommentPrefix.Maximum) {
+          FirstLineSpaceChange = Minimum - SpacesInPrefix;
+        } else if (static_cast<unsigned>(SpacesInPrefix) >
+                   Style.SpacesInLineCommentPrefix.Maximum) {
           FirstLineSpaceChange =
               Style.SpacesInLineCommentPrefix.Maximum - SpacesInPrefix;
         } else {
           FirstLineSpaceChange = 0;
         }
       }
 
       if (Lines[i].size() != IndentPrefix.size()) {
         PrefixSpaceChange[i] = FirstLineSpaceChange;
 
-        if (SpacesInPrefix + PrefixSpaceChange[i] <
-            Style.SpacesInLineCommentPrefix.Minimum) {
-          PrefixSpaceChange[i] += Style.SpacesInLineCommentPrefix.Minimum -
-                                  (SpacesInPrefix + PrefixSpaceChange[i]);
+        if (SpacesInPrefix + PrefixSpaceChange[i] < Minimum) {
+          PrefixSpaceChange[i] +=
+              Minimum - (SpacesInPrefix + PrefixSpaceChange[i]);
         }
 
         assert(Lines[i].size() > IndentPrefix.size());
         const auto FirstNonSpace = Lines[i][IndentPrefix.size()];
         const auto AllowsSpaceChange =
             SpacesInPrefix != 0 ||
             (!NoSpaceBeforeFirstCommentChar() ||
              (FirstNonSpace == '}' && FirstLineSpaceChange != 0));

clang/unittests/Format/FormatTestComments.cpp

EXPECT_EQ("// Lorem ipsum dolor\n"
             "\n"
             "// Lorem   ipsum\n"
             "// dolor   sit amet\n"
             "\n"
             "void f() { // Hello\n"
             "           // World\n"
             "}",
             format(WrapCode, Style));
+  EXPECT_EQ("// x\n"
+            "// y",
+            format("//   x\n"
+                   "// y",
+                   Style));
 
   Style.SpacesInLineCommentPrefix = {3, 3};
   EXPECT_EQ("//   Lorem ipsum\n"
             "//   dolor sit amet\n"
             "\n"
             "//   Lorem   ipsum\n"
             "//   dolor   sit\n"
             "//   amet\n"