From 34d15151c4b6d24e0026f87adcc64449252f4d7e Mon Sep 17 00:00:00 2001
From: Manuel Klimek <klimek@google.com>
Date: Tue, 28 May 2013 10:01:59 +0000
Subject: [PATCH] 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.

llvm-svn: 182760
---
 clang/lib/Format/BreakableToken.cpp   | 18 ++++----
 clang/unittests/Format/FormatTest.cpp | 59 ++++++++++++++++-----------
 2 files changed, 44 insertions(+), 33 deletions(-)

diff --git a/clang/lib/Format/BreakableToken.cpp b/clang/lib/Format/BreakableToken.cpp
index 1fd538e25aeb..c102f8b1b295 100644
--- a/clang/lib/Format/BreakableToken.cpp
+++ b/clang/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/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index e05cd8090579..2af1dd75a536 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/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) {