r308882 - [clang-format] Fix comment levels between '} else {' and PPDirective.
Krasimir Georgiev via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 24 07:51:59 PDT 2017
Author: krasimir
Date: Mon Jul 24 07:51:59 2017
New Revision: 308882
URL: http://llvm.org/viewvc/llvm-project?rev=308882&view=rev
Log:
[clang-format] Fix comment levels between '} else {' and PPDirective.
Summary:
This fixes a regression exposed by r307795 and rL308725 in which the level of a
comment line between '} else {' and a preprocessor directive is incorrectly set
as the level of the '} else {' line. For example, this :
```
int f(int i) {
if (i) {
++i;
} else {
// comment
#ifdef A
--i;
#endif
}
}
```
was formatted as:
```
int f(int i) {
if (i) {
++i;
} else {
// comment
#ifdef A
--i;
#endif
}
}
```
Reviewers: djasper, klimek
Reviewed By: klimek
Subscribers: klimek, cfe-commits
Differential Revision: https://reviews.llvm.org/D35794
Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/lib/Format/UnwrappedLineParser.cpp
cfe/trunk/lib/Format/UnwrappedLineParser.h
cfe/trunk/unittests/Format/FormatTestComments.cpp
Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=308882&r1=308881&r2=308882&view=diff
==============================================================================
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Mon Jul 24 07:51:59 2017
@@ -2820,7 +2820,7 @@ bool TokenAnnotator::canBreakBefore(cons
}
void TokenAnnotator::printDebugInfo(const AnnotatedLine &Line) {
- llvm::errs() << "AnnotatedTokens:\n";
+ llvm::errs() << "AnnotatedTokens(L=" << Line.Level << "):\n";
const FormatToken *Tok = Line.First;
while (Tok) {
llvm::errs() << " M=" << Tok->MustBreakBefore
Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=308882&r1=308881&r2=308882&view=diff
==============================================================================
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original)
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Mon Jul 24 07:51:59 2017
@@ -460,7 +460,7 @@ void UnwrappedLineParser::parseBlock(boo
FormatTok->BlockKind = BK_Block;
unsigned InitialLevel = Line->Level;
- nextToken();
+ nextToken(/*LevelDifference=*/AddLevel ? 1 : 0);
if (MacroBlock && FormatTok->is(tok::l_paren))
parseParens();
@@ -486,9 +486,8 @@ void UnwrappedLineParser::parseBlock(boo
return;
}
- flushComments(isOnNewLine(*FormatTok));
- Line->Level = InitialLevel;
- nextToken(); // Munch the closing brace.
+ // Munch the closing brace.
+ nextToken(/*LevelDifference=*/AddLevel ? -1 : 0);
if (MacroBlock && FormatTok->is(tok::l_paren))
parseParens();
@@ -496,6 +495,7 @@ void UnwrappedLineParser::parseBlock(boo
if (MunchSemi && FormatTok->Tok.is(tok::semi))
nextToken();
Line->MatchingOpeningBlockLineIndex = OpeningLineIndex;
+ Line->Level = InitialLevel;
if (OpeningLineIndex != UnwrappedLine::kInvalidIndex) {
// Update the opening line to add the forward reference as well
(*CurrentLines)[OpeningLineIndex].MatchingOpeningBlockLineIndex =
@@ -2288,13 +2288,13 @@ void UnwrappedLineParser::flushComments(
CommentsBeforeNextToken.clear();
}
-void UnwrappedLineParser::nextToken() {
+void UnwrappedLineParser::nextToken(int LevelDifference) {
if (eof())
return;
flushComments(isOnNewLine(*FormatTok));
pushToken(FormatTok);
if (Style.Language != FormatStyle::LK_JavaScript)
- readToken();
+ readToken(LevelDifference);
else
readTokenWithJavaScriptASI();
}
@@ -2363,7 +2363,7 @@ void UnwrappedLineParser::distributeComm
}
}
-void UnwrappedLineParser::readToken() {
+void UnwrappedLineParser::readToken(int LevelDifference) {
SmallVector<FormatToken *, 1> Comments;
do {
FormatTok = Tokens->getNextToken();
@@ -2376,6 +2376,10 @@ void UnwrappedLineParser::readToken() {
// directives only after that unwrapped line was finished later.
bool SwitchToPreprocessorLines = !Line->Tokens.empty();
ScopedLineState BlockState(*this, SwitchToPreprocessorLines);
+ assert((LevelDifference >= 0 ||
+ static_cast<unsigned>(-LevelDifference) <= Line->Level) &&
+ "LevelDifference makes Line->Level negative");
+ Line->Level += LevelDifference;
// Comments stored before the preprocessor directive need to be output
// before the preprocessor directive, at the same level as the
// preprocessor directive, as we consider them to apply to the directive.
Modified: cfe/trunk/lib/Format/UnwrappedLineParser.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.h?rev=308882&r1=308881&r2=308882&view=diff
==============================================================================
--- cfe/trunk/lib/Format/UnwrappedLineParser.h (original)
+++ cfe/trunk/lib/Format/UnwrappedLineParser.h Mon Jul 24 07:51:59 2017
@@ -123,9 +123,13 @@ private:
void tryToParseJSFunction();
void addUnwrappedLine();
bool eof() const;
- void nextToken();
+ // LevelDifference is the difference of levels after and before the current
+ // token. For example:
+ // - if the token is '{' and opens a block, LevelDifference is 1.
+ // - if the token is '}' and closes a block, LevelDifference is -1.
+ void nextToken(int LevelDifference = 0);
+ void readToken(int LevelDifference = 0);
const FormatToken *getPreviousToken();
- void readToken();
// Decides which comment tokens should be added to the current line and which
// should be added as comments before the next token.
Modified: cfe/trunk/unittests/Format/FormatTestComments.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestComments.cpp?rev=308882&r1=308881&r2=308882&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTestComments.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestComments.cpp Mon Jul 24 07:51:59 2017
@@ -855,6 +855,48 @@ TEST_F(FormatTestComments, KeepsLevelOfC
"#endif\n"
"}"));
+ EXPECT_EQ("int f(int i) {\n"
+ " if (true) {\n"
+ " i++;\n"
+ " } else {\n"
+ " // comment in else\n"
+ "#ifdef A\n"
+ " j++;\n"
+ "#endif\n"
+ " }\n"
+ "}",
+ format("int f(int i) {\n"
+ " if (true) {\n"
+ " i++;\n"
+ " } else {\n"
+ " // comment in else\n"
+ "#ifdef A\n"
+ " j++;\n"
+ "#endif\n"
+ " }\n"
+ "}"));
+
+ EXPECT_EQ("int f(int i) {\n"
+ " if (true) {\n"
+ " i++;\n"
+ " } else {\n"
+ " /* comment in else */\n"
+ "#ifdef A\n"
+ " j++;\n"
+ "#endif\n"
+ " }\n"
+ "}",
+ format("int f(int i) {\n"
+ " if (true) {\n"
+ " i++;\n"
+ " } else {\n"
+ " /* comment in else */\n"
+ "#ifdef A\n"
+ " j++;\n"
+ "#endif\n"
+ " }\n"
+ "}"));
+
// Keep the current level if there is an empty line between the comment and
// the preprocessor directive.
EXPECT_EQ("void f() {\n"
@@ -912,8 +954,55 @@ TEST_F(FormatTestComments, KeepsLevelOfC
"#endif\n"
"}"));
+ EXPECT_EQ("int f(int i) {\n"
+ " if (true) {\n"
+ " i++;\n"
+ " } else {\n"
+ " // comment in else\n"
+ "\n"
+ "#ifdef A\n"
+ " j++;\n"
+ "#endif\n"
+ " }\n"
+ "}",
+ format("int f(int i) {\n"
+ " if (true) {\n"
+ " i++;\n"
+ " } else {\n"
+ "// comment in else\n"
+ "\n"
+ "#ifdef A\n"
+ " j++;\n"
+ "#endif\n"
+ " }\n"
+ "}"));
+
+ EXPECT_EQ("int f(int i) {\n"
+ " if (true) {\n"
+ " i++;\n"
+ " } else {\n"
+ " /* comment in else */\n"
+ "\n"
+ "#ifdef A\n"
+ " j++;\n"
+ "#endif\n"
+ " }\n"
+ "}",
+ format("int f(int i) {\n"
+ " if (true) {\n"
+ " i++;\n"
+ " } else {\n"
+ "/* comment in else */\n"
+ "\n"
+ "#ifdef A\n"
+ " j++;\n"
+ "#endif\n"
+ " }\n"
+ "}"));
+
// Align with the preprocessor directive if the comment was originally aligned
- // with the preprocessor directive.
+ // with the preprocessor directive and there is no newline between the comment
+ // and the preprocessor directive.
EXPECT_EQ("void f() {\n"
" int i;\n"
"/* comment */\n"
@@ -945,6 +1034,48 @@ TEST_F(FormatTestComments, KeepsLevelOfC
" int j;\n"
"#endif\n"
"}"));
+
+ EXPECT_EQ("int f(int i) {\n"
+ " if (true) {\n"
+ " i++;\n"
+ " } else {\n"
+ "// comment in else\n"
+ "#ifdef A\n"
+ " j++;\n"
+ "#endif\n"
+ " }\n"
+ "}",
+ format("int f(int i) {\n"
+ " if (true) {\n"
+ " i++;\n"
+ " } else {\n"
+ " // comment in else\n"
+ " #ifdef A\n"
+ " j++;\n"
+ "#endif\n"
+ " }\n"
+ "}"));
+
+ EXPECT_EQ("int f(int i) {\n"
+ " if (true) {\n"
+ " i++;\n"
+ " } else {\n"
+ "/* comment in else */\n"
+ "#ifdef A\n"
+ " j++;\n"
+ "#endif\n"
+ " }\n"
+ "}",
+ format("int f(int i) {\n"
+ " if (true) {\n"
+ " i++;\n"
+ " } else {\n"
+ " /* comment in else */\n"
+ " #ifdef A\n"
+ " j++;\n"
+ "#endif\n"
+ " }\n"
+ "}"));
}
TEST_F(FormatTestComments, SplitsLongLinesInComments) {
More information about the cfe-commits
mailing list