Use a SourceRange for the whitespace location in FormatToken.

Replaces the use of WhitespaceStart + WhitspaceLength.
This made a bug in the formatter obvous where we would incorrectly
calculate the next column.

FIXME: There's a similar bug left regarding TokenLength. We should
probably also move to have a TokenRange instead.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@182572 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Format/Format.cpp b/lib/Format/Format.cpp
index e19583376a5e03019d1755e4c8ada8ea33930727..1cda3c711cac0c0c188916177353e7e8ec1b10c2 100644 (file)
--- a/lib/Format/Format.cpp
+++ b/lib/Format/Format.cpp
@@ -470,8 +470,13 @@ private:
     const AnnotatedToken &Previous = *State.NextToken->Parent;
 
     if (State.Stack.size() == 0 || Current.Type == TT_ImplicitStringLiteral) {
-      State.Column += State.NextToken->FormatTok.WhiteSpaceLength +
-                      State.NextToken->FormatTok.TokenLength;
+      // FIXME: Is this correct?
+      int WhitespaceLength =
+          SourceMgr.getSpellingColumnNumber(
+              State.NextToken->FormatTok.WhitespaceRange.getEnd()) -
+          SourceMgr.getSpellingColumnNumber(
+              State.NextToken->FormatTok.WhitespaceRange.getBegin());
+      State.Column += WhitespaceLength + State.NextToken->FormatTok.TokenLength;
       if (State.NextToken->Children.empty())
         State.NextToken = NULL;
       else
@@ -1078,9 +1083,9 @@ public:
   virtual FormatToken getNextToken() {
     if (GreaterStashed) {
       FormatTok.NewlinesBefore = 0;
-      FormatTok.WhiteSpaceStart =
+      SourceLocation GreaterLocation =
           FormatTok.Tok.getLocation().getLocWithOffset(1);
-      FormatTok.WhiteSpaceLength = 0;
+      FormatTok.WhitespaceRange = SourceRange(GreaterLocation, GreaterLocation);
       GreaterStashed = false;
       return FormatTok;
     }
@@ -1088,23 +1093,27 @@ public:
     FormatTok = FormatToken();
     Lex.LexFromRawLexer(FormatTok.Tok);
     StringRef Text = rawTokenText(FormatTok.Tok);
-    FormatTok.WhiteSpaceStart = FormatTok.Tok.getLocation();
-    if (SourceMgr.getFileOffset(FormatTok.WhiteSpaceStart) == 0)
+    SourceLocation WhitespaceStart = FormatTok.Tok.getLocation();
+    if (SourceMgr.getFileOffset(WhitespaceStart) == 0)
       FormatTok.IsFirst = true;
 
     // Consume and record whitespace until we find a significant token.
+    unsigned WhitespaceLength = 0;
     while (FormatTok.Tok.is(tok::unknown)) {
       unsigned Newlines = Text.count('\n');
       if (Newlines > 0)
-        FormatTok.LastNewlineOffset =
-            FormatTok.WhiteSpaceLength + Text.rfind('\n') + 1;
+        FormatTok.LastNewlineOffset = WhitespaceLength + Text.rfind('\n') + 1;
       unsigned EscapedNewlines = Text.count("\\\n");
       FormatTok.NewlinesBefore += Newlines;
       FormatTok.HasUnescapedNewline |= EscapedNewlines != Newlines;
-      FormatTok.WhiteSpaceLength += FormatTok.Tok.getLength();
+      WhitespaceLength += FormatTok.Tok.getLength();
 
-      if (FormatTok.Tok.is(tok::eof))
+      if (FormatTok.Tok.is(tok::eof)) {
+        FormatTok.WhitespaceRange =
+            SourceRange(WhitespaceStart,
+                        WhitespaceStart.getLocWithOffset(WhitespaceLength));
         return FormatTok;
+      }
       Lex.LexFromRawLexer(FormatTok.Tok);
       Text = rawTokenText(FormatTok.Tok);
     }
@@ -1126,7 +1135,7 @@ public:
     unsigned i = 0;
     while (i + 1 < Text.size() && Text[i] == '\\' && Text[i + 1] == '\n') {
       // FIXME: ++FormatTok.NewlinesBefore is missing...
-      FormatTok.WhiteSpaceLength += 2;
+      WhitespaceLength += 2;
       FormatTok.TokenLength -= 2;
       i += 2;
     }
@@ -1143,6 +1152,8 @@ public:
       GreaterStashed = true;
     }
 
+    FormatTok.WhitespaceRange = SourceRange(
+        WhitespaceStart, WhitespaceStart.getLocWithOffset(WhitespaceLength));
     return FormatTok;
   }
 
@@ -1235,7 +1246,7 @@ public:
       } else if (TheLine.Type != LT_Invalid &&
                  (WasMoved || FormatPPDirective || touchesLine(TheLine))) {
         unsigned LevelIndent = getIndent(IndentForLevel, TheLine.Level);
-        if (FirstTok.WhiteSpaceStart.isValid() &&
+        if (FirstTok.WhitespaceRange.isValid() &&
             // Insert a break even if there is a structural error in case where
             // we break apart a line consisting of multiple unwrapped lines.
             (FirstTok.NewlinesBefore == 0 || !StructuralError)) {
@@ -1300,8 +1311,11 @@ private:
       AnnotatedToken *Tok = &AnnotatedLines[i].First.Children[0];
       while (!Tok->Children.empty()) {
         if (Tok->Type == TT_PointerOrReference) {
-          bool SpacesBefore = Tok->FormatTok.WhiteSpaceLength > 0;
-          bool SpacesAfter = Tok->Children[0].FormatTok.WhiteSpaceLength > 0;
+          bool SpacesBefore = Tok->FormatTok.WhitespaceRange.getBegin() !=
+                              Tok->FormatTok.WhitespaceRange.getEnd();
+          bool SpacesAfter =
+              Tok->Children[0].FormatTok.WhitespaceRange.getBegin() !=
+              Tok->Children[0].FormatTok.WhitespaceRange.getEnd();
           if (SpacesBefore && !SpacesAfter)
             ++CountBoundToVariable;
           else if (!SpacesBefore && SpacesAfter)
@@ -1310,7 +1324,8 @@ private:
 
         if (Tok->Type == TT_TemplateCloser &&
             Tok->Parent->Type == TT_TemplateCloser &&
-            Tok->FormatTok.WhiteSpaceLength == 0)
+            Tok->FormatTok.WhitespaceRange.getBegin() ==
+                Tok->FormatTok.WhitespaceRange.getEnd())
           HasCpp03IncompatibleFormat = true;
         Tok = &Tok->Children[0];
       }
@@ -1509,7 +1524,8 @@ private:
     const FormatToken *First = &TheLine.First.FormatTok;
     const FormatToken *Last = &TheLine.Last->FormatTok;
     CharSourceRange LineRange = CharSourceRange::getCharRange(
-        First->WhiteSpaceStart.getLocWithOffset(First->LastNewlineOffset),
+        First->WhitespaceRange.getBegin().getLocWithOffset(
+            First->LastNewlineOffset),
         Last->Tok.getLocation().getLocWithOffset(Last->TokenLength - 1));
     return touchesRanges(LineRange);
   }
@@ -1528,8 +1544,9 @@ private:
   bool touchesEmptyLineBefore(const AnnotatedLine &TheLine) {
     const FormatToken *First = &TheLine.First.FormatTok;
     CharSourceRange LineRange = CharSourceRange::getCharRange(
-        First->WhiteSpaceStart,
-        First->WhiteSpaceStart.getLocWithOffset(First->LastNewlineOffset));
+        First->WhitespaceRange.getBegin(),
+        First->WhitespaceRange.getBegin().getLocWithOffset(
+            First->LastNewlineOffset));
     return touchesRanges(LineRange);
   }
 

diff --git a/lib/Format/UnwrappedLineParser.cpp b/lib/Format/UnwrappedLineParser.cpp
index 9c33e4f484ee9c1a2f9e737761421472925d7e74..081f5b933e0f77ac53ea363f4d17939ba38e4409 100644 (file)
--- a/lib/Format/UnwrappedLineParser.cpp
+++ b/lib/Format/UnwrappedLineParser.cpp
@@ -339,7 +339,8 @@ void UnwrappedLineParser::parsePPDefine() {
   }
   nextToken();
   if (FormatTok.Tok.getKind() == tok::l_paren &&
-      FormatTok.WhiteSpaceLength == 0) {
+      FormatTok.WhitespaceRange.getBegin() ==
+          FormatTok.WhitespaceRange.getEnd()) {
     parseParens();
   }
   addUnwrappedLine();

diff --git a/lib/Format/UnwrappedLineParser.h b/lib/Format/UnwrappedLineParser.h
index 70e860c2b5d6d4bce375e3b5ef67806b50ed718e..4746951dcd7ed572555c09c37080358cfb240ef7 100644 (file)
--- a/lib/Format/UnwrappedLineParser.h
+++ b/lib/Format/UnwrappedLineParser.h
@@ -29,7 +29,7 @@ namespace format {
 /// whitespace characters preceeding it.
 struct FormatToken {
   FormatToken()
-      : NewlinesBefore(0), HasUnescapedNewline(false), WhiteSpaceLength(0),
+      : NewlinesBefore(0), HasUnescapedNewline(false),
         LastNewlineOffset(0), TokenLength(0), IsFirst(false),
         MustBreakBefore(false), TrailingWhiteSpaceLength(0) {}
 
@@ -46,15 +46,8 @@ struct FormatToken {
   /// Token.
   bool HasUnescapedNewline;
 
-  /// \brief The location of the start of the whitespace immediately preceeding
-  /// the \c Token.
-  ///
-  /// Used together with \c WhiteSpaceLength to create a \c Replacement.
-  SourceLocation WhiteSpaceStart;
-
-  /// \brief The length in characters of the whitespace immediately preceeding
-  /// the \c Token.
-  unsigned WhiteSpaceLength;
+  /// \brief The range of the whitespace immediately preceeding the \c Token.
+  SourceRange WhitespaceRange;
 
   /// \brief The offset just past the last '\n' in this token's leading
   /// whitespace (relative to \c WhiteSpaceStart). 0 if there is no '\n'.
@@ -83,7 +76,7 @@ struct FormatToken {
   /// This can be different to Tok.getLocation(), which includes leading escaped
   /// newlines.
   SourceLocation getStartOfNonWhitespace() const {
-    return WhiteSpaceStart.getLocWithOffset(WhiteSpaceLength);
+    return WhitespaceRange.getEnd();
   }
 };
 

diff --git a/lib/Format/WhitespaceManager.cpp b/lib/Format/WhitespaceManager.cpp
index d1a925c5160ae217e43215ae0dee2c205ca39153..a75514ffe7541776cdebd840286bb43dbc74ace1 100644 (file)
--- a/lib/Format/WhitespaceManager.cpp
+++ b/lib/Format/WhitespaceManager.cpp
@@ -43,9 +43,7 @@ void WhitespaceManager::replaceWhitespace(const AnnotatedToken &Tok,
                                           unsigned StartOfTokenColumn,
                                           bool InPPDirective) {
   Changes.push_back(Change(
-      true, SourceRange(Tok.FormatTok.WhiteSpaceStart,
-                        Tok.FormatTok.WhiteSpaceStart.getLocWithOffset(
-                            Tok.FormatTok.WhiteSpaceLength)),
+      true, Tok.FormatTok.WhitespaceRange,
       Spaces, StartOfTokenColumn, Newlines, "", "", Tok.FormatTok.Tok.getKind(),
       InPPDirective && !Tok.FormatTok.IsFirst));
 
@@ -65,14 +63,11 @@ void WhitespaceManager::replaceWhitespace(const AnnotatedToken &Tok,
 
 void WhitespaceManager::addUntouchableToken(const FormatToken &Tok,
                                             bool InPPDirective) {
-  Changes.push_back(Change(
-      false,
-      SourceRange(Tok.WhiteSpaceStart,
-                  Tok.WhiteSpaceStart.getLocWithOffset(Tok.WhiteSpaceLength)),
-      Tok.WhiteSpaceLength - Tok.NewlinesBefore,
-      SourceMgr.getSpellingColumnNumber(Tok.Tok.getLocation()) - 1,
-      Tok.NewlinesBefore, "", "", Tok.Tok.getKind(),
-      InPPDirective && !Tok.IsFirst));
+  Changes.push_back(
+      Change(false, Tok.WhitespaceRange, /*Spaces=*/0,
+             SourceMgr.getSpellingColumnNumber(Tok.Tok.getLocation()) - 1,
+             Tok.NewlinesBefore, "", "", Tok.Tok.getKind(),
+             InPPDirective && !Tok.IsFirst));
 }
 
 void WhitespaceManager::breakToken(const FormatToken &Tok, unsigned Offset,

diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp
index 2d8a1b29cf8c31bf553db99aba68f4e1154d80f7..4919a142a83e813517d20caccde79da16ec5e698 100644 (file)
--- a/unittests/Format/FormatTest.cpp
+++ b/unittests/Format/FormatTest.cpp
@@ -567,6 +567,12 @@ TEST_F(FormatTest, UnderstandsSingleLineComments) {
                "#include \"a/b/c\" // comment");
   verifyFormat("#include <a>     // comment\n"
                "#include <a/b/c> // comment");
+  EXPECT_EQ("#include \\\n"
+            "  \"a\"            // comment\n"
+            "#include \"a/b/c\" // comment",
+            format("#include \\\n"
+                   "  \"a\" // comment\n"
+                   "#include \"a/b/c\" // comment"));
 
   verifyFormat("enum E {\n"
                "  // comment\n"