[PATCH] D48259: [clang-format] Fix bug with UT_Always when there is less than one full tab

[Guillaume Reymond via Phabricator via cfe-commits]

guiguiiiiiiii created this revision.
guiguiiiiiiii added reviewers: klimek, djasper.
guiguiiiiiiii added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

There's a bug in the `WhitespaceManager` that prevents correct alignment when using tab.
This patch fix a test condition that was triggering the bug and add missing unit tests that would have failed.

The issue can be seen by invoking clang-format on this simple snippet with `-style="{BasedOnStyle: llvm, UseTab: Always, AlignConsecutiveAssignments: true, AlignConsecutiveDeclarations: true, TabWidth: 4}"`

  int a = 42;
  int aa = 42;
  int aaa = 42;
  int aaaa = 42;

Displaying the result in a editor with a tabsize of 4, the first = won't be aligned correctly.

The logic in `WhitespaceManager::appendIndentText` treats the first tab as a special case, which is what must be done in order to handle tabstop correctly.
However, handling the first tab is done conditionally using the first tab width.
The proposed fix is to handle the first tab independently of its size as the code that follow this test...

  Text.append(Spaces / Style.TabWidth, '\t');

...will only work if the `'\t'` we're adding are positionned on a tabstop (so `'\t'` is equal to **TabWidth** in the output, otherwise it will be between 1 and **TabWidth** - 1)


Repository:
  rC Clang

https://reviews.llvm.org/D48259

Files:
  lib/Format/WhitespaceManager.cpp
  unittests/Format/FormatTest.cpp


Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -9367,6 +9367,18 @@
                    "int oneTwoThree = 123;\n"
                    "int oneTwo = 12;",
                    Alignment));
+  // Test with use of tabs for alignment
+  FormatStyle::UseTabStyle OldTabStyle = Alignment.UseTab;
+  unsigned OldTabWidth = Alignment.TabWidth;
+  Alignment.UseTab = FormatStyle::UT_Always;
+  Alignment.TabWidth = 4;
+  verifyFormat("int a\t = 42;\n"
+               "int aa   = 42;\n"
+               "int aaa  = 42;\n"
+               "int aaaa = 42;",
+               Alignment);
+  Alignment.UseTab = OldTabStyle;
+  Alignment.TabWidth = OldTabWidth;
   Alignment.AlignEscapedNewlines = FormatStyle::ENAS_DontAlign;
   verifyFormat("#define A \\\n"
                "  int aaaa       = 12; \\\n"
@@ -9542,6 +9554,18 @@
                    "unsigned oneTwoThree = 123;\n"
                    "int oneTwo = 12;",
                    Alignment));
+  // Test with use of tabs for alignment
+  FormatStyle::UseTabStyle OldTabStyle = Alignment.UseTab;
+  unsigned OldTabWidth = Alignment.TabWidth;
+  Alignment.UseTab = FormatStyle::UT_Always;
+  Alignment.TabWidth = 4;
+  verifyFormat("int\t   a = 42;\n"
+               "float  b = 42;\n"
+               "char   c = 42;\n"
+               "double d = 42;",
+               Alignment);
+  Alignment.UseTab = OldTabStyle;
+  Alignment.TabWidth = OldTabWidth;
   // Function prototype alignment
   verifyFormat("int    a();\n"
                "double b();",
Index: lib/Format/WhitespaceManager.cpp
===================================================================
--- lib/Format/WhitespaceManager.cpp
+++ lib/Format/WhitespaceManager.cpp
@@ -675,7 +675,7 @@
     unsigned FirstTabWidth =
         Style.TabWidth - WhitespaceStartColumn % Style.TabWidth;
     // Indent with tabs only when there's at least one full tab.
-    if (FirstTabWidth + Style.TabWidth <= Spaces) {
+    if (Style.TabWidth <= Spaces) {
       Spaces -= FirstTabWidth;
       Text.append("\t");
     }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D48259.151629.patch
Type: text/x-patch
Size: 2151 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180617/58ecde60/attachment.bin>