From: Krasimir Georgiev Date: Mon, 24 Jul 2017 14:51:59 +0000 (+0000) Subject: [clang-format] Fix comment levels between '} else {' and PPDirective. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=6fc7e24c557dbb0e2bd50646574e1de7d8d0dab5;p=clang [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 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@308882 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Format/TokenAnnotator.cpp b/lib/Format/TokenAnnotator.cpp index 46ea06b880..b2ffbbb167 100644 --- a/lib/Format/TokenAnnotator.cpp +++ b/lib/Format/TokenAnnotator.cpp @@ -2820,7 +2820,7 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine &Line, } 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 diff --git a/lib/Format/UnwrappedLineParser.cpp b/lib/Format/UnwrappedLineParser.cpp index 4858e596ca..427dfd5725 100644 --- a/lib/Format/UnwrappedLineParser.cpp +++ b/lib/Format/UnwrappedLineParser.cpp @@ -460,7 +460,7 @@ void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, bool AddLevel, 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(bool MustBeDeclaration, bool AddLevel, 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(bool MustBeDeclaration, bool AddLevel, 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(bool NewlineBeforeNext) { 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::distributeComments( } } -void UnwrappedLineParser::readToken() { +void UnwrappedLineParser::readToken(int LevelDifference) { SmallVector 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(-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. diff --git a/lib/Format/UnwrappedLineParser.h b/lib/Format/UnwrappedLineParser.h index a2aa2f0067..5e1eef7661 100644 --- a/lib/Format/UnwrappedLineParser.h +++ b/lib/Format/UnwrappedLineParser.h @@ -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. diff --git a/unittests/Format/FormatTestComments.cpp b/unittests/Format/FormatTestComments.cpp index 7d2ec83963..d1cf10bffc 100644 --- a/unittests/Format/FormatTestComments.cpp +++ b/unittests/Format/FormatTestComments.cpp @@ -855,6 +855,48 @@ TEST_F(FormatTestComments, KeepsLevelOfCommentBeforePPDirective) { "#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, KeepsLevelOfCommentBeforePPDirective) { "#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, KeepsLevelOfCommentBeforePPDirective) { " 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) {