From 9e90b62e018d8983d3477f3f1a4abeb685e50ae9 Mon Sep 17 00:00:00 2001 From: Alexander Kornienko Date: Wed, 17 Apr 2013 17:34:05 +0000 Subject: [PATCH] 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 | 130 +++++++++++--------- clang/lib/Format/BreakableToken.h | 158 ++++++++++++++----------- clang/lib/Format/Format.cpp | 37 +++--- clang/lib/Format/UnwrappedLineParser.h | 14 ++- clang/lib/Format/WhitespaceManager.cpp | 72 ++--------- clang/lib/Format/WhitespaceManager.h | 15 --- clang/unittests/Format/FormatTest.cpp | 27 ++++- 7 files changed, 223 insertions(+), 230 deletions(-) diff --git a/clang/lib/Format/BreakableToken.cpp b/clang/lib/Format/BreakableToken.cpp index 4ec3de960899..3e2e0ce7cf3d 100644 --- a/clang/lib/Format/BreakableToken.cpp +++ b/clang/lib/Format/BreakableToken.cpp @@ -14,25 +14,69 @@ //===----------------------------------------------------------------------===// #include "BreakableToken.h" +#include "llvm/ADT/STLExtras.h" #include 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) { - - SourceLocation TokenLoc = Tok.Tok.getLocation(); - TokenText = StringRef(SourceMgr.getCharacterData(TokenLoc), Tok.TokenLength); + : BreakableComment(SourceMgr, Token.FormatTok, StartColumn + 2) { 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(StartColumn - OriginalStartColumn + CommonPrefixLength, 0); } void BreakableBlockComment::alignLines(WhitespaceManager &Whitespaces) { - SourceLocation TokenLoc = Tok.Tok.getLocation(); - int IndentDelta = StartColumn - OriginalStartColumn; + SourceLocation TokenLoc = Tok.getStartOfNonWhitespace(); + 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)); -} - -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); + Lines[i] = Lines[i].substr(CommonPrefixLength + Decoration.size()); } 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 diff --git a/clang/lib/Format/BreakableToken.h b/clang/lib/Format/BreakableToken.h index 0609104a6fc5..c1303183d312 100644 --- a/clang/lib/Format/BreakableToken.h +++ b/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; - virtual unsigned getPrefixLength() = 0; - virtual unsigned getSuffixLength(unsigned LineIndex) = 0; + unsigned TailOffset) const = 0; // Contains starting character index and length of split. typedef std::pair 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) - : Tok(Tok), StartColumn(StartColumn) {} + BreakableStringLiteral(const SourceManager &SourceMgr, const FormatToken &Tok, + 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) { - return getPrefixLength() + getLine(LineIndex).size() - TailOffset + - getSuffixLength(LineIndex); + unsigned TailOffset) const { + return getDecorationLength() + getLine().size() - TailOffset; } - virtual unsigned getPrefixLength() { return StartColumn + 1; } - - virtual unsigned getSuffixLength(unsigned LineIndex) { return 1; } - virtual Split getSplit(unsigned LineIndex, unsigned TailOffset, - unsigned ColumnLimit) { - StringRef Text = getLine(LineIndex).substr(TailOffset); - unsigned DecorationLength = getPrefixLength() + getSuffixLength(0); - if (ColumnLimit <= DecorationLength) + unsigned ColumnLimit) const { + StringRef Text = getLine().substr(TailOffset); + if (ColumnLimit <= getDecorationLength()) 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 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 ContentStartColumn = getPrefixLength(); - if (TailOffset == 0 && LineIndex == 0) - ContentStartColumn = StartColumn + 2; - return ContentStartColumn + getLine(LineIndex).size() - TailOffset + - getSuffixLength(LineIndex); + unsigned TailOffset) const { + return BreakableComment::getLineLengthAfterSplit(LineIndex, TailOffset) + + (LineIndex + 1 < Lines.size() ? 0 : 2); } - 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 Lines; +}; + +class BreakableLineComment : public BreakableComment { +public: + BreakableLineComment(const SourceManager &SourceMgr, + const AnnotatedToken &Token, unsigned StartColumn); + +private: + static StringRef getLineCommentPrefix(StringRef Comment); }; } // namespace format diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index a847d4ef9bd5..5c064a6df9ea 100644 --- a/clang/lib/Format/Format.cpp +++ b/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; } diff --git a/clang/lib/Format/UnwrappedLineParser.h b/clang/lib/Format/UnwrappedLineParser.h index 1326d500e256..0c618e24d44e 100644 --- a/clang/lib/Format/UnwrappedLineParser.h +++ b/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 diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp index 6801e7e8c900..d29cd52b0eed 100644 --- a/clang/lib/Format/WhitespaceManager.cpp +++ b/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), - Tok.FormatTok.Tok.getLength() - Tok.FormatTok.TokenLength, "")); + SourceMgr, TokenEndLoc, Tok.FormatTok.TrailingWhiteSpaceLength, "")); + 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, ' '); diff --git a/clang/lib/Format/WhitespaceManager.h b/clang/lib/Format/WhitespaceManager.h index d88006959568..252997f6d8d4 100644 --- a/clang/lib/Format/WhitespaceManager.h +++ b/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, diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index ff277156e3de..2ba6969d2543 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/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"