clang-format with UseTab: Always sometimes doesn't insert the right amount of tabs.

[hyklv]

Summary

Hi,

Trailing comments are not always aligned properly when UseTab is set to Always.

Consider:

int a;	      // x
int bbbbbbbb; // x

With .clang-format:

---
Language:        Cpp
BasedOnStyle: LLVM
UseTab: Always
...

The trailing comments of this code block should be aligned, but aren't

To align the first trailing comment it needs to insert 8 spaces. This should be one tab plus six spaces. It skips the logic of the first partial tab in FirstTabWidth (=2) + Style.TabWidth (=8) <= Spaces (=8) and only inserts one tab. Proposed fix and test is attached.

[lebedev.ri]

Test?
Is this a fix, or a new formatting style?

WhitespaceManager.cpp

3–6

That is incorrect.

[MyDeveloperDay]

The reviewers set a high barrier of entry, you need to at least start by adding some unit tests (or someone will break your code in the future), if you think it was a bug it might be worth logging that in bugzilla too!

[hyklv]

I removed accidental changes in the copyright notice, updated the code to insert one space instead of one tab. Added a test case for the alignment.

[hyklv]

In D57655#1382437, @lebedev.ri wrote:

Test?
Is this a fix, or a new formatting style?

a fix.

[hyklv]

In D57655#1382855, @MyDeveloperDay wrote:

The reviewers set a high barrier of entry, you need to at least start by adding some unit tests (or someone will break your code in the future), if you think it was a bug it might be worth logging that in bugzilla too!

thanks for pointing me in the right direction! i added a test.

[alexfh]

Seems reasonable. LG with a couple of nits. Please let me know if you need to commit this for you.

D:/Format/llvm/tools/clang/lib/Format/WhitespaceManager.cpp

685 ↗

(On Diff #186315)

nit: I'd just break here and remove the else.

686 ↗

(On Diff #186315)

nit: "Align to the next tab."

[hyklv]

Updated comment and added break;

[hyklv]

In D57655#1395924, @alexfh wrote:

Seems reasonable. LG with a couple of nits. Please let me know if you need to commit this for you.

cool. could you commit the change for me?

WhitespaceManager.cpp

 //===--- WhitespaceManager.cpp - Format C++ code --------------------------===//
 //
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
 /// This file implements WhitespaceManager class.
 ///
 //===----------------------------------------------------------------------===//
 
void WhitespaceManager::appendIndentText(std::string &Text,
                                          unsigned WhitespaceStartColumn) {
   switch (Style.UseTab) {
   case FormatStyle::UT_Never:
     Text.append(Spaces, ' ');
     break;
   case FormatStyle::UT_Always: {
     unsigned FirstTabWidth =
         Style.TabWidth - WhitespaceStartColumn % Style.TabWidth;
-    // Indent with tabs only when there's at least one full tab.
-    if (FirstTabWidth + Style.TabWidth <= Spaces) {
+    // Insert only spaces when we want to end up before the next tab.
+    if (Spaces < FirstTabWidth) {
+      Text.append(Spaces, ' ');
+    } else {
+      // Align to next tab.
       Spaces -= FirstTabWidth;
       Text.append("\t");
-    }
+
       Text.append(Spaces / Style.TabWidth, '\t');
       Text.append(Spaces % Style.TabWidth, ' ');
+    }
     break;
   }
   case FormatStyle::UT_ForIndentation:
     if (WhitespaceStartColumn == 0) {
       unsigned Indentation = IndentLevel * Style.IndentWidth;
       // This happens, e.g. when a line in a block comment is indented less than
       // the first one.
       if (Indentation > Spaces)