Insert a space at the start of a line comment in case it starts with an alphanumeric character.

Summary:
"//Test" becomes "// Test". This change is aimed to improve code
readability and conformance to certain coding styles. If a comment starts with a
non-alphanumeric character, the space isn't added, e.g. "//-*-c++-*-" stays
unchanged.

Reviewers: klimek

Reviewed By: klimek

CC: cfe-commits

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

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

lib/Format/BreakableToken.cpp

@@ -16,6 +16,7 @@
 #define DEBUG_TYPE "format-token-breaker"
 
 #include "BreakableToken.h"
+#include "clang/Basic/CharInfo.h"
 #include "clang/Format/Format.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Debug.h"
@@ -118,15 +119,6 @@ unsigned BreakableSingleLineToken::getLineLengthAfterSplit(
          encoding::getCodePointCount(Line.substr(Offset, Length), Encoding);
 }
 
-void BreakableSingleLineToken::insertBreak(unsigned LineIndex,
-                                           unsigned TailOffset, Split Split,
-                                           bool InPPDirective,
-                                           WhitespaceManager &Whitespaces) {
-  Whitespaces.breakToken(Tok, Prefix.size() + TailOffset + Split.first,
-                         Split.second, Postfix, Prefix, InPPDirective,
-                         StartColumn);
-}
-
 BreakableSingleLineToken::BreakableSingleLineToken(const FormatToken &Tok,
                                                    unsigned StartColumn,
                                                    StringRef Prefix,
@@ -151,6 +143,15 @@ BreakableStringLiteral::getSplit(unsigned LineIndex, unsigned TailOffset,
                         Encoding);
 }
 
+void BreakableStringLiteral::insertBreak(unsigned LineIndex,
+                                         unsigned TailOffset, Split Split,
+                                         bool InPPDirective,
+                                         WhitespaceManager &Whitespaces) {
+  Whitespaces.replaceWhitespaceInToken(
+      Tok, Prefix.size() + TailOffset + Split.first, Split.second, Postfix,
+      Prefix, InPPDirective, 1, StartColumn);
+}
+
 static StringRef getLineCommentPrefix(StringRef Comment) {
   const char *KnownPrefixes[] = { "/// ", "///", "// ", "//" };
   for (size_t i = 0, e = llvm::array_lengthof(KnownPrefixes); i != e; ++i)
@@ -164,7 +165,16 @@ BreakableLineComment::BreakableLineComment(const FormatToken &Token,
                                            encoding::Encoding Encoding)
     : BreakableSingleLineToken(Token, StartColumn,
                                getLineCommentPrefix(Token.TokenText), "",
-                               Encoding) {}
+                               Encoding) {
+  OriginalPrefix = Prefix;
+  if (Token.TokenText.size() > Prefix.size() &&
+      isAlphanumeric(Token.TokenText[Prefix.size()])) {
+    if (Prefix == "//")
+      Prefix = "// ";
+    else if (Prefix == "///")
+      Prefix = "/// ";
+  }
+}
 
 BreakableToken::Split
 BreakableLineComment::getSplit(unsigned LineIndex, unsigned TailOffset,
@@ -173,6 +183,24 @@ BreakableLineComment::getSplit(unsigned LineIndex, unsigned TailOffset,
                          ColumnLimit, Encoding);
 }
 
+void BreakableLineComment::insertBreak(unsigned LineIndex, unsigned TailOffset,
+                                       Split Split, bool InPPDirective,
+                                       WhitespaceManager &Whitespaces) {
+  Whitespaces.replaceWhitespaceInToken(
+      Tok, OriginalPrefix.size() + TailOffset + Split.first, Split.second,
+      Postfix, Prefix, InPPDirective, 1, StartColumn);
+}
+
+void
+BreakableLineComment::replaceWhitespaceBefore(unsigned LineIndex,
+                                              unsigned InPPDirective,
+                                              WhitespaceManager &Whitespaces) {
+  if (OriginalPrefix != Prefix) {
+    Whitespaces.replaceWhitespaceInToken(Tok, OriginalPrefix.size(), 0, "", "",
+                                         false, 0, 1);
+  }
+}
+
 BreakableBlockComment::BreakableBlockComment(
     const FormatStyle &Style, const FormatToken &Token, unsigned StartColumn,
     unsigned OriginalStartColumn, bool FirstInLine, encoding::Encoding Encoding)
@@ -299,8 +327,9 @@ void BreakableBlockComment::insertBreak(unsigned LineIndex, unsigned TailOffset,
       Text.data() - Tok.TokenText.data() + Split.first;
   unsigned CharsToRemove = Split.second;
   assert(IndentAtLineBreak >= Decoration.size());
-  Whitespaces.breakToken(Tok, BreakOffsetInToken, CharsToRemove, "", Prefix,
-                         InPPDirective, IndentAtLineBreak - Decoration.size());
+  Whitespaces.replaceWhitespaceInToken(Tok, BreakOffsetInToken, CharsToRemove,
+                                       "", Prefix, InPPDirective, 1,
+                                       IndentAtLineBreak - Decoration.size());
 }
 
 void
@@ -331,9 +360,9 @@ BreakableBlockComment::replaceWhitespaceBefore(unsigned LineIndex,
       Lines[LineIndex].data() - Tok.TokenText.data() -
       LeadingWhitespace[LineIndex];
   assert(StartOfLineColumn[LineIndex] >= Prefix.size());
-  Whitespaces.breakToken(
+  Whitespaces.replaceWhitespaceInToken(
       Tok, WhitespaceOffsetInToken, LeadingWhitespace[LineIndex], "", Prefix,
-      InPPDirective, StartOfLineColumn[LineIndex] - Prefix.size());
+      InPPDirective, 1, StartOfLineColumn[LineIndex] - Prefix.size());
 }
 
 unsigned

lib/Format/BreakableToken.h

@@ -84,8 +84,6 @@ public:
   virtual unsigned getLineLengthAfterSplit(unsigned LineIndex,
                                            unsigned TailOffset,
                                            StringRef::size_type Length) const;
-  virtual void insertBreak(unsigned LineIndex, unsigned TailOffset, Split Split,
-                           bool InPPDirective, WhitespaceManager &Whitespaces);
 
 protected:
   BreakableSingleLineToken(const FormatToken &Tok, unsigned StartColumn,
@@ -113,6 +111,9 @@ public:
 
   virtual Split getSplit(unsigned LineIndex, unsigned TailOffset,
                          unsigned ColumnLimit) const;
+  virtual void insertBreak(unsigned LineIndex, unsigned TailOffset, Split Split,
+                           bool InPPDirective,
+                           WhitespaceManager &Whitespaces);
 };
 
 class BreakableLineComment : public BreakableSingleLineToken {
@@ -126,6 +127,15 @@ public:
 
   virtual Split getSplit(unsigned LineIndex, unsigned TailOffset,
                          unsigned ColumnLimit) const;
+  virtual void insertBreak(unsigned LineIndex, unsigned TailOffset, Split Split,
+                           bool InPPDirective, WhitespaceManager &Whitespaces);
+  virtual void replaceWhitespaceBefore(unsigned LineIndex,
+                                       unsigned InPPDirective,
+                                       WhitespaceManager &Whitespaces);
+
+private:
+  // The prefix without an additional space if one was added.
+  StringRef OriginalPrefix;
 };
 
 class BreakableBlockComment : public BreakableToken {

lib/Format/WhitespaceManager.cpp

@@ -56,16 +56,15 @@ void WhitespaceManager::addUntouchableToken(const FormatToken &Tok,
              InPPDirective && !Tok.IsFirst));
 }
 
-void WhitespaceManager::breakToken(const FormatToken &Tok, unsigned Offset,
-                                   unsigned ReplaceChars,
-                                   StringRef PreviousPostfix,
-                                   StringRef CurrentPrefix, bool InPPDirective,
-                                   unsigned Spaces) {
+void WhitespaceManager::replaceWhitespaceInToken(
+    const FormatToken &Tok, unsigned Offset, unsigned ReplaceChars,
+    StringRef PreviousPostfix, StringRef CurrentPrefix, bool InPPDirective,
+    unsigned Newlines, unsigned Spaces) {
   Changes.push_back(Change(
       true, SourceRange(Tok.getStartOfNonWhitespace().getLocWithOffset(Offset),
                         Tok.getStartOfNonWhitespace().getLocWithOffset(
                             Offset + ReplaceChars)),
-      Spaces, Spaces, 1, PreviousPostfix, CurrentPrefix,
+      Spaces, Spaces, Newlines, PreviousPostfix, CurrentPrefix,
       // FIXME: Unify token adjustment, so we don't split it between
       // BreakableToken and the WhitespaceManager. That would also allow us to
       // correctly store a tok::TokenKind instead of rolling our own enum.
@@ -214,10 +213,10 @@ void WhitespaceManager::generateChanges() {
       std::string ReplacementText =
           C.PreviousLinePostfix +
           (C.ContinuesPPDirective
-               ? getNewLineText(C.NewlinesBefore, C.Spaces,
+               ? getNewlineText(C.NewlinesBefore, C.Spaces,
                                 C.PreviousEndOfTokenColumn,
                                 C.EscapedNewlineColumn)
-               : getNewLineText(C.NewlinesBefore, C.Spaces)) +
+               : getNewlineText(C.NewlinesBefore, C.Spaces)) +
           C.CurrentLinePrefix;
       storeReplacement(C.OriginalWhitespaceRange, ReplacementText);
     }
@@ -237,26 +236,26 @@ void WhitespaceManager::storeReplacement(const SourceRange &Range,
       SourceMgr, CharSourceRange::getCharRange(Range), Text));
 }
 
-std::string WhitespaceManager::getNewLineText(unsigned NewLines,
+std::string WhitespaceManager::getNewlineText(unsigned Newlines,
                                               unsigned Spaces) {
-  return std::string(NewLines, '\n') + getIndentText(Spaces);
+  return std::string(Newlines, '\n') + getIndentText(Spaces);
 }
 
-std::string WhitespaceManager::getNewLineText(unsigned NewLines,
+std::string WhitespaceManager::getNewlineText(unsigned Newlines,
                                               unsigned Spaces,
                                               unsigned PreviousEndOfTokenColumn,
                                               unsigned EscapedNewlineColumn) {
-  std::string NewLineText;
-  if (NewLines > 0) {
+  std::string NewlineText;
+  if (Newlines > 0) {
     unsigned Offset =
         std::min<int>(EscapedNewlineColumn - 1, PreviousEndOfTokenColumn);
-    for (unsigned i = 0; i < NewLines; ++i) {
-      NewLineText += std::string(EscapedNewlineColumn - Offset - 1, ' ');
-      NewLineText += "\\\n";
+    for (unsigned i = 0; i < Newlines; ++i) {
+      NewlineText += std::string(EscapedNewlineColumn - Offset - 1, ' ');
+      NewlineText += "\\\n";
       Offset = 0;
     }
   }
-  return NewLineText + getIndentText(Spaces);
+  return NewlineText + getIndentText(Spaces);
 }
 
 std::string WhitespaceManager::getIndentText(unsigned Spaces) {

lib/Format/WhitespaceManager.h

@@ -52,17 +52,19 @@ public:
   /// was not called.
   void addUntouchableToken(const FormatToken &Tok, bool InPPDirective);
 
-  /// \brief Inserts a line break into the middle of a token.
+  /// \brief Inserts or replaces whitespace in the middle of a token.
   ///
-  /// Will break at \p Offset inside \p Tok, putting \p PreviousPostfix before
-  /// the line break and \p CurrentPrefix before the rest of the token starts in
-  /// the next line.
+  /// Inserts \p PreviousPostfix, \p Newlines, \p Spaces and \p CurrentPrefix
+  /// (in this order) at \p Offset inside \p Tok, replacing \p ReplaceChars
+  /// characters.
   ///
-  /// \p InPPDirective and \p Spaces are used to generate the correct line
-  /// break.
-  void breakToken(const FormatToken &Tok, unsigned Offset,
-                  unsigned ReplaceChars, StringRef PreviousPostfix,
-                  StringRef CurrentPrefix, bool InPPDirective, unsigned Spaces);
+  /// When \p InPPDirective is true, escaped newlines are inserted. \p Spaces is
+  /// used to align backslashes correctly.
+  void replaceWhitespaceInToken(const FormatToken &Tok, unsigned Offset,
+                                unsigned ReplaceChars,
+                                StringRef PreviousPostfix,
+                                StringRef CurrentPrefix, bool InPPDirective,
+                                unsigned Newlines, unsigned Spaces);
 
   /// \brief Returns all the \c Replacements created during formatting.
   const tooling::Replacements &generateReplacements();
@@ -151,8 +153,8 @@ private:
 
   /// \brief Stores \p Text as the replacement for the whitespace in \p Range.
   void storeReplacement(const SourceRange &Range, StringRef Text);
-  std::string getNewLineText(unsigned NewLines, unsigned Spaces);
-  std::string getNewLineText(unsigned NewLines, unsigned Spaces,
+  std::string getNewlineText(unsigned Newlines, unsigned Spaces);
+  std::string getNewlineText(unsigned Newlines, unsigned Spaces,
                              unsigned PreviousEndOfTokenColumn,
                              unsigned EscapedNewlineColumn);
   std::string getIndentText(unsigned Spaces);

unittests/Format/FormatTest.cpp

@@ -860,10 +860,15 @@ TEST_F(FormatTest, SplitsLongCxxComments) {
   EXPECT_EQ("//    Don't_touch_leading_whitespace",
             format("//    Don't_touch_leading_whitespace",
                    getLLVMStyleWithColumns(20)));
-  EXPECT_EQ(
-      "//Don't add leading\n"
-      "//whitespace",
-      format("//Don't add leading whitespace", getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("// Add leading\n"
+            "// whitespace",
+            format("//Add leading whitespace", getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("// whitespace", format("//whitespace", getLLVMStyle()));
+  EXPECT_EQ("// Even if it makes the line exceed the column\n"
+            "// limit",
+            format("//Even if it makes the line exceed the column limit",
+                   getLLVMStyleWithColumns(51)));
+  EXPECT_EQ("//--But not here", format("//--But not here", getLLVMStyle()));
   EXPECT_EQ("// A comment before\n"
             "// a macro\n"
             "// definition\n"