bkramer created this revision.
bkramer added a reviewer: djasper.
bkramer added a subscriber: cfe-commits.
Herald added a subscriber: klimek.

As soon as a comment had whitespace changes inside of the token, we
couldn't identify the whole comment as a trailing comment anymore and
alignment stopped working. Add a new boolean to Change for this special
case and fix trailing comment identification to use it.

Before this fix

int xy;  // a
int z;   //b

became

int xy;  // a
int z;  // b

with this patch we immediately get to:

int xy;  // a
int z;   // b

http://reviews.llvm.org/D16058

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

Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1022,6 +1022,10 @@
                    "  lineWith(); // comment\n"
                    "  // at start\n"
                    "}"));
+  EXPECT_EQ("int xy; // a\n"
+            "int z;  // b",
+            format("int xy;    // a\n"
+                   "int z;    //b"));
 
   verifyFormat("#define A                                                  \\\n"
                "  int i; /* iiiiiiiiiiiiiiiiiiiii */                       \\\n"
Index: lib/Format/WhitespaceManager.h
===================================================================
--- lib/Format/WhitespaceManager.h
+++ lib/Format/WhitespaceManager.h
@@ -109,7 +109,8 @@
            unsigned IndentLevel, int Spaces, unsigned StartOfTokenColumn,
            unsigned NewlinesBefore, StringRef PreviousLinePostfix,
            StringRef CurrentLinePrefix, tok::TokenKind Kind,
-           bool ContinuesPPDirective, bool IsStartOfDeclName);
+           bool ContinuesPPDirective, bool IsStartOfDeclName,
+           bool IsTrailingCommentContinuation);
 
     bool CreateReplacement;
     // Changes might be in the middle of a token, so we cannot just keep the
@@ -139,6 +140,10 @@
     // comments. Uncompensated negative offset is truncated to 0.
     int Spaces;
 
+    // If this change is inside of a line comment but not at the start of such a
+    // comment.
+    bool IsTrailingCommentContinuation;
+
     // \c IsTrailingComment, \c TokenLength, \c PreviousEndOfTokenColumn and
     // \c EscapedNewlineColumn will be calculated in
     // \c calculateLineBreakInformation.
Index: lib/Format/WhitespaceManager.cpp
===================================================================
--- lib/Format/WhitespaceManager.cpp
+++ lib/Format/WhitespaceManager.cpp
@@ -30,17 +30,19 @@
     unsigned IndentLevel, int Spaces, unsigned StartOfTokenColumn,
     unsigned NewlinesBefore, StringRef PreviousLinePostfix,
     StringRef CurrentLinePrefix, tok::TokenKind Kind, bool ContinuesPPDirective,
-    bool IsStartOfDeclName)
+    bool IsStartOfDeclName, bool IsTrailingCommentContinuation)
     : CreateReplacement(CreateReplacement),
       OriginalWhitespaceRange(OriginalWhitespaceRange),
       StartOfTokenColumn(StartOfTokenColumn), NewlinesBefore(NewlinesBefore),
       PreviousLinePostfix(PreviousLinePostfix),
       CurrentLinePrefix(CurrentLinePrefix), Kind(Kind),
       ContinuesPPDirective(ContinuesPPDirective),
       IsStartOfDeclName(IsStartOfDeclName), IndentLevel(IndentLevel),
-      Spaces(Spaces), IsTrailingComment(false), TokenLength(0),
-      PreviousEndOfTokenColumn(0), EscapedNewlineColumn(0),
-      StartOfBlockComment(nullptr), IndentationOffset(0) {}
+      Spaces(Spaces),
+      IsTrailingCommentContinuation(IsTrailingCommentContinuation),
+      IsTrailingComment(false), TokenLength(0), PreviousEndOfTokenColumn(0),
+      EscapedNewlineColumn(0), StartOfBlockComment(nullptr),
+      IndentationOffset(0) {}
 
 void WhitespaceManager::reset() {
   Changes.clear();
@@ -58,7 +60,8 @@
       Change(/*CreateReplacement=*/true, Tok.WhitespaceRange, IndentLevel,
              Spaces, StartOfTokenColumn, Newlines, "", "", Tok.Tok.getKind(),
              InPPDirective && !Tok.IsFirst,
-             Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName)));
+             Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName),
+             /*IsTrailingCommentContinuation=*/false));
 }
 
 void WhitespaceManager::addUntouchableToken(const FormatToken &Tok,
@@ -69,7 +72,8 @@
       /*CreateReplacement=*/false, Tok.WhitespaceRange, /*IndentLevel=*/0,
       /*Spaces=*/0, Tok.OriginalColumn, Tok.NewlinesBefore, "", "",
       Tok.Tok.getKind(), InPPDirective && !Tok.IsFirst,
-      Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName)));
+      Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName),
+      /*IsTrailingCommentContinuation=*/false));
 }
 
 void WhitespaceManager::replaceWhitespaceInToken(
@@ -82,15 +86,11 @@
   Changes.push_back(Change(
       true, SourceRange(Start, Start.getLocWithOffset(ReplaceChars)),
       IndentLevel, Spaces, std::max(0, Spaces), Newlines, PreviousPostfix,
-      CurrentPrefix,
-      // If we don't add a newline this change doesn't start a comment. Thus,
-      // when we align line comments, we don't need to treat this change as one.
-      // FIXME: We still need to take this change in account to properly
-      // calculate the new length of the comment and to calculate the changes
-      // for which to do the alignment when aligning comments.
-      Tok.is(TT_LineComment) && Newlines > 0 ? tok::comment : tok::unknown,
+      CurrentPrefix, Tok.is(TT_LineComment) ? tok::comment : tok::unknown,
       InPPDirective && !Tok.IsFirst,
-      Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName)));
+      Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName),
+      /*IsTrailingCommentContinuation=*/Tok.is(TT_LineComment) &&
+          Newlines == 0));
 }
 
 const tooling::Replacements &WhitespaceManager::generateReplacements() {
@@ -124,7 +124,8 @@
         Changes[i - 1].StartOfTokenColumn + Changes[i - 1].TokenLength;
 
     Changes[i - 1].IsTrailingComment =
-        (Changes[i].NewlinesBefore > 0 || Changes[i].Kind == tok::eof) &&
+        (Changes[i].NewlinesBefore > 0 || Changes[i].Kind == tok::eof ||
+         Changes[i].IsTrailingCommentContinuation) &&
         Changes[i - 1].Kind == tok::comment;
   }
   // FIXME: The last token is currently not always an eof token; in those
@@ -134,6 +135,10 @@
 
   const WhitespaceManager::Change *LastBlockComment = nullptr;
   for (auto &Change : Changes) {
+    // Reset the IsTrailingComment flag for changes inside of trailing comments
+    // so they don't get realigned later.
+    if (Change.IsTrailingCommentContinuation)
+      Change.IsTrailingComment = false;
     Change.StartOfBlockComment = nullptr;
     Change.IndentationOffset = 0;
     if (Change.Kind == tok::comment) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to