From: Manuel Klimek <klimek@google.com>
Date: Tue, 28 May 2013 10:01:59 +0000 (+0000)
Subject: Disable tab expansion when counting the columns in block comments.
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=d63312b1484a4642225c97e1dd013fe520b2b9a4;p=clang

Disable tab expansion when counting the columns in block comments.

To fully support this, we also need to expand tabs in the text before
the block comment. This patch breaks indentation when there was a
non-standard mixture of spaces and tabs used for indentation, but
fixes a regression in the simple case:
{
  /*
   * Comment.
   */
  int i;
}
Is now formatted correctly, if there were tabs used for indentation
before.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@182760 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/lib/Format/BreakableToken.cpp b/lib/Format/BreakableToken.cpp
index 1fd538e25a..c102f8b1b2 100644
--- a/lib/Format/BreakableToken.cpp
+++ b/lib/Format/BreakableToken.cpp
@@ -256,15 +256,6 @@ void BreakableBlockComment::adjustWhitespace(const FormatStyle &Style,
   size_t StartOfLine = Lines[LineIndex].find_first_not_of(" \t");
   if (StartOfLine == StringRef::npos)
     StartOfLine = Lines[LineIndex].size();
-  // FIXME: Tabs are not always 8 characters. Make configurable in the style.
-  unsigned Column = 0;
-  StringRef OriginalIndentText = Lines[LineIndex].substr(0, StartOfLine);
-  for (int i = 0, e = OriginalIndentText.size(); i != e; ++i) {
-    if (Lines[LineIndex][i] == '\t')
-      Column += 8 - (Column % 8);
-    else
-      ++Column;
-  }
 
   // Adjust Lines to only contain relevant text.
   Lines[LineIndex - 1] = Lines[LineIndex - 1].substr(0, EndOfPreviousLine);
@@ -273,8 +264,15 @@ void BreakableBlockComment::adjustWhitespace(const FormatStyle &Style,
   // to the current line.
   LeadingWhitespace[LineIndex] =
       Lines[LineIndex].begin() - Lines[LineIndex - 1].end();
+
+  // FIXME: We currently count tabs as 1 character. To solve this, we need to
+  // get the correct indentation width of the start of the comment, which
+  // requires correct counting of the tab expansions before the comment, and
+  // a configurable tab width. Since the current implementation only breaks
+  // if leading tabs are intermixed with spaces, that is not a high priority.
+
   // Adjust the start column uniformly accross all lines.
-  StartOfLineColumn[LineIndex] = std::max<int>(0, Column + IndentDelta);
+  StartOfLineColumn[LineIndex] = std::max<int>(0, StartOfLine + IndentDelta);
 }
 
 unsigned BreakableBlockComment::getLineCount() const { return Lines.size(); }
diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp
index e05cd80905..2af1dd75a5 100644
--- a/unittests/Format/FormatTest.cpp
+++ b/unittests/Format/FormatTest.cpp
@@ -4495,35 +4495,48 @@ TEST_F(FormatTest, ConfigurableUseOfTab) {
                "\t\t    parameter2); \\\n"
                "\t}",
                Tab);
-  EXPECT_EQ("/*\n"
-            "\t      a\t\tcomment\n"
-            "\t      in multiple lines\n"
-            "       */",
-            format("   /*\t \t \n"
-                   " \t \t a\t\tcomment\t \t\n"
-                   " \t \t in multiple lines\t\n"
-                   " \t  */",
-                   Tab));
-  Tab.UseTab = false;
-  // FIXME: Change this test to a different tab size than
-  // 8 once configurable.
-  EXPECT_EQ("/*\n"
-            "              a\t\tcomment\n"
-            "              in multiple lines\n"
-            "       */",
-            format("   /*\t \t \n"
-                   " \t \t a\t\tcomment\t \t\n"
-                   " \t \t in multiple lines\t\n"
-                   " \t  */",
-                   Tab));
-
-  // FIXME: This is broken, as the spelling column number we
-  // get from the SourceManager counts tab as '1'.
+
+
+  // FIXME: To correctly count mixed whitespace we need to
+  // also correctly count mixed whitespace in front of the comment.
+  //
+  // EXPECT_EQ("/*\n"
+  //           "\t      a\t\tcomment\n"
+  //           "\t      in multiple lines\n"
+  //           "       */",
+  //           format("   /*\t \t \n"
+  //                  " \t \t a\t\tcomment\t \t\n"
+  //                  " \t \t in multiple lines\t\n"
+  //                  " \t  */",
+  //                  Tab));
+  // Tab.UseTab = false;
+  // EXPECT_EQ("/*\n"
+  //           "              a\t\tcomment\n"
+  //           "              in multiple lines\n"
+  //           "       */",
+  //           format("   /*\t \t \n"
+  //                  " \t \t a\t\tcomment\t \t\n"
+  //                  " \t \t in multiple lines\t\n"
+  //                  " \t  */",
+  //                  Tab));
   // EXPECT_EQ("/* some\n"
   //           "   comment */",
   //          format(" \t \t /* some\n"
   //                 " \t \t    comment */",
   //                 Tab));
+
+  EXPECT_EQ("{\n"
+            "  /*\n"
+            "   * Comment\n"
+            "   */\n"
+            "  int i;\n"
+            "}",
+            format("{\n"
+                   "\t/*\n"
+                   "\t * Comment\n"
+                   "\t */\n"
+                   "\t int i;\n"
+                   "}"));
 }
 
 TEST_F(FormatTest, LinuxBraceBreaking) {