Unified token breaking logic: support for line comments.

Summary:
Added BreakableLineComment, moved common code from
BreakableBlockComment to newly added BreakableComment. As a side-effect of the
rewrite, found another problem with escaped newlines and had to change
code which removes trailing whitespace from line comments not to break after
this patch.

Reviewers: klimek, djasper

Reviewed By: klimek

CC: cfe-commits

Differential Revision: http://llvm-reviews.chandlerc.com/D682

llvm-svn: 179693

clang/lib/Format/BreakableToken.cpp

@@ -14,25 +14,69 @@
 //===----------------------------------------------------------------------===//
 
 #include "BreakableToken.h"
+#include "llvm/ADT/STLExtras.h"
 #include <algorithm>
 
 namespace clang {
 namespace format {
 
+BreakableToken::Split BreakableComment::getSplit(unsigned LineIndex,
+                                                 unsigned TailOffset,
+                                                 unsigned ColumnLimit) const {
+  StringRef Text = getLine(LineIndex).substr(TailOffset);
+  unsigned ContentStartColumn = getContentStartColumn(LineIndex, TailOffset);
+  if (ColumnLimit <= ContentStartColumn + 1)
+    return Split(StringRef::npos, 0);
+
+  unsigned MaxSplit = ColumnLimit - ContentStartColumn + 1;
+  StringRef::size_type SpaceOffset = Text.rfind(' ', MaxSplit);
+  if (SpaceOffset == StringRef::npos ||
+      Text.find_last_not_of(' ', SpaceOffset) == StringRef::npos) {
+    SpaceOffset = Text.find(' ', MaxSplit);
+  }
+  if (SpaceOffset != StringRef::npos && SpaceOffset != 0) {
+    StringRef BeforeCut = Text.substr(0, SpaceOffset).rtrim();
+    StringRef AfterCut = Text.substr(SpaceOffset).ltrim();
+    return BreakableToken::Split(BeforeCut.size(),
+                                 AfterCut.begin() - BeforeCut.end());
+  }
+  return BreakableToken::Split(StringRef::npos, 0);
+}
+
+void BreakableComment::insertBreak(unsigned LineIndex, unsigned TailOffset,
+                                   Split Split, bool InPPDirective,
+                                   WhitespaceManager &Whitespaces) {
+  StringRef Text = getLine(LineIndex).substr(TailOffset);
+  StringRef AdditionalPrefix = Decoration;
+  if (Text.size() == Split.first + Split.second) {
+    // For all but the last line handle trailing space in trimLine.
+    if (LineIndex < Lines.size() - 1)
+      return;
+    // For the last line we need to break before "*/", but not to add "* ".
+    AdditionalPrefix = "";
+  }
+
+  unsigned WhitespaceStartColumn =
+      getContentStartColumn(LineIndex, TailOffset) + Split.first;
+  unsigned BreakOffset = Text.data() - TokenText.data() + Split.first;
+  unsigned CharsToRemove = Split.second;
+  Whitespaces.breakToken(Tok, BreakOffset, CharsToRemove, "", AdditionalPrefix,
+                         InPPDirective, IndentAtLineBreak,
+                         WhitespaceStartColumn);
+}
+
 BreakableBlockComment::BreakableBlockComment(const SourceManager &SourceMgr,
                                              const AnnotatedToken &Token,
                                              unsigned StartColumn)
-    : Tok(Token.FormatTok), StartColumn(StartColumn) {
+    : BreakableComment(SourceMgr, Token.FormatTok, StartColumn + 2) {
-
-  SourceLocation TokenLoc = Tok.Tok.getLocation();
-  TokenText = StringRef(SourceMgr.getCharacterData(TokenLoc), Tok.TokenLength);
   assert(TokenText.startswith("/*") && TokenText.endswith("*/"));
 
-  OriginalStartColumn = SourceMgr.getSpellingColumnNumber(TokenLoc) - 1;
+  OriginalStartColumn =
+      SourceMgr.getSpellingColumnNumber(Tok.getStartOfNonWhitespace()) - 1;
 
   TokenText.substr(2, TokenText.size() - 4).split(Lines, "\n");
 
-  NeedsStar = true;
+  bool NeedsStar = true;
   CommonPrefixLength = UINT_MAX;
   if (Lines.size() == 1) {
     if (Token.Parent == 0) {
@@ -60,13 +104,15 @@ BreakableBlockComment::BreakableBlockComment(const SourceManager &SourceMgr,
   if (CommonPrefixLength == UINT_MAX)
     CommonPrefixLength = 0;
 
+  Decoration = NeedsStar ? "* " : "";
+
   IndentAtLineBreak =
       std::max<int>(StartColumn - OriginalStartColumn + CommonPrefixLength, 0);
 }
 
 void BreakableBlockComment::alignLines(WhitespaceManager &Whitespaces) {
-  SourceLocation TokenLoc = Tok.Tok.getLocation();
+  SourceLocation TokenLoc = Tok.getStartOfNonWhitespace();
-  int IndentDelta = StartColumn - OriginalStartColumn;
+  int IndentDelta = (StartColumn - 2) - OriginalStartColumn;
   if (IndentDelta > 0) {
     std::string WhiteSpace(IndentDelta, ' ');
     for (size_t i = 1; i < Lines.size(); ++i) {
@@ -89,54 +135,7 @@ void BreakableBlockComment::alignLines(WhitespaceManager &Whitespaces) {
   }
 
   for (unsigned i = 1; i < Lines.size(); ++i)
-    Lines[i] = Lines[i].substr(CommonPrefixLength + (NeedsStar ? 2 : 0));
+    Lines[i] = Lines[i].substr(CommonPrefixLength + Decoration.size());
-}
-
-BreakableToken::Split BreakableBlockComment::getSplit(unsigned LineIndex,
-                                                      unsigned TailOffset,
-                                                      unsigned ColumnLimit) {
-  StringRef Text = getLine(LineIndex).substr(TailOffset);
-  unsigned DecorationLength =
-      (TailOffset == 0 && LineIndex == 0) ? StartColumn + 2 : getPrefixLength();
-  if (ColumnLimit <= DecorationLength + 1)
-    return Split(StringRef::npos, 0);
-
-  unsigned MaxSplit = ColumnLimit - DecorationLength + 1;
-  StringRef::size_type SpaceOffset = Text.rfind(' ', MaxSplit);
-  if (SpaceOffset == StringRef::npos ||
-      Text.find_last_not_of(' ', SpaceOffset) == StringRef::npos) {
-    SpaceOffset = Text.find(' ', MaxSplit);
-  }
-  if (SpaceOffset != StringRef::npos && SpaceOffset != 0) {
-    StringRef BeforeCut = Text.substr(0, SpaceOffset).rtrim();
-    StringRef AfterCut = Text.substr(SpaceOffset).ltrim();
-    return BreakableToken::Split(BeforeCut.size(),
-                                 AfterCut.begin() - BeforeCut.end());
-  }
-  return BreakableToken::Split(StringRef::npos, 0);
-}
-
-void BreakableBlockComment::insertBreak(unsigned LineIndex, unsigned TailOffset,
-                                        Split Split, bool InPPDirective,
-                                        WhitespaceManager &Whitespaces) {
-  StringRef Text = getLine(LineIndex).substr(TailOffset);
-  StringRef AdditionalPrefix = NeedsStar ? "* " : "";
-  if (Text.size() == Split.first + Split.second) {
-    // For all but the last line handle trailing space separately.
-    if (LineIndex < Lines.size() - 1)
-      return;
-    // For the last line we need to break before "*/", but not to add "* ".
-    AdditionalPrefix = "";
-  }
-
-  unsigned WhitespaceStartColumn =
-      Split.first +
-      (LineIndex == 0 && TailOffset == 0 ? StartColumn + 2 : getPrefixLength());
-  unsigned BreakOffset = Text.data() - TokenText.data() + Split.first;
-  unsigned CharsToRemove = Split.second;
-  Whitespaces.breakToken(Tok, BreakOffset, CharsToRemove, "", AdditionalPrefix,
-                         InPPDirective, IndentAtLineBreak,
-                         WhitespaceStartColumn);
 }
 
 void BreakableBlockComment::trimLine(unsigned LineIndex, unsigned TailOffset,
@@ -157,5 +156,24 @@ void BreakableBlockComment::trimLine(unsigned LineIndex, unsigned TailOffset,
                          0, WhitespaceStartColumn);
 }
 
+BreakableLineComment::BreakableLineComment(const SourceManager &SourceMgr,
+                                           const AnnotatedToken &Token,
+                                           unsigned StartColumn)
+    : BreakableComment(SourceMgr, Token.FormatTok, StartColumn) {
+  assert(TokenText.startswith("//"));
+  Decoration = getLineCommentPrefix(TokenText);
+  Lines.push_back(TokenText.substr(Decoration.size()));
+  IndentAtLineBreak = StartColumn;
+  this->StartColumn += Decoration.size(); // Start column of the contents.
+}
+
+StringRef BreakableLineComment::getLineCommentPrefix(StringRef Comment) {
+  const char *KnownPrefixes[] = { "/// ", "///", "// ", "//" };
+  for (size_t i = 0; i < llvm::array_lengthof(KnownPrefixes); ++i)
+    if (Comment.startswith(KnownPrefixes[i]))
+      return KnownPrefixes[i];
+  return "";
+}
+
 } // namespace format
 } // namespace clang

clang/lib/Format/BreakableToken.h

@@ -26,54 +26,58 @@ namespace format {
 
 class BreakableToken {
 public:
+  BreakableToken(const SourceManager &SourceMgr, const FormatToken &Tok,
+                 unsigned StartColumn)
+      : Tok(Tok), StartColumn(StartColumn),
+        TokenText(SourceMgr.getCharacterData(Tok.getStartOfNonWhitespace()),
+                  Tok.TokenLength) {}
   virtual ~BreakableToken() {}
   virtual unsigned getLineCount() const = 0;
-  virtual unsigned getLineSize(unsigned Index) = 0;
+  virtual unsigned getLineSize(unsigned Index) const = 0;
   virtual unsigned getLineLengthAfterSplit(unsigned LineIndex,
-                                           unsigned TailOffset) = 0;
+                                           unsigned TailOffset) const = 0;
-  virtual unsigned getPrefixLength() = 0;
-  virtual unsigned getSuffixLength(unsigned LineIndex) = 0;
 
   // Contains starting character index and length of split.
   typedef std::pair<StringRef::size_type, unsigned> Split;
   virtual Split getSplit(unsigned LineIndex, unsigned TailOffset,
-                         unsigned ColumnLimit) = 0;
+                         unsigned ColumnLimit) const = 0;
   virtual void insertBreak(unsigned LineIndex, unsigned TailOffset, Split Split,
                            bool InPPDirective,
                            WhitespaceManager &Whitespaces) = 0;
   virtual void trimLine(unsigned LineIndex, unsigned TailOffset,
                         unsigned InPPDirective,
-                        WhitespaceManager &Whitespaces) = 0;
+                        WhitespaceManager &Whitespaces) {}
+protected:
+  const FormatToken &Tok;
+  unsigned StartColumn;
+  StringRef TokenText;
 };
 
 class BreakableStringLiteral : public BreakableToken {
 public:
-  BreakableStringLiteral(const FormatToken &Tok, unsigned StartColumn)
+  BreakableStringLiteral(const SourceManager &SourceMgr, const FormatToken &Tok,
-      : Tok(Tok), StartColumn(StartColumn) {}
+                         unsigned StartColumn)
+      : BreakableToken(SourceMgr, Tok, StartColumn) {
+    assert(TokenText.startswith("\"") && TokenText.endswith("\""));
+  }
 
   virtual unsigned getLineCount() const { return 1; }
 
-  virtual unsigned getLineSize(unsigned Index) {
+  virtual unsigned getLineSize(unsigned Index) const {
     return Tok.TokenLength - 2; // Should be in sync with getLine
   }
 
   virtual unsigned getLineLengthAfterSplit(unsigned LineIndex,
-                                           unsigned TailOffset) {
+                                           unsigned TailOffset) const {
-    return getPrefixLength() + getLine(LineIndex).size() - TailOffset +
+    return getDecorationLength() + getLine().size() - TailOffset;
-           getSuffixLength(LineIndex);
   }
 
-  virtual unsigned getPrefixLength() { return StartColumn + 1; }
-
-  virtual unsigned getSuffixLength(unsigned LineIndex) { return 1; }
-
   virtual Split getSplit(unsigned LineIndex, unsigned TailOffset,
-                         unsigned ColumnLimit) {
+                         unsigned ColumnLimit) const {
-    StringRef Text = getLine(LineIndex).substr(TailOffset);
+    StringRef Text = getLine().substr(TailOffset);
-    unsigned DecorationLength = getPrefixLength() + getSuffixLength(0);
+    if (ColumnLimit <= getDecorationLength())
-    if (ColumnLimit <= DecorationLength)
       return Split(StringRef::npos, 0);
-    unsigned MaxSplit = ColumnLimit - DecorationLength;
+    unsigned MaxSplit = ColumnLimit - getDecorationLength();
     assert(MaxSplit < Text.size());
     StringRef::size_type SpaceOffset = Text.rfind(' ', MaxSplit);
     if (SpaceOffset != StringRef::npos && SpaceOffset != 0)
@@ -91,22 +95,20 @@ public:
   virtual void insertBreak(unsigned LineIndex, unsigned TailOffset, Split Split,
                            bool InPPDirective, WhitespaceManager &Whitespaces) {
     unsigned WhitespaceStartColumn = StartColumn + Split.first + 2;
-    Whitespaces.breakToken(Tok, TailOffset + Split.first + 1, Split.second,
+    Whitespaces.breakToken(Tok, 1 + TailOffset + Split.first, Split.second,
                            "\"", "\"", InPPDirective, StartColumn,
                            WhitespaceStartColumn);
   }
 
-  virtual void trimLine(unsigned LineIndex, unsigned TailOffset,
-                        unsigned InPPDirective,
-                        WhitespaceManager &Whitespaces) {}
-
 private:
-  StringRef getLine(unsigned Index) {
+  StringRef getLine() const {
     // Get string without quotes.
     // FIXME: Handle string prefixes.
-    return StringRef(Tok.Tok.getLiteralData() + 1, Tok.TokenLength - 2);
+    return TokenText.substr(1, TokenText.size() - 2);
   }
 
+  unsigned getDecorationLength() const { return StartColumn + 2; }
+
   static StringRef::size_type getStartOfCharacter(StringRef Text,
                                                   StringRef::size_type Offset) {
     StringRef::size_type NextEscape = Text.find('\\');
@@ -157,67 +159,79 @@ private:
     return I;
   }
 
-  const FormatToken &Tok;
-  unsigned StartColumn;
 };
 
-class BreakableBlockComment : public BreakableToken {
+class BreakableComment : public BreakableToken {
+public:
+  virtual unsigned getLineSize(unsigned Index) const {
+    return getLine(Index).size();
+  }
+
+  virtual unsigned getLineCount() const { return Lines.size(); }
+
+  virtual unsigned getLineLengthAfterSplit(unsigned LineIndex,
+                                           unsigned TailOffset) const {
+    return getContentStartColumn(LineIndex, TailOffset) +
+           getLine(LineIndex).size() - TailOffset;
+  }
+
+  virtual Split getSplit(unsigned LineIndex, unsigned TailOffset,
+                         unsigned ColumnLimit) const;
+  virtual void insertBreak(unsigned LineIndex, unsigned TailOffset, Split Split,
+                           bool InPPDirective, WhitespaceManager &Whitespaces);
+
+protected:
+  BreakableComment(const SourceManager &SourceMgr, const FormatToken &Tok,
+                   unsigned StartColumn)
+      : BreakableToken(SourceMgr, Tok, StartColumn) {}
+
+  // Get comment lines without /* */, common prefix and trailing whitespace.
+  // Last line is not trimmed, as it is terminated by */, so its trailing
+  // whitespace is not really trailing.
+  StringRef getLine(unsigned Index) const {
+    return Index < Lines.size() - 1 ? Lines[Index].rtrim() : Lines[Index];
+  }
+
+  unsigned getContentStartColumn(unsigned LineIndex,
+                                 unsigned TailOffset) const {
+    return (TailOffset == 0 && LineIndex == 0)
+               ? StartColumn
+               : IndentAtLineBreak + Decoration.size();
+  }
+
+  unsigned IndentAtLineBreak;
+  StringRef Decoration;
+  SmallVector<StringRef, 16> Lines;
+};
+
+class BreakableBlockComment : public BreakableComment {
 public:
   BreakableBlockComment(const SourceManager &SourceMgr,
                         const AnnotatedToken &Token, unsigned StartColumn);
 
   void alignLines(WhitespaceManager &Whitespaces);
 
-  virtual unsigned getLineCount() const { return Lines.size(); }
-
-  virtual unsigned getLineSize(unsigned Index) {
-    return getLine(Index).size();
-  }
-
   virtual unsigned getLineLengthAfterSplit(unsigned LineIndex,
-                                           unsigned TailOffset) {
+                                           unsigned TailOffset) const {
-    unsigned ContentStartColumn = getPrefixLength();
+    return BreakableComment::getLineLengthAfterSplit(LineIndex, TailOffset) +
-    if (TailOffset == 0 && LineIndex == 0)
+           (LineIndex + 1 < Lines.size() ? 0 : 2);
-      ContentStartColumn = StartColumn + 2;
-    return ContentStartColumn + getLine(LineIndex).size() - TailOffset +
-           getSuffixLength(LineIndex);
   }
 
-  virtual unsigned getPrefixLength() {
-    return IndentAtLineBreak + (NeedsStar ? 2 : 0);
-  }
-
-  virtual unsigned getSuffixLength(unsigned LineIndex) {
-    if (LineIndex + 1 < Lines.size())
-      return 0;
-    return 2;
-  }
-
-  virtual Split getSplit(unsigned LineIndex, unsigned TailOffset,
-                         unsigned ColumnLimit);
-
-  virtual void insertBreak(unsigned LineIndex, unsigned TailOffset, Split Split,
-                           bool InPPDirective, WhitespaceManager &Whitespaces);
-
   virtual void trimLine(unsigned LineIndex, unsigned TailOffset,
                         unsigned InPPDirective, WhitespaceManager &Whitespaces);
 
 private:
-  // Get comment lines without /* */, common prefix and trailing whitespace.
-  // Last line is not trimmed, as it is terminated by */, so its trailing
-  // whitespace is not really trailing.
-  StringRef getLine(unsigned Index) {
-    return Index < Lines.size() - 1 ? Lines[Index].rtrim() : Lines[Index];
-  }
-
-  const FormatToken &Tok;
-  const unsigned StartColumn;
-  StringRef TokenText;
   unsigned OriginalStartColumn;
   unsigned CommonPrefixLength;
-  unsigned IndentAtLineBreak;
+};
-  bool NeedsStar;
+
-  SmallVector<StringRef, 16> Lines;
+class BreakableLineComment : public BreakableComment {
+public:
+  BreakableLineComment(const SourceManager &SourceMgr,
+                       const AnnotatedToken &Token, unsigned StartColumn);
+
+private:
+  static StringRef getLineCommentPrefix(StringRef Comment);
 };
 
 } // namespace format

clang/lib/Format/Format.cpp

@@ -617,54 +617,47 @@ private:
     unsigned StartColumn = State.Column - Current.FormatTok.TokenLength;
     if (Current.is(tok::string_literal)) {
       // Only break up default narrow strings.
-      const char *LiteralData = Current.FormatTok.Tok.getLiteralData();
+      const char *LiteralData = SourceMgr.getCharacterData(
+          Current.FormatTok.getStartOfNonWhitespace());
       if (!LiteralData || *LiteralData != '"')
         return 0;
 
-      Token.reset(new BreakableStringLiteral(Current.FormatTok, StartColumn));
+      Token.reset(new BreakableStringLiteral(SourceMgr, Current.FormatTok,
+                                             StartColumn));
     } else if (Current.Type == TT_BlockComment) {
       BreakableBlockComment *BBC =
           new BreakableBlockComment(SourceMgr, Current, StartColumn);
       if (!DryRun)
         BBC->alignLines(Whitespaces);
       Token.reset(BBC);
+    } else if (Current.Type == TT_LineComment) {
+      Token.reset(new BreakableLineComment(SourceMgr, Current, StartColumn));
     } else {
       return 0;
     }
 
-    if (Token->getPrefixLength() + Token->getSuffixLength(0) >
-        getColumnLimit()) {
-      return 0;
-    }
-
     bool BreakInserted = false;
     unsigned Penalty = 0;
     for (unsigned LineIndex = 0; LineIndex < Token->getLineCount();
          ++LineIndex) {
-      unsigned TokenLineSize = Token->getLineSize(LineIndex);
       unsigned TailOffset = 0;
       unsigned RemainingLength =
           Token->getLineLengthAfterSplit(LineIndex, TailOffset);
       while (RemainingLength > getColumnLimit()) {
-        unsigned DecorationLength =
-            RemainingLength - (TokenLineSize - TailOffset);
-        if (DecorationLength + 1 > getColumnLimit()) {
-          // Can't reduce line length by splitting here.
-          break;
-        }
         BreakableToken::Split Split =
             Token->getSplit(LineIndex, TailOffset, getColumnLimit());
         if (Split.first == StringRef::npos)
           break;
         assert(Split.first != 0);
+        unsigned NewRemainingLength = Token->getLineLengthAfterSplit(
+            LineIndex, TailOffset + Split.first + Split.second);
+        if (NewRemainingLength >= RemainingLength)
+          break;
         if (!DryRun) {
           Token->insertBreak(LineIndex, TailOffset, Split, Line.InPPDirective,
                              Whitespaces);
         }
         TailOffset += Split.first + Split.second;
-        unsigned NewRemainingLength =
-            Token->getLineLengthAfterSplit(LineIndex, TailOffset);
-        assert(NewRemainingLength < RemainingLength);
         RemainingLength = NewRemainingLength;
         Penalty += Style.PenaltyExcessCharacter;
         BreakInserted = true;
@@ -925,6 +918,11 @@ public:
     // Now FormatTok is the next non-whitespace token.
     FormatTok.TokenLength = Text.size();
 
+    if (FormatTok.Tok.is(tok::comment)) {
+      FormatTok.TrailingWhiteSpaceLength = Text.size() - Text.rtrim().size();
+      FormatTok.TokenLength -= FormatTok.TrailingWhiteSpaceLength;
+    }
+
     // In case the token starts with escaped newlines, we want to
     // take them into account as whitespace - this pattern is quite frequent
     // in macro definitions.
@@ -951,11 +949,6 @@ public:
       GreaterStashed = true;
     }
 
-    // If we reformat comments, we remove trailing whitespace. Update the length
-    // accordingly.
-    if (FormatTok.Tok.is(tok::comment))
-      FormatTok.TokenLength = Text.rtrim().size();
-
     return FormatTok;
   }
 

clang/lib/Format/UnwrappedLineParser.h

@@ -34,7 +34,7 @@ struct FormatToken {
   FormatToken()
       : NewlinesBefore(0), HasUnescapedNewline(false), WhiteSpaceLength(0),
         LastNewlineOffset(0), TokenLength(0), IsFirst(false),
-        MustBreakBefore(false) {}
+        MustBreakBefore(false), TrailingWhiteSpaceLength(0) {}
 
   /// \brief The \c Token.
   Token Tok;
@@ -76,6 +76,18 @@ struct FormatToken {
   /// This happens for example when a preprocessor directive ended directly
   /// before the token.
   bool MustBreakBefore;
+
+  /// \brief Number of characters of trailing whitespace.
+  unsigned TrailingWhiteSpaceLength;
+
+  /// \brief Returns actual token start location without leading escaped
+  /// newlines and whitespace.
+  ///
+  /// This can be different to Tok.getLocation(), which includes leading escaped
+  /// newlines.
+  SourceLocation getStartOfNonWhitespace() const {
+    return WhiteSpaceStart.getLocWithOffset(WhiteSpaceLength);
+  }
 };
 
 /// \brief An unwrapped line is a sequence of \c Token, that we would like to

clang/lib/Format/WhitespaceManager.cpp

@@ -25,20 +25,19 @@ void WhitespaceManager::replaceWhitespace(const AnnotatedToken &Tok,
   if (NewLines >= 2)
     alignComments();
 
-  SourceLocation TokenLoc = Tok.FormatTok.Tok.getLocation();
-  bool LineExceedsColumnLimit =
-      Spaces + WhitespaceStartColumn + Tok.FormatTok.TokenLength >
-      Style.ColumnLimit;
-
   // Align line comments if they are trailing or if they continue other
   // trailing comments.
   if (Tok.isTrailingComment()) {
+    SourceLocation TokenEndLoc = Tok.FormatTok.getStartOfNonWhitespace()
+        .getLocWithOffset(Tok.FormatTok.TokenLength);
     // Remove the comment's trailing whitespace.
-    if (Tok.FormatTok.Tok.getLength() != Tok.FormatTok.TokenLength)
+    if (Tok.FormatTok.TrailingWhiteSpaceLength != 0)
       Replaces.insert(tooling::Replacement(
-          SourceMgr, TokenLoc.getLocWithOffset(Tok.FormatTok.TokenLength),
+          SourceMgr, TokenEndLoc, Tok.FormatTok.TrailingWhiteSpaceLength, ""));
-          Tok.FormatTok.Tok.getLength() - Tok.FormatTok.TokenLength, ""));
 
+    bool LineExceedsColumnLimit =
+        Spaces + WhitespaceStartColumn + Tok.FormatTok.TokenLength >
+        Style.ColumnLimit;
     // Align comment with other comments.
     if ((Tok.Parent != NULL || !Comments.empty()) && !LineExceedsColumnLimit) {
       StoredComment Comment;
@@ -58,16 +57,6 @@ void WhitespaceManager::replaceWhitespace(const AnnotatedToken &Tok,
   if (Tok.Children.empty() && !Tok.isTrailingComment())
     alignComments();
 
-  if (Tok.Type == TT_LineComment && LineExceedsColumnLimit) {
-    StringRef Line(SourceMgr.getCharacterData(TokenLoc),
-                   Tok.FormatTok.TokenLength);
-    int StartColumn = Spaces + (NewLines == 0 ? WhitespaceStartColumn : 0);
-    StringRef Prefix = getLineCommentPrefix(Line);
-    std::string NewPrefix = std::string(StartColumn, ' ') + Prefix.str();
-    splitLineComment(Tok.FormatTok, Line.substr(Prefix.size()),
-                     StartColumn + Prefix.size(), NewPrefix);
-  }
-
   storeReplacement(Tok.FormatTok, getNewLineText(NewLines, Spaces));
 }
 
@@ -89,7 +78,8 @@ void WhitespaceManager::breakToken(const FormatToken &Tok, unsigned Offset,
   else
     NewLineText = getNewLineText(1, Spaces, WhitespaceStartColumn);
   std::string ReplacementText = (Prefix + NewLineText + Postfix).str();
-  SourceLocation Location = Tok.Tok.getLocation().getLocWithOffset(Offset);
+  SourceLocation Location =
+      Tok.getStartOfNonWhitespace().getLocWithOffset(Offset);
   Replaces.insert(
       tooling::Replacement(SourceMgr, Location, ReplaceChars, ReplacementText));
 }
@@ -113,50 +103,6 @@ void WhitespaceManager::addUntouchableComment(unsigned Column) {
   Comments.push_back(Comment);
 }
 
-StringRef WhitespaceManager::getLineCommentPrefix(StringRef Comment) {
-  const char *KnownPrefixes[] = { "/// ", "///", "// ", "//" };
-  for (size_t i = 0; i < llvm::array_lengthof(KnownPrefixes); ++i)
-    if (Comment.startswith(KnownPrefixes[i]))
-      return KnownPrefixes[i];
-  return "";
-}
-
-void
-WhitespaceManager::splitLineComment(const FormatToken &Tok, StringRef Line,
-                                    size_t StartColumn, StringRef LinePrefix,
-                                    const char *WhiteSpaceChars /*= " "*/) {
-  const char *TokenStart = SourceMgr.getCharacterData(Tok.Tok.getLocation());
-
-  StringRef TrimmedLine = Line.rtrim();
-  // Don't touch leading whitespace.
-  Line = TrimmedLine.ltrim();
-  StartColumn += TrimmedLine.size() - Line.size();
-
-  while (Line.size() + StartColumn > Style.ColumnLimit) {
-    // Try to break at the last whitespace before the column limit.
-    size_t SpacePos =
-        Line.find_last_of(WhiteSpaceChars, Style.ColumnLimit - StartColumn + 1);
-    if (SpacePos == StringRef::npos) {
-      // Try to find any whitespace in the line.
-      SpacePos = Line.find_first_of(WhiteSpaceChars);
-      if (SpacePos == StringRef::npos) // No whitespace found, give up.
-        break;
-    }
-
-    StringRef NextCut = Line.substr(0, SpacePos).rtrim();
-    StringRef RemainingLine = Line.substr(SpacePos).ltrim();
-    if (RemainingLine.empty())
-      break;
-
-    Line = RemainingLine;
-
-    size_t ReplaceChars = Line.begin() - NextCut.end();
-    breakToken(Tok, NextCut.end() - TokenStart, ReplaceChars, "", LinePrefix,
-               false, 0, 0);
-    StartColumn = LinePrefix.size();
-  }
-}
-
 std::string WhitespaceManager::getNewLineText(unsigned NewLines,
                                               unsigned Spaces) {
   return std::string(NewLines, '\n') + std::string(Spaces, ' ');

clang/lib/Format/WhitespaceManager.h

@@ -67,21 +67,6 @@ public:
   void addUntouchableComment(unsigned Column);
 
 private:
-  static StringRef getLineCommentPrefix(StringRef Comment);
-
-  /// \brief Splits one line in a line comment, if it doesn't fit to
-  /// provided column limit. Removes trailing whitespace in each line.
-  ///
-  /// \param Line points to the line contents without leading // or /*.
-  ///
-  /// \param StartColumn is the column where the first character of Line will be
-  /// located after formatting.
-  ///
-  /// \param LinePrefix is inserted after each line break.
-  void splitLineComment(const FormatToken &Tok, StringRef Line,
-                        size_t StartColumn, StringRef LinePrefix,
-                        const char *WhiteSpaceChars = " ");
-
   std::string getNewLineText(unsigned NewLines, unsigned Spaces);
 
   std::string getNewLineText(unsigned NewLines, unsigned Spaces,

clang/unittests/Format/FormatTest.cpp

@@ -635,7 +635,7 @@ TEST_F(FormatTest, UnderstandsMultiLineComments) {
   EXPECT_EQ(
       "f(aaaaaaaaaaaaaaaaaaaaaaaaa, /* Trailing comment for aa... */\n"
       "  bbbbbbbbbbbbbbbbbbbbbbbbb);",
-      format("f(aaaaaaaaaaaaaaaaaaaaaaaaa ,  /* Trailing comment for aa... */\n"
+      format("f(aaaaaaaaaaaaaaaaaaaaaaaaa ,   \\\n/* Trailing comment for aa... */\n"
              "  bbbbbbbbbbbbbbbbbbbbbbbbb);"));
   EXPECT_EQ(
       "f(aaaaaaaaaaaaaaaaaaaaaaaaa,\n"
@@ -701,6 +701,16 @@ TEST_F(FormatTest, SplitsLongCxxComments) {
             "// one line",
             format("// A comment that doesn't fit on one line",
                    getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("// a b c d\n"
+            "// e f  g\n"
+            "// h i j k",
+            format("// a b c d e f  g h i j k",
+                   getLLVMStyleWithColumns(10)));
+  EXPECT_EQ("// a b c d\n"
+            "// e f  g\n"
+            "// h i j k",
+            format("\\\n// a b c d e f  g h i j k",
+                   getLLVMStyleWithColumns(10)));
   EXPECT_EQ("if (true) // A comment that\n"
             "          // doesn't fit on\n"
             "          // one line",
@@ -743,6 +753,18 @@ TEST_F(FormatTest, SplitsLongLinesInComments) {
                    "doesn't                                    "
                    "fit on one line.  */",
                    getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("/* a b c d\n"
+            " * e f  g\n"
+            " * h i j k\n"
+            " */",
+            format("/* a b c d e f  g h i j k */",
+                   getLLVMStyleWithColumns(10)));
+  EXPECT_EQ("/* a b c d\n"
+            " * e f  g\n"
+            " * h i j k\n"
+            " */",
+            format("\\\n/* a b c d e f  g h i j k */",
+                   getLLVMStyleWithColumns(10)));
   EXPECT_EQ("/*\n"
             "This is a long\n"
             "comment that doesn't\n"
@@ -3670,6 +3692,9 @@ TEST_F(FormatTest, BreakStringLiterals) {
   EXPECT_EQ("\"some text \"\n"
             "\"other\";",
             format("\"some text other\";", getLLVMStyleWithColumns(12)));
+  EXPECT_EQ("\"some text \"\n"
+            "\"other\";",
+            format("\\\n\"some text other\";", getLLVMStyleWithColumns(12)));
   EXPECT_EQ(
       "#define A  \\\n"
       "  \"some \"  \\\n"