r257341 - [clang-format] Fix comment aligning when there are changes within the comment
Benjamin Kramer via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 11 08:27:16 PST 2016
Author: d0k
Date: Mon Jan 11 10:27:16 2016
New Revision: 257341
URL: http://llvm.org/viewvc/llvm-project?rev=257341&view=rev
Log:
[clang-format] Fix comment aligning when there are changes within the comment
As soon as a comment had whitespace changes inside of the token, we
couldn't identify the whole comment as a trailing comment anymore and
alignment stopped working. Add a new boolean to Change for this special
case and fix trailing comment identification to use it.
This also changes WhitespaceManager to sum the length of all Changes
inside of a token into the first Change.
Before this fix
int xy; // a
int z; //b
became
int xy; // a
int z; // b
with this patch we immediately get to:
int xy; // a
int z; // b
Differential Revision: http://reviews.llvm.org/D16058
Modified:
cfe/trunk/lib/Format/WhitespaceManager.cpp
cfe/trunk/lib/Format/WhitespaceManager.h
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/lib/Format/WhitespaceManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/WhitespaceManager.cpp?rev=257341&r1=257340&r2=257341&view=diff
==============================================================================
--- cfe/trunk/lib/Format/WhitespaceManager.cpp (original)
+++ cfe/trunk/lib/Format/WhitespaceManager.cpp Mon Jan 11 10:27:16 2016
@@ -30,7 +30,7 @@ WhitespaceManager::Change::Change(
unsigned IndentLevel, int Spaces, unsigned StartOfTokenColumn,
unsigned NewlinesBefore, StringRef PreviousLinePostfix,
StringRef CurrentLinePrefix, tok::TokenKind Kind, bool ContinuesPPDirective,
- bool IsStartOfDeclName)
+ bool IsStartOfDeclName, bool IsInsideToken)
: CreateReplacement(CreateReplacement),
OriginalWhitespaceRange(OriginalWhitespaceRange),
StartOfTokenColumn(StartOfTokenColumn), NewlinesBefore(NewlinesBefore),
@@ -38,8 +38,8 @@ WhitespaceManager::Change::Change(
CurrentLinePrefix(CurrentLinePrefix), Kind(Kind),
ContinuesPPDirective(ContinuesPPDirective),
IsStartOfDeclName(IsStartOfDeclName), IndentLevel(IndentLevel),
- Spaces(Spaces), IsTrailingComment(false), TokenLength(0),
- PreviousEndOfTokenColumn(0), EscapedNewlineColumn(0),
+ Spaces(Spaces), IsInsideToken(IsInsideToken), IsTrailingComment(false),
+ TokenLength(0), PreviousEndOfTokenColumn(0), EscapedNewlineColumn(0),
StartOfBlockComment(nullptr), IndentationOffset(0) {}
void WhitespaceManager::reset() {
@@ -58,7 +58,8 @@ void WhitespaceManager::replaceWhitespac
Change(/*CreateReplacement=*/true, Tok.WhitespaceRange, IndentLevel,
Spaces, StartOfTokenColumn, Newlines, "", "", Tok.Tok.getKind(),
InPPDirective && !Tok.IsFirst,
- Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName)));
+ Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName),
+ /*IsInsideToken=*/false));
}
void WhitespaceManager::addUntouchableToken(const FormatToken &Tok,
@@ -69,7 +70,8 @@ void WhitespaceManager::addUntouchableTo
/*CreateReplacement=*/false, Tok.WhitespaceRange, /*IndentLevel=*/0,
/*Spaces=*/0, Tok.OriginalColumn, Tok.NewlinesBefore, "", "",
Tok.Tok.getKind(), InPPDirective && !Tok.IsFirst,
- Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName)));
+ Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName),
+ /*IsInsideToken=*/false));
}
void WhitespaceManager::replaceWhitespaceInToken(
@@ -82,15 +84,10 @@ void WhitespaceManager::replaceWhitespac
Changes.push_back(Change(
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
- // calculate the new length of the comment and to calculate the changes
- // for which to do the alignment when aligning comments.
- Tok.is(TT_LineComment) && Newlines > 0 ? tok::comment : tok::unknown,
+ CurrentPrefix, Tok.is(TT_LineComment) ? tok::comment : tok::unknown,
InPPDirective && !Tok.IsFirst,
- Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName)));
+ Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName),
+ /*IsInsideToken=*/Newlines == 0));
}
const tooling::Replacements &WhitespaceManager::generateReplacements() {
@@ -110,6 +107,7 @@ const tooling::Replacements &WhitespaceM
void WhitespaceManager::calculateLineBreakInformation() {
Changes[0].PreviousEndOfTokenColumn = 0;
+ Change *LastOutsideTokenChange = &Changes[0];
for (unsigned i = 1, e = Changes.size(); i != e; ++i) {
unsigned OriginalWhitespaceStart =
SourceMgr.getFileOffset(Changes[i].OriginalWhitespaceRange.getBegin());
@@ -120,11 +118,20 @@ void WhitespaceManager::calculateLineBre
Changes[i].PreviousLinePostfix.size() +
Changes[i - 1].CurrentLinePrefix.size();
+ // If there are multiple changes in this token, sum up all the changes until
+ // the end of the line.
+ if (Changes[i - 1].IsInsideToken)
+ LastOutsideTokenChange->TokenLength +=
+ Changes[i - 1].TokenLength + Changes[i - 1].Spaces;
+ else
+ LastOutsideTokenChange = &Changes[i - 1];
+
Changes[i].PreviousEndOfTokenColumn =
Changes[i - 1].StartOfTokenColumn + Changes[i - 1].TokenLength;
Changes[i - 1].IsTrailingComment =
- (Changes[i].NewlinesBefore > 0 || Changes[i].Kind == tok::eof) &&
+ (Changes[i].NewlinesBefore > 0 || Changes[i].Kind == tok::eof ||
+ (Changes[i].IsInsideToken && Changes[i].Kind == tok::comment)) &&
Changes[i - 1].Kind == tok::comment;
}
// FIXME: The last token is currently not always an eof token; in those
@@ -134,6 +141,10 @@ void WhitespaceManager::calculateLineBre
const WhitespaceManager::Change *LastBlockComment = nullptr;
for (auto &Change : Changes) {
+ // Reset the IsTrailingComment flag for changes inside of trailing comments
+ // so they don't get realigned later.
+ if (Change.IsInsideToken)
+ Change.IsTrailingComment = false;
Change.StartOfBlockComment = nullptr;
Change.IndentationOffset = 0;
if (Change.Kind == tok::comment) {
Modified: cfe/trunk/lib/Format/WhitespaceManager.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/WhitespaceManager.h?rev=257341&r1=257340&r2=257341&view=diff
==============================================================================
--- cfe/trunk/lib/Format/WhitespaceManager.h (original)
+++ cfe/trunk/lib/Format/WhitespaceManager.h Mon Jan 11 10:27:16 2016
@@ -109,7 +109,8 @@ public:
unsigned IndentLevel, int Spaces, unsigned StartOfTokenColumn,
unsigned NewlinesBefore, StringRef PreviousLinePostfix,
StringRef CurrentLinePrefix, tok::TokenKind Kind,
- bool ContinuesPPDirective, bool IsStartOfDeclName);
+ bool ContinuesPPDirective, bool IsStartOfDeclName,
+ bool IsInsideToken);
bool CreateReplacement;
// Changes might be in the middle of a token, so we cannot just keep the
@@ -139,6 +140,10 @@ public:
// comments. Uncompensated negative offset is truncated to 0.
int Spaces;
+ // If this change is inside of a token but not at the start of the token or
+ // directly after a newline.
+ bool IsInsideToken;
+
// \c IsTrailingComment, \c TokenLength, \c PreviousEndOfTokenColumn and
// \c EscapedNewlineColumn will be calculated in
// \c calculateLineBreakInformation.
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=257341&r1=257340&r2=257341&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Jan 11 10:27:16 2016
@@ -1022,6 +1022,15 @@ TEST_F(FormatTest, UnderstandsSingleLine
" lineWith(); // comment\n"
" // at start\n"
"}"));
+ EXPECT_EQ("int xy; // a\n"
+ "int z; // b",
+ format("int xy; // a\n"
+ "int z; //b"));
+ EXPECT_EQ("int xy; // a\n"
+ "int z; // bb",
+ format("int xy; // a\n"
+ "int z; //bb",
+ getLLVMStyleWithColumns(12)));
verifyFormat("#define A \\\n"
" int i; /* iiiiiiiiiiiiiiiiiiiii */ \\\n"
More information about the cfe-commits
mailing list