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

[Hylke Kleve via Phabricator via cfe-commits]

hyklv created this revision.
hyklv added reviewers: alexfh, djasper.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Hi,

I'm trying to get clang-format suited for our code layout. I've set it up to align declarations and trailing comments. Tab size is 4; UseTab is set to Always. Configuration file is pasted below. I see that it misaligns the trailing comment for a file with this code:

  struct NavOffMeshGraphEdge
  {
  	pcRTTIRefObject mObject;					  ///< The object NPC's will end up using to traverse this edge.
  	pNavMeshObject  mNavMeshObject{ nullptr };	  ///< The navmesh object assigning costs for path finding over this edge.
  };

To align the second trailing comment it needs to insert 4 spaces. This should be one tab plus two spaces. It skips the logic of the first partial tab in `FirstTabWidth (=2) + Style.TabWidth (=4) <= Spaces (=4)` while it shouldn't. Proposed fix is attached.

Our .clang-format is setup with:

  ---
  Language:        Cpp
  AccessModifierOffset: -4
  AlignAfterOpenBracket: Align
  AlignConsecutiveAssignments: true
  AlignConsecutiveDeclarations: true
  AlignEscapedNewlines: Right
  AlignOperands:   true
  AlignTrailingComments: true
  AllowAllParametersOfDeclarationOnNextLine: false
  AllowShortBlocksOnASingleLine: true
  AllowShortCaseLabelsOnASingleLine: true
  AllowShortFunctionsOnASingleLine: Inline
  AllowShortIfStatementsOnASingleLine: false
  AllowShortLoopsOnASingleLine: false
  AlwaysBreakAfterDefinitionReturnType: None
  AlwaysBreakAfterReturnType: None
  AlwaysBreakBeforeMultilineStrings: false
  AlwaysBreakTemplateDeclarations: true
  BinPackArguments: true
  BinPackParameters: true
  BraceWrapping:   
    AfterClass:      true
    AfterControlStatement: true
    AfterEnum:       true
    AfterFunction:   true
    AfterNamespace:  true
    AfterObjCDeclaration: false
    AfterStruct:     true
    AfterUnion:      true
    BeforeCatch:     true
    BeforeElse:      true
    IndentBraces:    false
    SplitEmptyFunction: true
    SplitEmptyRecord: true
    SplitEmptyNamespace: true
  BreakBeforeBinaryOperators: None
  BreakBeforeBraces: Custom
  BreakBeforeInheritanceComma: false
  BreakBeforeTernaryOperators: false
  BreakConstructorInitializersBeforeComma: false
  BreakConstructorInitializers: AfterColon
  BreakAfterJavaFieldAnnotations: false
  BreakStringLiterals: false
  ColumnLimit:     5000
  CommentPragmas:  '^ IWYU pragma:'
  CompactNamespaces: false
  ConstructorInitializerAllOnOneLineOrOnePerLine: true
  ConstructorInitializerIndentWidth: 4
  ContinuationIndentWidth: 4
  Cpp11BracedListStyle: false
  DerivePointerAlignment: false
  DisableFormat:   false
  ExperimentalAutoDetectBinPacking: false
  FixNamespaceComments: false
  ForEachMacros:   
    - foreach
    - Q_FOREACH
    - BOOST_FOREACH
  IncludeCategories: 
    - Regex:           '^".*\.h"'
      Priority:        1
    - Regex:           '^<ext/.*\.h>'
      Priority:        2
    - Regex:           '^<.*\.h>'
      Priority:        1
    - Regex:           '^<.*'
      Priority:        2
    - Regex:           '.*'
      Priority:        3
  IncludeIsMainRegex: '([-_](test|unittest))?$'
  IndentCaseLabels: false
  IndentWidth:     4
  IndentWrappedFunctionNames: true
  IndentPPDirectives: None
  JavaScriptQuotes: Leave
  JavaScriptWrapImports: true
  KeepEmptyLinesAtTheStartOfBlocks: false
  MacroBlockBegin: "^RTTI_START_"
  MacroBlockEnd: "^\
  RTTI_END"
  MaxEmptyLinesToKeep: 3
  NamespaceIndentation: None
  ObjCBlockIndentWidth: 2
  ObjCSpaceAfterProperty: false
  ObjCSpaceBeforeProtocolList: false
  PenaltyBreakAssignment: 2
  PenaltyBreakBeforeFirstCallParameter: 1
  PenaltyBreakComment: 300
  PenaltyBreakFirstLessLess: 120
  PenaltyBreakString: 1000
  PenaltyExcessCharacter: 1000000
  PenaltyReturnTypeOnItsOwnLine: 200
  PointerAlignment: Left
  ReflowComments:  true
  SortIncludes:    true
  SortUsingDeclarations: true
  SpaceAfterCStyleCast: true
  SpaceAfterTemplateKeyword: true
  SpaceBeforeAssignmentOperators: true
  SpaceBeforeParens: ControlStatements
  SpaceInEmptyParentheses: false
  SpacesBeforeTrailingComments: 4
  SpacesInAngles:  false
  SpacesInContainerLiterals: false
  SpacesInCStyleCastParentheses: false
  SpacesInParentheses: false
  SpacesInSquareBrackets: false
  Standard:        Auto
  TabWidth:        4
  UseTab:          Always
  ...




Repository:
  rC Clang

https://reviews.llvm.org/D57655

Files:
  WhitespaceManager.cpp


Index: WhitespaceManager.cpp
===================================================================
--- WhitespaceManager.cpp
+++ WhitespaceManager.cpp
@@ -1,9 +1,10 @@
 //===--- 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
@@ -679,13 +680,17 @@
   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, ' ');
     }
-    Text.append(Spaces / Style.TabWidth, '\t');
-    Text.append(Spaces % Style.TabWidth, ' ');
     break;
   }
   case FormatStyle::UT_ForIndentation:


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D57655.184956.patch
Type: text/x-patch
Size: 1510 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190203/8967b261/attachment.bin>