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) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits