maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

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

llvm-svn: 206472
This commit is contained in:
Alexander Kornienko 2014-04-17 16:12:46 +00:00
parent ed5aced64e
commit 67d9c8c87e
5 changed files with 136 additions and 70 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
clang/lib/Format/BreakableToken.cpp
View File

@ -350,10 +350,9 @@ void BreakableBlockComment::adjustWhitespace(unsigned LineIndex,
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::replaceWhitespaceBefore(unsigned LineIndex,
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::getContentStartColumn(unsigned LineIndex,
// 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

2
clang/lib/Format/BreakableToken.h
View File

@ -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.

146
clang/lib/Format/WhitespaceManager.cpp
View File

@ -28,7 +28,7 @@ WhitespaceManager::Change::IsBeforeInFile::operator()(const Change &C1,
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::addUntouchableToken(const FormatToken &Tok,
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::calculateLineBreakInformation() {
// 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::alignTrailingComments() {
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::alignTrailingComments() {
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);
}

26
clang/lib/Format/WhitespaceManager.h
View File

@ -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

24
clang/unittests/Format/FormatTest.cpp
View File

@ -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) {