r206472 - Fix alignment of trailing block comments.
Alexander Kornienko
alexfh at google.com
Thu Apr 17 09:12:47 PDT 2014
Author: alexfh
Date: Thu Apr 17 11:12:46 2014
New Revision: 206472
URL: http://llvm.org/viewvc/llvm-project?rev=206472&view=rev
Log:
Fix alignment of trailing block comments.
Summary:
This patch ensures that the lines of the block comments retain relative
column offsets. In order to do this WhitespaceManager::Changes representing
continuation of block comments keep a pointer on the change representing the
whitespace change before the block comment, and a relative column offset to this
change, so that the correct column can be reconstructed at the end of alignment
process.
Fixes http://llvm.org/PR19325
Reviewers: djasper
Reviewed By: djasper
CC: cfe-commits, klimek
Differential Revision: http://reviews.llvm.org/D3408
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=206472&r1=206471&r2=206472&view=diff
==============================================================================
--- cfe/trunk/lib/Format/BreakableToken.cpp (original)
+++ cfe/trunk/lib/Format/BreakableToken.cpp Thu Apr 17 11:12:46 2014
@@ -350,10 +350,9 @@ void BreakableBlockComment::adjustWhites
Lines[LineIndex].begin() - Lines[LineIndex - 1].end();
// Adjust the start column uniformly across all lines.
- StartOfLineColumn[LineIndex] = std::max<int>(
- 0,
+ StartOfLineColumn[LineIndex] =
encoding::columnWidthWithTabs(Whitespace, 0, Style.TabWidth, Encoding) +
- IndentDelta);
+ IndentDelta;
}
unsigned BreakableBlockComment::getLineCount() const { return Lines.size(); }
@@ -437,7 +436,6 @@ BreakableBlockComment::replaceWhitespace
unsigned WhitespaceOffsetInToken = Lines[LineIndex].data() -
Tok.TokenText.data() -
LeadingWhitespace[LineIndex];
- assert(StartOfLineColumn[LineIndex] >= Prefix.size());
Whitespaces.replaceWhitespaceInToken(
Tok, WhitespaceOffsetInToken, LeadingWhitespace[LineIndex], "", Prefix,
InPPDirective, 1, IndentLevel,
@@ -450,7 +448,7 @@ BreakableBlockComment::getContentStartCo
// If we break, we always break at the predefined indent.
if (TailOffset != 0)
return IndentAtLineBreak;
- return StartOfLineColumn[LineIndex];
+ return std::max(0, StartOfLineColumn[LineIndex]);
}
} // namespace format
Modified: cfe/trunk/lib/Format/BreakableToken.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/BreakableToken.h?rev=206472&r1=206471&r2=206472&view=diff
==============================================================================
--- cfe/trunk/lib/Format/BreakableToken.h (original)
+++ cfe/trunk/lib/Format/BreakableToken.h Thu Apr 17 11:12:46 2014
@@ -212,7 +212,7 @@ private:
// StartOfLineColumn[i] is the target column at which Line[i] should be.
// Note that this excludes a leading "* " or "*" in case all lines have
// a "*" prefix.
- SmallVector<unsigned, 16> StartOfLineColumn;
+ SmallVector<int, 16> StartOfLineColumn;
// The column at which the text of a broken line should start.
// Note that an optional decoration would go before that column.
Modified: cfe/trunk/lib/Format/WhitespaceManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/WhitespaceManager.cpp?rev=206472&r1=206471&r2=206472&view=diff
==============================================================================
--- cfe/trunk/lib/Format/WhitespaceManager.cpp (original)
+++ cfe/trunk/lib/Format/WhitespaceManager.cpp Thu Apr 17 11:12:46 2014
@@ -28,7 +28,7 @@ WhitespaceManager::Change::IsBeforeInFil
WhitespaceManager::Change::Change(
bool CreateReplacement, const SourceRange &OriginalWhitespaceRange,
- unsigned IndentLevel, unsigned Spaces, unsigned StartOfTokenColumn,
+ unsigned IndentLevel, int Spaces, unsigned StartOfTokenColumn,
unsigned NewlinesBefore, StringRef PreviousLinePostfix,
StringRef CurrentLinePrefix, tok::TokenKind Kind, bool ContinuesPPDirective)
: CreateReplacement(CreateReplacement),
@@ -69,14 +69,14 @@ void WhitespaceManager::addUntouchableTo
void WhitespaceManager::replaceWhitespaceInToken(
const FormatToken &Tok, unsigned Offset, unsigned ReplaceChars,
StringRef PreviousPostfix, StringRef CurrentPrefix, bool InPPDirective,
- unsigned Newlines, unsigned IndentLevel, unsigned Spaces) {
+ unsigned Newlines, unsigned IndentLevel, int Spaces) {
if (Tok.Finalized)
return;
+ SourceLocation Start = Tok.getStartOfNonWhitespace().getLocWithOffset(Offset);
Changes.push_back(Change(
- true, SourceRange(Tok.getStartOfNonWhitespace().getLocWithOffset(Offset),
- Tok.getStartOfNonWhitespace().getLocWithOffset(
- Offset + ReplaceChars)),
- IndentLevel, Spaces, Spaces, Newlines, PreviousPostfix, CurrentPrefix,
+ true, SourceRange(Start, Start.getLocWithOffset(ReplaceChars)),
+ IndentLevel, Spaces, std::max(0, Spaces), Newlines, PreviousPostfix,
+ CurrentPrefix,
// If we don't add a newline this change doesn't start a comment. Thus,
// when we align line comments, we don't need to treat this change as one.
// FIXME: We still need to take this change in account to properly
@@ -122,6 +122,22 @@ void WhitespaceManager::calculateLineBre
// cases, setting TokenLength of the last token to 0 is wrong.
Changes.back().TokenLength = 0;
Changes.back().IsTrailingComment = Changes.back().Kind == tok::comment;
+
+ const WhitespaceManager::Change *LastBlockComment = nullptr;
+ for (auto &Change : Changes) {
+ Change.StartOfBlockComment = nullptr;
+ Change.IndentationOffset = 0;
+ if (Change.Kind == tok::comment) {
+ LastBlockComment = &Change;
+ } else if (Change.Kind == tok::unknown) {
+ if ((Change.StartOfBlockComment = LastBlockComment))
+ Change.IndentationOffset =
+ Change.StartOfTokenColumn -
+ Change.StartOfBlockComment->StartOfTokenColumn;
+ } else {
+ LastBlockComment = nullptr;
+ }
+ }
}
void WhitespaceManager::alignTrailingComments() {
@@ -131,58 +147,61 @@ void WhitespaceManager::alignTrailingCom
bool BreakBeforeNext = false;
unsigned Newlines = 0;
for (unsigned i = 0, e = Changes.size(); i != e; ++i) {
+ if (Changes[i].StartOfBlockComment)
+ continue;
+ Newlines += Changes[i].NewlinesBefore;
+ if (!Changes[i].IsTrailingComment)
+ continue;
+
unsigned ChangeMinColumn = Changes[i].StartOfTokenColumn;
// FIXME: Correctly handle ChangeMaxColumn in PP directives.
unsigned ChangeMaxColumn = Style.ColumnLimit - Changes[i].TokenLength;
- Newlines += Changes[i].NewlinesBefore;
- if (Changes[i].IsTrailingComment) {
- // If this comment follows an } in column 0, it probably documents the
- // closing of a namespace and we don't want to align it.
- bool FollowsRBraceInColumn0 = i > 0 && Changes[i].NewlinesBefore == 0 &&
- Changes[i - 1].Kind == tok::r_brace &&
- Changes[i - 1].StartOfTokenColumn == 0;
- bool WasAlignedWithStartOfNextLine = false;
- if (Changes[i].NewlinesBefore == 1) { // A comment on its own line.
- for (unsigned j = i + 1; j != e; ++j) {
- if (Changes[j].Kind != tok::comment) { // Skip over comments.
- // The start of the next token was previously aligned with the
- // start of this comment.
- WasAlignedWithStartOfNextLine =
- (SourceMgr.getSpellingColumnNumber(
- Changes[i].OriginalWhitespaceRange.getEnd()) ==
- SourceMgr.getSpellingColumnNumber(
- Changes[j].OriginalWhitespaceRange.getEnd()));
- break;
- }
+ // If this comment follows an } in column 0, it probably documents the
+ // closing of a namespace and we don't want to align it.
+ bool FollowsRBraceInColumn0 = i > 0 && Changes[i].NewlinesBefore == 0 &&
+ Changes[i - 1].Kind == tok::r_brace &&
+ Changes[i - 1].StartOfTokenColumn == 0;
+ bool WasAlignedWithStartOfNextLine = false;
+ if (Changes[i].NewlinesBefore == 1) { // A comment on its own line.
+ for (unsigned j = i + 1; j != e; ++j) {
+ if (Changes[j].Kind != tok::comment) { // Skip over comments.
+ // The start of the next token was previously aligned with the
+ // start of this comment.
+ WasAlignedWithStartOfNextLine =
+ (SourceMgr.getSpellingColumnNumber(
+ Changes[i].OriginalWhitespaceRange.getEnd()) ==
+ SourceMgr.getSpellingColumnNumber(
+ Changes[j].OriginalWhitespaceRange.getEnd()));
+ break;
}
}
- if (!Style.AlignTrailingComments || FollowsRBraceInColumn0) {
- alignTrailingComments(StartOfSequence, i, MinColumn);
- MinColumn = ChangeMinColumn;
- MaxColumn = ChangeMinColumn;
- StartOfSequence = i;
- } else if (BreakBeforeNext || Newlines > 1 ||
- (ChangeMinColumn > MaxColumn || ChangeMaxColumn < MinColumn) ||
- // Break the comment sequence if the previous line did not end
- // in a trailing comment.
- (Changes[i].NewlinesBefore == 1 && i > 0 &&
- !Changes[i - 1].IsTrailingComment) ||
- WasAlignedWithStartOfNextLine) {
- alignTrailingComments(StartOfSequence, i, MinColumn);
- MinColumn = ChangeMinColumn;
- MaxColumn = ChangeMaxColumn;
- StartOfSequence = i;
- } else {
- MinColumn = std::max(MinColumn, ChangeMinColumn);
- MaxColumn = std::min(MaxColumn, ChangeMaxColumn);
- }
- BreakBeforeNext =
- (i == 0) || (Changes[i].NewlinesBefore > 1) ||
- // Never start a sequence with a comment at the beginning of
- // the line.
- (Changes[i].NewlinesBefore == 1 && StartOfSequence == i);
- Newlines = 0;
}
+ if (!Style.AlignTrailingComments || FollowsRBraceInColumn0) {
+ alignTrailingComments(StartOfSequence, i, MinColumn);
+ MinColumn = ChangeMinColumn;
+ MaxColumn = ChangeMinColumn;
+ StartOfSequence = i;
+ } else if (BreakBeforeNext || Newlines > 1 ||
+ (ChangeMinColumn > MaxColumn || ChangeMaxColumn < MinColumn) ||
+ // Break the comment sequence if the previous line did not end
+ // in a trailing comment.
+ (Changes[i].NewlinesBefore == 1 && i > 0 &&
+ !Changes[i - 1].IsTrailingComment) ||
+ WasAlignedWithStartOfNextLine) {
+ alignTrailingComments(StartOfSequence, i, MinColumn);
+ MinColumn = ChangeMinColumn;
+ MaxColumn = ChangeMaxColumn;
+ StartOfSequence = i;
+ } else {
+ MinColumn = std::max(MinColumn, ChangeMinColumn);
+ MaxColumn = std::min(MaxColumn, ChangeMaxColumn);
+ }
+ BreakBeforeNext =
+ (i == 0) || (Changes[i].NewlinesBefore > 1) ||
+ // Never start a sequence with a comment at the beginning of
+ // the line.
+ (Changes[i].NewlinesBefore == 1 && StartOfSequence == i);
+ Newlines = 0;
}
alignTrailingComments(StartOfSequence, Changes.size(), MinColumn);
}
@@ -190,15 +209,20 @@ void WhitespaceManager::alignTrailingCom
void WhitespaceManager::alignTrailingComments(unsigned Start, unsigned End,
unsigned Column) {
for (unsigned i = Start; i != End; ++i) {
+ int Shift = 0;
if (Changes[i].IsTrailingComment) {
- assert(Column >= Changes[i].StartOfTokenColumn);
- Changes[i].Spaces += Column - Changes[i].StartOfTokenColumn;
- if (i + 1 != End) {
- Changes[i + 1].PreviousEndOfTokenColumn +=
- Column - Changes[i].StartOfTokenColumn;
- }
- Changes[i].StartOfTokenColumn = Column;
+ Shift = Column - Changes[i].StartOfTokenColumn;
+ }
+ if (Changes[i].StartOfBlockComment) {
+ Shift = Changes[i].IndentationOffset +
+ Changes[i].StartOfBlockComment->StartOfTokenColumn -
+ Changes[i].StartOfTokenColumn;
}
+ assert(Shift >= 0);
+ Changes[i].Spaces += Shift;
+ if (i + 1 != End)
+ Changes[i + 1].PreviousEndOfTokenColumn += Shift;
+ Changes[i].StartOfTokenColumn += Shift;
}
}
@@ -245,8 +269,8 @@ void WhitespaceManager::generateChanges(
C.PreviousEndOfTokenColumn, C.EscapedNewlineColumn);
else
appendNewlineText(ReplacementText, C.NewlinesBefore);
- appendIndentText(ReplacementText, C.IndentLevel, C.Spaces,
- C.StartOfTokenColumn - C.Spaces);
+ appendIndentText(ReplacementText, C.IndentLevel, std::max(0, C.Spaces),
+ C.StartOfTokenColumn - std::max(0, C.Spaces));
ReplacementText.append(C.CurrentLinePrefix);
storeReplacement(C.OriginalWhitespaceRange, ReplacementText);
}
Modified: cfe/trunk/lib/Format/WhitespaceManager.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/WhitespaceManager.h?rev=206472&r1=206471&r2=206472&view=diff
==============================================================================
--- cfe/trunk/lib/Format/WhitespaceManager.h (original)
+++ cfe/trunk/lib/Format/WhitespaceManager.h Thu Apr 17 11:12:46 2014
@@ -63,6 +63,12 @@ public:
/// (in this order) at \p Offset inside \p Tok, replacing \p ReplaceChars
/// characters.
///
+ /// Note: \p Spaces can be negative to retain information about initial
+ /// relative column offset between a line of a block comment and the start of
+ /// the comment. This negative offset may be compensated by trailing comment
+ /// alignment here. In all other cases negative \p Spaces will be truncated to
+ /// 0.
+ ///
/// When \p InPPDirective is true, escaped newlines are inserted. \p Spaces is
/// used to align backslashes correctly.
void replaceWhitespaceInToken(const FormatToken &Tok, unsigned Offset,
@@ -70,7 +76,7 @@ public:
StringRef PreviousPostfix,
StringRef CurrentPrefix, bool InPPDirective,
unsigned Newlines, unsigned IndentLevel,
- unsigned Spaces);
+ int Spaces);
/// \brief Returns all the \c Replacements created during formatting.
const tooling::Replacements &generateReplacements();
@@ -101,7 +107,7 @@ private:
/// \p StartOfTokenColumn and \p InPPDirective will be used to lay out
/// trailing comments and escaped newlines.
Change(bool CreateReplacement, const SourceRange &OriginalWhitespaceRange,
- unsigned IndentLevel, unsigned Spaces, unsigned StartOfTokenColumn,
+ unsigned IndentLevel, int Spaces, unsigned StartOfTokenColumn,
unsigned NewlinesBefore, StringRef PreviousLinePostfix,
StringRef CurrentLinePrefix, tok::TokenKind Kind,
bool ContinuesPPDirective);
@@ -128,7 +134,10 @@ private:
// The number of spaces in front of the token or broken part of the token.
// This will be adapted when aligning tokens.
- unsigned Spaces;
+ // Can be negative to retain information about the initial relative offset
+ // of the lines in a block comment. This is used when aligning trailing
+ // comments. Uncompensated negative offset is truncated to 0.
+ int Spaces;
// \c IsTrailingComment, \c TokenLength, \c PreviousEndOfTokenColumn and
// \c EscapedNewlineColumn will be calculated in
@@ -137,6 +146,17 @@ private:
unsigned TokenLength;
unsigned PreviousEndOfTokenColumn;
unsigned EscapedNewlineColumn;
+
+ // These fields are used to retain correct relative line indentation in a
+ // block comment when aligning trailing comments.
+ //
+ // If this Change represents a continuation of a block comment,
+ // \c StartOfBlockComment is pointer to the first Change in the block
+ // comment. \c IndentationOffset is a relative column offset to this
+ // change, so that the correct column can be reconstructed at the end of
+ // the alignment process.
+ const Change *StartOfBlockComment;
+ int IndentationOffset;
};
/// \brief Calculate \c IsTrailingComment, \c TokenLength for the last tokens
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=206472&r1=206471&r2=206472&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Thu Apr 17 11:12:46 2014
@@ -1013,6 +1013,30 @@ TEST_F(FormatTest, AlignsBlockComments)
format("int i; /* Comment with empty...\n"
" *\n"
" * line. */"));
+ EXPECT_EQ("int foobar = 0; /* comment */\n"
+ "int bar = 0; /* multiline\n"
+ " comment 1 */\n"
+ "int baz = 0; /* multiline\n"
+ " comment 2 */\n"
+ "int bzz = 0; /* multiline\n"
+ " comment 3 */",
+ format("int foobar = 0; /* comment */\n"
+ "int bar = 0; /* multiline\n"
+ " comment 1 */\n"
+ "int baz = 0; /* multiline\n"
+ " comment 2 */\n"
+ "int bzz = 0; /* multiline\n"
+ " comment 3 */"));
+ EXPECT_EQ("int foobar = 0; /* comment */\n"
+ "int bar = 0; /* multiline\n"
+ " comment */\n"
+ "int baz = 0; /* multiline\n"
+ "comment */",
+ format("int foobar = 0; /* comment */\n"
+ "int bar = 0; /* multiline\n"
+ "comment */\n"
+ "int baz = 0; /* multiline\n"
+ "comment */"));
}
TEST_F(FormatTest, CorrectlyHandlesLengthOfBlockComments) {
More information about the cfe-commits
mailing list