Author: alexfh Date: Tue Jun 11 11:01:49 2013 New Revision: 183750 URL: http://llvm.org/viewvc/llvm-project?rev=183750&view=rev Log: 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 Modified: cfe/trunk/lib/Format/BreakableToken.cpp cfe/trunk/lib/Format/BreakableToken.h cfe/trunk/lib/Format/WhitespaceManager.cpp cfe/trunk/lib/Format/WhitespaceManager.h cfe/trunk/unittests/Format/FormatTest.cpp Modified: cfe/trunk/lib/Format/BreakableToken.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/BreakableToken.cpp?rev=183750&r1=183749&r2=183750&view=diff ============================================================================== --- cfe/trunk/lib/Format/BreakableToken.cpp (original) +++ cfe/trunk/lib/Format/BreakableToken.cpp Tue Jun 11 11:01:49 2013 @@ -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::getLi 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(unsigne 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::BreakableLineComme 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 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( 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::replaceWhitespace 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 Modified: cfe/trunk/lib/Format/BreakableToken.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/BreakableToken.h?rev=183750&r1=183749&r2=183750&view=diff ============================================================================== --- cfe/trunk/lib/Format/BreakableToken.h (original) +++ cfe/trunk/lib/Format/BreakableToken.h Tue Jun 11 11:01:49 2013 @@ -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 { Modified: cfe/trunk/lib/Format/WhitespaceManager.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/WhitespaceManager.cpp?rev=183750&r1=183749&r2=183750&view=diff ============================================================================== --- cfe/trunk/lib/Format/WhitespaceManager.cpp (original) +++ cfe/trunk/lib/Format/WhitespaceManager.cpp Tue Jun 11 11:01:49 2013 @@ -56,16 +56,15 @@ void WhitespaceManager::addUntouchableTo 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 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) { Modified: cfe/trunk/lib/Format/WhitespaceManager.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/WhitespaceManager.h?rev=183750&r1=183749&r2=183750&view=diff ============================================================================== --- cfe/trunk/lib/Format/WhitespaceManager.h (original) +++ cfe/trunk/lib/Format/WhitespaceManager.h Tue Jun 11 11:01:49 2013 @@ -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); Modified: cfe/trunk/unittests/Format/FormatTest.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=183750&r1=183749&r2=183750&view=diff ============================================================================== --- cfe/trunk/unittests/Format/FormatTest.cpp (original) +++ cfe/trunk/unittests/Format/FormatTest.cpp Tue Jun 11 11:01:49 2013 @@ -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" _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
