From 06c84f2e071203e6a2acaa3ff0c1c30d63c9c063 Mon Sep 17 00:00:00 2001 From: Manuel Klimek Date: Wed, 20 Nov 2013 11:20:32 +0000 Subject: [PATCH] Fix bug where optimization would lead to strange line breaks. Before: void f() { CHECK_EQ(aaaa, ( *bbbbbbbbb)->cccccc) << "qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq"; } After: void f() { CHECK_EQ(aaaa, (*bbbbbbbbb)->cccccc) << "qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq"; } llvm-svn: 195240 --- clang/lib/Format/ContinuationIndenter.cpp | 22 ++++++++++++++++------ clang/lib/Format/ContinuationIndenter.h | 4 ++-- clang/unittests/Format/FormatTest.cpp | 5 +++++ 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp index 971acc2b7a3c..bbc1ac538246 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -224,13 +224,14 @@ unsigned ContinuationIndenter::addTokenToState(LineState &State, bool Newline, if (Newline) Penalty = addTokenOnNewLine(State, DryRun); else - addTokenOnCurrentLine(State, DryRun, ExtraSpaces); + Penalty = addTokenOnCurrentLine(State, DryRun, ExtraSpaces); return moveStateToNextToken(State, DryRun, Newline) + Penalty; } -void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun, - unsigned ExtraSpaces) { +unsigned ContinuationIndenter::addTokenOnCurrentLine(LineState &State, + bool DryRun, + unsigned ExtraSpaces) { FormatToken &Current = *State.NextToken; const FormatToken &Previous = *State.NextToken->Previous; if (Current.is(tok::equal) && @@ -249,6 +250,15 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun, State.Stack.back().LastSpace = State.Stack.back().VariablePos; } + unsigned Penalty = 0; + // A break before a "<<" will get Style.PenaltyBreakFirstLessLess, so a + // continuation with "<<" has a smaller penalty in general. + // If the LHS is long, we don't want to penalize the break though, so we + // also add Style.PenaltyBreakFirstLessLess. + if (Current.is(tok::lessless) && State.Stack.back().FirstLessLess == 0 && + State.Column > Style.ColumnLimit / 2) + Penalty += Style.PenaltyBreakFirstLessLess; + unsigned Spaces = Current.SpacesRequiredBefore + ExtraSpaces; if (!DryRun) @@ -307,6 +317,7 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun, State.Stack[State.Stack.size() - 2].CallContinuation == 0) State.Stack.back().LastSpace = State.Column; } + return Penalty; } unsigned ContinuationIndenter::addTokenOnNewLine(LineState &State, @@ -332,9 +343,8 @@ unsigned ContinuationIndenter::addTokenOnNewLine(LineState &State, Penalty += State.NextToken->SplitPenalty; // Breaking before the first "<<" is generally not desirable if the LHS is - // short. - if (Current.is(tok::lessless) && State.Stack.back().FirstLessLess == 0 && - State.Column <= Style.ColumnLimit / 2) + // short (not breaking with a long LHS is penalized in addTokenOnCurrentLine). + if (Current.is(tok::lessless) && State.Stack.back().FirstLessLess == 0) Penalty += Style.PenaltyBreakFirstLessLess; if (Current.is(tok::l_brace) && Current.BlockKind == BK_Block) { diff --git a/clang/lib/Format/ContinuationIndenter.h b/clang/lib/Format/ContinuationIndenter.h index b31756583389..b675b0a0c552 100644 --- a/clang/lib/Format/ContinuationIndenter.h +++ b/clang/lib/Format/ContinuationIndenter.h @@ -91,8 +91,8 @@ private: /// /// If \p DryRun is \c false, also creates and stores the required /// \c Replacement. - void addTokenOnCurrentLine(LineState &State, bool DryRun, - unsigned ExtraSpaces); + unsigned addTokenOnCurrentLine(LineState &State, bool DryRun, + unsigned ExtraSpaces); /// \brief Appends the next token to \p State and updates information /// necessary for indentation. diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index b6574c7503aa..d9ca12996fa0 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -3744,6 +3744,11 @@ TEST_F(FormatTest, AlignsPipes) { EXPECT_EQ("llvm::errs() << \"\n" " << a;", format("llvm::errs() << \"\n<cccccc)\n" + " << \"qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq\";\n" + "}"); } TEST_F(FormatTest, UnderstandsEquals) {