[clang-format] Correctly count a tab's width in a comment

[HazardyKnusperkeks]

Summary

It worked only correct for a tab as the first char.

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

[rymiel]

The behaviour of tabs still doesn't match that of spaces, but at least this makes it idempotent.

This also fixes https://github.com/llvm/llvm-project/issues/51808, I'm closing it as duplicate in favor of the issue fixed by this patch.

[HazardyKnusperkeks]

In D138402#3940981, @rymiel wrote:

The behaviour of tabs still doesn't match that of spaces, but at least this makes it idempotent.

I did initially looked into that, but then asked do we want to treat tabs equally to spaces?
It would add a lot of logic, and the question how to map SpacesInLineCommentPrefix to tabs?

This also fixes https://github.com/llvm/llvm-project/issues/51808, I'm closing it as duplicate in favor of the issue fixed by this patch.

Thanks.

[owenpan]

clang/unittests/Format/FormatTestComments.cpp

736

error: reference to type 'const clang::format::FormatStyle' could not bind to an lvalue of type 'const char[61]'
      "//\t\t\t\tofMap(message.velocity, 0, 127, 0, ofGetWidth() * 0.2)");
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

[HazardyKnusperkeks]

clang/unittests/Format/FormatTestComments.cpp

736

Just needs the verifyFormat from D138263.

[owenpan]

clang/unittests/Format/FormatTestComments.cpp

736

I see. Do you want to stack this patch onto D138263 (which I might not be able to get to for a while) or copy the needed code from there to here?

[owenpan]

clang/unittests/Format/FormatTestComments.cpp

736

Nvm. It's stacked now.

[HazardyKnusperkeks]

Pulled the verifyFormat into this change.

[owenpan]

Thanks for unstacking it!

clang/unittests/Format/FormatTestComments.cpp

66–67

Similar to what's done in FormatTest.cpp.

clang/lib/Format/BreakableToken.cpp

getCommentSplit(StringRef Text, unsigned ContentStartColumn,
 
   unsigned MaxSplit = ColumnLimit - ContentStartColumn + 1;
   unsigned MaxSplitBytes = 0;
 
   for (unsigned NumChars = 0;
        NumChars < MaxSplit && MaxSplitBytes < Text.size();) {
     unsigned BytesInChar =
         encoding::getCodePointNumBytes(Text[MaxSplitBytes], Encoding);
-    NumChars +=
-        encoding::columnWidthWithTabs(Text.substr(MaxSplitBytes, BytesInChar),
-                                      ContentStartColumn, TabWidth, Encoding);
+    NumChars += encoding::columnWidthWithTabs(
+        Text.substr(MaxSplitBytes, BytesInChar), ContentStartColumn + NumChars,
+        TabWidth, Encoding);
     MaxSplitBytes += BytesInChar;
   }
 
   // In JavaScript, some @tags can be followed by {, and machinery that parses
   // these comments will fail to understand the comment if followed by a line
   // break. So avoid ever breaking before a {.
   if (Style.isJavaScript()) {
     StringRef::size_type SpaceOffset =

clang/unittests/Format/FormatTestComments.cpp

protected:
   FormatStyle getTextProtoStyleWithColumns(unsigned ColumnLimit) {
     FormatStyle Style = getGoogleStyle(FormatStyle::FormatStyle::LK_TextProto);
     Style.ColumnLimit = ColumnLimit;
     return Style;
   }
 
   void verifyFormat(llvm::StringRef Code,
                     const FormatStyle &Style = getLLVMStyle()) {
     EXPECT_EQ(Code.str(), format(Code, Style)) << "Expected code is not stable";
     EXPECT_EQ(Code.str(), format(test::messUp(Code), Style));
   }
 
   void verifyFormat(llvm::StringRef Expected, llvm::StringRef Code,
                     const FormatStyle &Style = getLLVMStyle()) {
     EXPECT_EQ(Expected.str(), format(Expected, Style))
         << "Expected code is not stable";
     EXPECT_EQ(Expected.str(), format(test::messUp(Code), Style));
   }
EXPECT_EQ("{\n"
                    "}",
                    getLLVMStyleWithColumns(20)));
 
   EXPECT_EQ("//: A comment that\n"
             "//: doesn't fit on\n"
             "//: one line",
             format("//: A comment that doesn't fit on one line",
                    getLLVMStyleWithColumns(20)));
+
+  verifyFormat(
+      "//\t\t\t\tofMap(message.velocity, 0, 127, 0, ofGetWidth()\n"
+      "//* 0.2)",
+      "//\t\t\t\tofMap(message.velocity, 0, 127, 0, ofGetWidth() * 0.2)");
 }
 
 TEST_F(FormatTestComments, PreservesHangingIndentInCxxComments) {
   EXPECT_EQ("//     A comment\n"
             "//     that doesn't\n"
             "//     fit on one\n"
             "//     line",
             format("//     A comment that doesn't fit on one line",