Author: alexfh Date: Mon Jun 17 07:59:44 2013 New Revision: 184076 URL: http://llvm.org/viewvc/llvm-project?rev=184076&view=rev Log: Fixes incorrect indentation of line comments after break and re-alignment.
Summary: Selectively propagate the information about token kind in WhitespaceManager::replaceWhitespaceInToken.For correct alignment of new segments of line comments in order to align them correctly. Don't set BreakBeforeParameter in breakProtrudingToken for line comments, as it introduces a break after the _next_ parameter. Added tests for related functions. Reviewers: klimek Reviewed By: klimek CC: cfe-commits Differential Revision: http://llvm-reviews.chandlerc.com/D980 Modified: cfe/trunk/lib/Format/Format.cpp cfe/trunk/lib/Format/WhitespaceManager.cpp 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=184076&r1=184075&r2=184076&view=diff ============================================================================== --- cfe/trunk/lib/Format/Format.cpp (original) +++ cfe/trunk/lib/Format/Format.cpp Mon Jun 17 07:59:44 2013 @@ -873,8 +873,14 @@ private: if (BreakInserted) { State.Column = PositionAfterLastLineInToken; - for (unsigned i = 0, e = State.Stack.size(); i != e; ++i) - State.Stack[i].BreakBeforeParameter = true; + // If we break the token inside a parameter list, we need to break before + // the next parameter on all levels, so that the next parameter is clearly + // visible. Line comments already introduce a break. + if (Current.Type != TT_LineComment) { + for (unsigned i = 0, e = State.Stack.size(); i != e; ++i) + State.Stack[i].BreakBeforeParameter = true; + } + State.Stack.back().LastSpace = StartColumn; } return Penalty; Modified: cfe/trunk/lib/Format/WhitespaceManager.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/WhitespaceManager.cpp?rev=184076&r1=184075&r2=184076&view=diff ============================================================================== --- cfe/trunk/lib/Format/WhitespaceManager.cpp (original) +++ cfe/trunk/lib/Format/WhitespaceManager.cpp Mon Jun 17 07:59:44 2013 @@ -65,10 +65,13 @@ void WhitespaceManager::replaceWhitespac Tok.getStartOfNonWhitespace().getLocWithOffset( Offset + ReplaceChars)), Spaces, Spaces, Newlines, PreviousPostfix, CurrentPrefix, - // FIXME: Unify token adjustment, so we don't split it between - // BreakableToken and the WhitespaceManager. That would also allow us to - // correctly store a tok::TokenKind instead of rolling our own enum. - tok::unknown, InPPDirective && !Tok.IsFirst)); + // 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.Type == TT_LineComment && Newlines > 0 ? tok::comment : tok::unknown, + InPPDirective && !Tok.IsFirst)); } const tooling::Replacements &WhitespaceManager::generateReplacements() { Modified: cfe/trunk/unittests/Format/FormatTest.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=184076&r1=184075&r2=184076&view=diff ============================================================================== --- cfe/trunk/unittests/Format/FormatTest.cpp (original) +++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Jun 17 07:59:44 2013 @@ -876,6 +876,16 @@ TEST_F(FormatTest, SplitsLongCxxComments format("// A comment before a macro definition\n" "#define a b", getLLVMStyleWithColumns(20))); + EXPECT_EQ("void ffffff(int aaaaaaaaa, // wwww\n" + " int a, int bbb, // xxxxxxx\n" + " // yyyyyyyyy\n" + " int c, int d, int e) {}", + format("void ffffff(\n" + " int aaaaaaaaa, // wwww\n" + " int a,\n" + " int bbb, // xxxxxxx yyyyyyyyy\n" + " int c, int d, int e) {}", + getLLVMStyleWithColumns(40))); } TEST_F(FormatTest, PriorityOfCommentBreaking) { @@ -2456,6 +2466,14 @@ TEST_F(FormatTest, BreaksDesireably) { " Line.Tokens[i - 1].Tok.isNot(tok::l_paren) &&\n" " Line.Tokens[i - 1].Tok.isNot(tok::l_square);\n" " }\n }\n}"); + + // Break on an outer level if there was a break on an inner level. + EXPECT_EQ("f(g(h(a, // comment\n" + " b, c),\n" + " d, e),\n" + " x, y);", + format("f(g(h(a, // comment\n" + " b, c), d, e), x, y);")); } TEST_F(FormatTest, FormatsOneParameterPerLineIfNecessary) { @@ -4565,6 +4583,18 @@ TEST_F(FormatTest, BreakStringLiterals) format("variable = f(\"long string literal\", short, " "loooooooooooooooooooong);", getLLVMStyleWithColumns(20))); + + EXPECT_EQ("f(g(\"long string \"\n" + " \"literal\"),\n" + " b);", + format("f(g(\"long string literal\"), b);", + getLLVMStyleWithColumns(20))); + EXPECT_EQ("f(g(\"long string \"\n" + " \"literal\",\n" + " a),\n" + " b);", + format("f(g(\"long string literal\", a), b);", + getLLVMStyleWithColumns(20))); EXPECT_EQ( "f(\"one two\".split(\n" " variable));", _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
