[cfe-commits] r173168 - in /cfe/trunk: lib/Format/Format.cpp lib/Format/UnwrappedLineParser.cpp lib/Format/UnwrappedLineParser.h unittests/Format/FormatTest.cpp
Manuel Klimek
klimek at google.com
Tue Jan 22 08:31:55 PST 2013
Author: klimek
Date: Tue Jan 22 10:31:55 2013
New Revision: 173168
URL: http://llvm.org/viewvc/llvm-project?rev=173168&view=rev
Log:
Implements more principled comment parsing.
Changing nextToken() in the UnwrappedLineParser to get the next
non-comment token. This allows us to correctly layout a whole class of
snippets, like:
if /* */(/* */ a /* */) /* */
f() /* */; /* */
else /* */
g();
Fixes a bug in the formatter where we would assume there is a previous
non-comment token.
Also adds the indent level of an unwrapped line to the debug output in
the parser.
Modified:
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/lib/Format/UnwrappedLineParser.cpp
cfe/trunk/lib/Format/UnwrappedLineParser.h
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/lib/Format/Format.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=173168&r1=173167&r2=173168&view=diff
==============================================================================
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Tue Jan 22 10:31:55 2013
@@ -586,7 +586,8 @@
if (Previous.is(tok::l_paren) || Previous.is(tok::l_brace) ||
State.NextToken->Parent->Type == TT_TemplateOpener)
State.Stack[ParenLevel].Indent = State.Column + Spaces;
- if (Current.getPreviousNoneComment()->is(tok::comma) &&
+ if (Current.getPreviousNoneComment() != NULL &&
+ Current.getPreviousNoneComment()->is(tok::comma) &&
Current.isNot(tok::comment))
State.Stack[ParenLevel].HasMultiParameterLine = true;
@@ -1639,6 +1640,7 @@
// FIXME: Add a more explicit test.
unsigned i = 0;
while (i + 1 < Text.size() && Text[i] == '\\' && Text[i + 1] == '\n') {
+ // FIXME: ++FormatTok.NewlinesBefore is missing...
FormatTok.WhiteSpaceLength += 2;
FormatTok.TokenLength -= 2;
i += 2;
Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=173168&r1=173167&r2=173168&view=diff
==============================================================================
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original)
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Tue Jan 22 10:31:55 2013
@@ -130,6 +130,7 @@
bool UnwrappedLineParser::parseFile() {
bool Error = parseLevel(/*HasOpeningBrace=*/false);
// Make sure to format the remaining tokens.
+ flushComments(true);
addUnwrappedLine();
return Error;
}
@@ -174,12 +175,14 @@
Line->Level += AddLevels;
parseLevel(/*HasOpeningBrace=*/true);
- Line->Level -= AddLevels;
- if (!FormatTok.Tok.is(tok::r_brace))
+ if (!FormatTok.Tok.is(tok::r_brace)) {
+ Line->Level -= AddLevels;
return true;
+ }
nextToken(); // Munch the closing brace.
+ Line->Level -= AddLevels;
return false;
}
@@ -232,18 +235,8 @@
addUnwrappedLine();
}
-void UnwrappedLineParser::parseComments() {
- // Consume leading line comments, e.g. for branches without compounds.
- while (FormatTok.Tok.is(tok::comment)) {
- nextToken();
- addUnwrappedLine();
- }
-}
-
void UnwrappedLineParser::parseStructuralElement() {
assert(!FormatTok.Tok.is(tok::l_brace));
- parseComments();
-
int TokenNumber = 0;
switch (FormatTok.Tok.getKind()) {
case tok::at:
@@ -598,11 +591,6 @@
++Line->Level;
do {
switch (FormatTok.Tok.getKind()) {
- case tok::comment:
- // FIXME: Handle comments centrally, instead of special casing
- // them everywhere.
- parseComments();
- break;
case tok::l_paren:
parseParens();
break;
@@ -731,13 +719,8 @@
void UnwrappedLineParser::addUnwrappedLine() {
if (Line->Tokens.empty())
return;
- // Consume trailing comments.
- while (!eof() && FormatTok.NewlinesBefore == 0 &&
- FormatTok.Tok.is(tok::comment)) {
- nextToken();
- }
DEBUG({
- llvm::dbgs() << "Line: ";
+ llvm::dbgs() << "Line(" << Line->Level << "): ";
for (std::list<FormatToken>::iterator I = Line->Tokens.begin(),
E = Line->Tokens.end();
I != E; ++I) {
@@ -763,28 +746,63 @@
return FormatTok.Tok.is(tok::eof);
}
+void UnwrappedLineParser::flushComments(bool NewlineBeforeNext) {
+ bool JustComments = Line->Tokens.empty();
+ for (SmallVectorImpl<FormatToken>::const_iterator
+ I = CommentsBeforeNextToken.begin(),
+ E = CommentsBeforeNextToken.end();
+ I != E; ++I) {
+ if (I->HasUnescapedNewline && JustComments) {
+ addUnwrappedLine();
+ }
+ pushToken(*I);
+ }
+ if (NewlineBeforeNext && JustComments) {
+ addUnwrappedLine();
+ }
+ CommentsBeforeNextToken.clear();
+}
+
void UnwrappedLineParser::nextToken() {
if (eof())
return;
- Line->Tokens.push_back(FormatTok);
- if (MustBreakBeforeNextToken) {
- Line->Tokens.back().MustBreakBefore = true;
- MustBreakBeforeNextToken = false;
- }
+ flushComments(FormatTok.HasUnescapedNewline);
+ pushToken(FormatTok);
readToken();
}
void UnwrappedLineParser::readToken() {
- FormatTok = Tokens->getNextToken();
- while (!Line->InPPDirective && FormatTok.Tok.is(tok::hash) &&
- ((FormatTok.NewlinesBefore > 0 && FormatTok.HasUnescapedNewline) ||
- FormatTok.IsFirst)) {
- // If there is an unfinished unwrapped line, we flush the preprocessor
- // directives only after that unwrapped line was finished later.
- bool SwitchToPreprocessorLines = !Line->Tokens.empty() &&
- CurrentLines == &Lines;
- ScopedLineState BlockState(*this, SwitchToPreprocessorLines);
- parsePPDirective();
+ bool CommentsInCurrentLine = true;
+ do {
+ FormatTok = Tokens->getNextToken();
+ while (!Line->InPPDirective && FormatTok.Tok.is(tok::hash) &&
+ ((FormatTok.NewlinesBefore > 0 && FormatTok.HasUnescapedNewline) ||
+ FormatTok.IsFirst)) {
+ // If there is an unfinished unwrapped line, we flush the preprocessor
+ // directives only after that unwrapped line was finished later.
+ bool SwitchToPreprocessorLines = !Line->Tokens.empty() &&
+ CurrentLines == &Lines;
+ ScopedLineState BlockState(*this, SwitchToPreprocessorLines);
+ parsePPDirective();
+ }
+ if (!FormatTok.Tok.is(tok::comment))
+ return;
+ if (FormatTok.HasUnescapedNewline || FormatTok.IsFirst) {
+ CommentsInCurrentLine = false;
+ }
+ if (CommentsInCurrentLine) {
+ pushToken(FormatTok);
+ } else {
+ CommentsBeforeNextToken.push_back(FormatTok);
+ }
+ } while (!eof());
+}
+
+void UnwrappedLineParser::pushToken(const FormatToken &Tok) {
+ Line->Tokens.push_back(Tok);
+ if (MustBreakBeforeNextToken) {
+ Line->Tokens.back().MustBreakBefore = true;
+ MustBreakBeforeNextToken = false;
}
}
Modified: cfe/trunk/lib/Format/UnwrappedLineParser.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.h?rev=173168&r1=173167&r2=173168&view=diff
==============================================================================
--- cfe/trunk/lib/Format/UnwrappedLineParser.h (original)
+++ cfe/trunk/lib/Format/UnwrappedLineParser.h Tue Jan 22 10:31:55 2013
@@ -128,7 +128,6 @@
void parsePPDirective();
void parsePPDefine();
void parsePPUnknown();
- void parseComments();
void parseStructuralElement();
void parseBracedList();
void parseReturn();
@@ -151,11 +150,19 @@
bool eof() const;
void nextToken();
void readToken();
+ void flushComments(bool NewlineBeforeNext);
+ void pushToken(const FormatToken &Tok);
// FIXME: We are constantly running into bugs where Line.Level is incorrectly
// subtracted from beyond 0. Introduce a method to subtract from Line.Level
// and use that everywhere in the Parser.
OwningPtr<UnwrappedLine> Line;
+
+ // Comments are sorted into unwrapped lines by whether they are in the same
+ // line as the previous token, or not. If not, they belong to the next token.
+ // Since the next token might already be in a new unwrapped line, we need to
+ // store the comments belonging to that token.
+ SmallVector<FormatToken, 1> CommentsBeforeNextToken;
FormatToken FormatTok;
bool MustBreakBeforeNextToken;
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=173168&r1=173167&r2=173168&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Tue Jan 22 10:31:55 2013
@@ -1625,6 +1625,47 @@
" if (true) continue;", ShortMergedIf);
}
+TEST_F(FormatTest, BlockCommentsInControlLoops) {
+ verifyFormat("if (0) /* a comment in a strange place */ {\n"
+ " f();\n"
+ "}");
+ verifyFormat("if (0) /* a comment in a strange place */ {\n"
+ " f();\n"
+ "} /* another comment */ else /* comment #3 */ {\n"
+ " g();\n"
+ "}");
+ verifyFormat("while (0) /* a comment in a strange place */ {\n"
+ " f();\n"
+ "}");
+ verifyFormat("for (;;) /* a comment in a strange place */ {\n"
+ " f();\n"
+ "}");
+ verifyFormat("do /* a comment in a strange place */ {\n"
+ " f();\n"
+ "} /* another comment */ while (0);");
+}
+
+TEST_F(FormatTest, BlockComments) {
+ EXPECT_EQ("/* */ /* */ /* */\n/* */ /* */ /* */",
+ format("/* *//* */ /* */\n/* *//* */ /* */"));
+ EXPECT_EQ("/* */ a /* */ b;",
+ format(" /* */ a/* */ b;"));
+ EXPECT_EQ("#define A /* */\\\n"
+ " b\n"
+ "/* */\n"
+ "someCall(\n"
+ " parameter);",
+ format("#define A /* */ b\n"
+ "/* */\n"
+ "someCall(parameter);", getLLVMStyleWithColumns(15)));
+
+ EXPECT_EQ("#define A\n"
+ "/* */ someCall(\n"
+ " parameter);",
+ format("#define A\n"
+ "/* */someCall(parameter);", getLLVMStyleWithColumns(15)));
+}
+
//===----------------------------------------------------------------------===//
// Objective-C tests.
//===----------------------------------------------------------------------===//
More information about the cfe-commits
mailing list