From 2b2faa53ecd32e823c55430d0889c11ea91b582c Mon Sep 17 00:00:00 2001 From: Alexander Kornienko Date: Tue, 11 Jun 2013 16:01:49 +0000 Subject: [PATCH] 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 | 57 ++++++++++++++++++++++++-------- lib/Format/BreakableToken.h | 14 ++++++-- lib/Format/WhitespaceManager.cpp | 33 +++++++++--------- lib/Format/WhitespaceManager.h | 24 ++++++++------ unittests/Format/FormatTest.cpp | 13 +++++--- 5 files changed, 93 insertions(+), 48 deletions(-) diff --git a/lib/Format/BreakableToken.cpp b/lib/Format/BreakableToken.cpp index 94b4322e7e..d915aef2f9 100644 --- a/lib/Format/BreakableToken.cpp +++ b/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 diff --git a/lib/Format/BreakableToken.h b/lib/Format/BreakableToken.h index 100bb19dc5..4c8755b95b 100644 --- a/lib/Format/BreakableToken.h +++ b/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 { diff --git a/lib/Format/WhitespaceManager.cpp b/lib/Format/WhitespaceManager.cpp index e6e4e015f9..e3ca32c186 100644 --- a/lib/Format/WhitespaceManager.cpp +++ b/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(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) { diff --git a/lib/Format/WhitespaceManager.h b/lib/Format/WhitespaceManager.h index 25b7690a15..11167fa4c3 100644 --- a/lib/Format/WhitespaceManager.h +++ b/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); diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp index 1b7f699ef3..3a4cd8d324 100644 --- a/unittests/Format/FormatTest.cpp +++ b/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" -- 2.40.0